Boost C++ Libraries: Ticket #3950: boost::interprocess::mapped_region destructor deletes shm where it shouldn't.

Wed, 24 Feb 2010 22:27:02 GMT

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.

attachment set

Fri, 21 May 2010 19:40:18 GMT

attachment → boost_interprocess.patch

Fri, 21 May 2010 20:07:32 GMT

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 — Fri, 21 May 2010 21:15:20 GMT

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

Fri, 21 May 2010 22:10:56 GMT

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 — Fri, 21 May 2010 22:55:00 GMT

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.

Fri, 21 May 2010 23:20:48 GMT

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@… — Sun, 23 May 2010 01:30:56 GMT

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@… — Sun, 23 May 2010 01:34:24 GMT

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 — Sun, 23 May 2010 07:51:49 GMT

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@… — Sun, 23 May 2010 09:53:51 GMT

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.

status, milestone changed; resolution set

Ion Gaztañaga — Thu, 26 Aug 2010 10:18:38 GMT

status new → closed
resolution → fixed
milestone Boost 1.43.0 → Boost-1.45.0

Fixed for Boost 1.45 in release branch