boost::interprocess::mapped_region destructor deletes shm where it shouldn't.

[Lénaïc Huard <lhuard@…>]

Hello,

I have troubles with the following piece of code:

#include <cstdlib>
#include <boost/interprocess/anonymous_shared_memory.hpp>
#include <boost/interprocess/mapped_region.hpp>

int main()
{
  boost::interprocess::mapped_region aRegion( boost::interprocess::anonymous_shared_memory( sizeof( int ) ) );
  *(int *)aRegion.get_address() = 0x33;

  return EXIT_SUCCESS;
}

It should create an anonymous memory region and writes an int in it.

Whereas this piece of code works when compiled with gcc without optimizations, when it is compiled with -O2 or -O3 with gcc 4.4.3, it produces a segmentation fault.

strace and gdb show that the shared memory is mmapped and unmapped before the assignment is done. (Whereas when the code is compiled without optimization, the shared memory is clearly mapped, then the assignement is done, then it is unmapped.)

My understanding of what happens is that:

• the anonymous_shared_memory() function calls raw_mapped_region_creator::create_posix_mapped_region which creates a local mapped_region object.
• This mapped_region object is non-copyable, but movable.

When the return statement for that local object is called, it invokes mapped_region::mapped_region(BOOST_INTERPROCESS_RV_REF(mapped_region) other) which:

• create the returned object as invalid,
• swap the content of the local object with the one of the "invalid initialized" returned object.

Thanks to this, the returned object has the proper values and the local object is set as "invalid" just before its destructor is called so that the shm won't be unmapped at raw_mapped_region_creator::create_posix_mapped_region and anonymous_shared_memory exit.

For a reason I don't understand, I have the feeling that, when the code is compiled with gcc 4.4.3 -O2 or -O3, this "swap with invalid mapped_region" mechanism is zapped out so that the shm mmapped at the beginning of anonymous_shared_memory is unmapped by ~mapped_region when this function returns.

[Lénaïc Huard <lhuard@…>]

The issue #3951 is very very similar to this one.

Like in this ticket, I strongly suspect a bug in latest gcc which optimizations break something in the tricks emulating Rvalue references. Both issues disappear when the code samples are compiled with latest gcc and -std=c++0x or -std=gnu++0x in which case, the Rvalue references are managed by the compiler and the boost tricks become useless.

[Lénaïc Huard <lhuard@…>]

I found the problem.

The problem was indeed caused by the strict aliasing rule violation by the class rv (in detail/move.hpp)

The case described here above and in #3951 can be solved by flagging the class rv with the gcc specific __attribute__((__may_alias__)).

The full patch that fixes the issue is attached in ​boost_interprocess.patch.

Could you please have a look at it?

[Ion Gaztañaga]

Thanks! What if you modify move.hpp to modify all reinterpret_cast with static_cast? Does this also solve the problem?

[Lénaïc Huard <lhuard@…>]

I tried to replace all the reinterpret_cast by static_cast but it doesn't help to solve the issue.

According to what I've read ​here:

GCC is fairly aggressive about it: enabling strict aliasing will cause it to think that pointers that are "obviously" equivalent to a human (as in, foo* a; bar * b; b = (foo*)a;) cannot alias, which allows for some very aggressive transformations, but can obviously break non-carefully written code.

If rv references alias to references to other "supposed incompatible" types, GCC will assume that operations on the first reference cannot affect the object pointed to by the second reference. In nearly all cases. Hence the issue.

The code example of this issue doesn't raise any warning. But the one of #3951 raises:

/home/lenaic/doc/prog/boost-trunk/boost/interprocess/sync/scoped_lock.hpp:348:
 warning: dereferencing pointer ‘<anonymous>’ does break strict-aliasing rules
/home/lenaic/doc/prog/boost-trunk/boost/interprocess/sync/scoped_lock.hpp:347:
 warning: dereferencing pointer ‘<anonymous>’ does break strict-aliasing rules
/home/lenaic/doc/prog/boost-trunk/boost/interprocess/segment_manager.hpp:1334:
 note: initialized from here

And those warnings are well removed by the addition of __attribute__((__may_alias__)) on rv.

By the way, in the patch I provided, I'm still wondering why I had to touch mapped_region.hpp. But without moving those two methods definition to their declaration, I get this unbelievable error:

/home/lenaic/doc/prog/boost-trunk/boost/interprocess/mapped_region.hpp:381:
 error: prototype for  ‘boost::interprocess::mapped_region::mapped_region(boost::interprocess::rv<boost::interprocess::mapped_region>&)’ does not match any in class ‘boost::interprocess::mapped_region’                                                                                     
/home/lenaic/doc/prog/boost-trunk/boost/interprocess/mapped_region.hpp:84:
 error: candidates are: boost::interprocess::mapped_region::mapped_region(boost::interprocess::rv<boost::interprocess::mapped_region>&)
[...]

[Ion Gaztañaga]

Many thanks, I wouldn't be able to fix it by myself, I thought downcasted classes would not break strict aliasing but I was wrong.

[Lénaïc Huard <lhuard@…>]

You're welcome!

I'm not a strict aliasing rules expert. Maybe is it gcc which is wrong and which performs optimizations that are even more aggressive than what the standard allows. I'm not able to say.

[kab@…]

Replying to igaztanaga:

Many thanks, I wouldn't be able to fix it by myself, I thought downcasted classes would not break strict aliasing but I was wrong.

A downward static_cast should be fine, so long as the object is in fact of the type being downcasted to. See C++03 5.2.9/5. If gcc strict aliasing optimizations are breaking that, that would be very bad.

[kab@…]

Replying to kab@…:

See C++03 5.2.9/5.

Pointers are probably involved in this discussion, in which case the correct reference is C++03 5.2.9/8.

[Ion Gaztañaga]

I think GCC is right, because the dynamic type of the object is not rv<T> but T. We don't create any rv<T> object so downcasting T to a non-existent rv<T> object is at least doubtful. The standard says:

INCITS+ISO+IEC+14882-2003 C++03 standard.

3.10 Lvalues and rvalues / 15

If a program attempts to access the stored value of an object through an lvalue of other than one of the following types the behavior is undefined48):

• the dynamic type of the object,
• a cv-qualified version of the dynamic type of the object,
• a type that is the signed or unsigned type corresponding to the

dynamic type of the object,

• a type that is the signed or unsigned type corresponding to a cv-

qualified version of the dynamic type of the object,

• an aggregate or union type that includes one of the aforementioned

types among its members (including, recursively, a member of a subaggregate or contained union),

• a type that is a (possibly cv-qualified) base class type of the

dynamic type of the object,

• a char or unsigned char type.

rv<T> is a class, but not an aggregate class, since (8.5.1 Aggregates [dcl.init.aggr] p.2):

An aggregate class is a class with no user-declared constructors, no private or protected non-static data members, no base classes, and no virtual functions.

[kab@…]

Replying to igaztanaga:

I think GCC is right, because the dynamic type of the object is not rv<T> but T. We don't create any rv<T> object so downcasting T to a non-existent rv<T> object is at least doubtful.

Ah, I didn't realize that. So there are no objects whose dynamic type is rv<T>. That would mean the static downcast is hitting the "undefined behavior" case at the end of 5.2.9/8. And if the compiler can figure that out (that the T* really can't be an rv<T>*) then it can generate pretty much any old garbage code.

[Ion Gaztañaga]

Fixed for Boost 1.45 in release branch