Re: [boost] gcc-4.4.0 and boost::optional is giving (spurious?) strict-aliasing warnings

Subject: Re: [boost] gcc-4.4.0 and boost::optional is giving (spurious?) strict-aliasing warnings
From: Giovanni Piero Deretta (gpderetta_at_[hidden])
Date: 2009-05-07 07:06:34

On Thu, May 7, 2009 at 3:37 AM, Kim Barrett <kab.conundrums_at_[hidden]> wrote:
>  At 7:17 PM -0300 5/5/09, Brad Spencer wrote:
>>
>
> In boost/aligned_storage.hpp, the class template
> boost::aligned_storage defines its address member function thusly:
>
>    void* address() { return this; }
>    const void* address() const { return this; }
>

I'm not 100% sure, but this could be very wrong as, IIRC, there is no
guarantee that for non PODS the address of the first element (i.e.
aligned_storage.data_) is the same as 'this'.

but why doesn't aligned storage implements address() as:

   void * address { return this.data_.data_.buf; }

BTW, I see that boost optional actually uses its own implementation of
aligned storage, whose address member function returns a pointer to a
char buffer, so it shouldn't be a problem.

> There is code in boost/optional/optional.hpp (and probably the other
> libs you mention) which looks like:
>
>    internal_type* get_object() {
>        return static_cast<internal_type*>(m_storage.address());
>    }
>
> What this leads, after some expansion, to something roughly equivalent
> to
>
>    static_cast<internal_type*>(
>        static_cast<void*>(
>            <a pointer to an instance of aligned_storage> ))
>
> which I believe is not strict aliasing safe. It is effectively a cast
> from aligned_storage* to internal_type*, with a trip through void*
> along the way. That trip through void* does not make the conversion
> satisfy the strict aliasing rules.
>

IIRC aliasing rules talk about dynamic types of objects: roughly, you
can access a memory location only with a type which is the same as the
*dynamic type* of that location or as a char array.

Boost optional does an explicit placement new of the stored type on
the buffer which changes the dynamic type of the buffer so it should
be safe from a standard point of view. Before reusing the buffer It
invokes the destructor of the type, which ends the object lifetime.

That said, gcc has been known to miscompile placement new from time to
time (but it should be more or less fixed on recent versions: it
pretty much acts as a compiler optimization barrier).

In the end, IMHO the warning is spurious and could be safely ignored
(modulo compiler bugs).

> [Note that it is not the cast itself that violates the strict aliasing
> rules, it is the later dereference of the pointer resulting from the
> cast.]
>
> [Note that I'm pretty sure that switching from static_cast to
> reinterpret_cast doesn't help.]
>
> One of the escape hatches provided by the strict aliasing rules
> involves ensuring that there is a union type in scope at the point of
> the conversion which contains both of the involved types. [The
> "visible union" rule of 6.5.2.3#5.]

I do not think this rule actually help you: you can't still read from
a field different from the last written to (except for some exceptions
like structures with common initial sequences.) Most compilers give
additional guarantees, though.

> Another escape hatch offered by the strict aliasing rules involves
> memcpy and memmove. [6.5#6] I think that the following implementation of
> aligned_storage::address will satisfy the strict aliasing rules:
>
>    void* address() {
>        void* data = data_.data_.buf;
>        return memmove(data, data, sizeof(*data));
>    }
>
> And I think gcc will optimize away the memmove, at least at sufficient
> optimization levels. But note that I have not tested this at all.
>
> [Hm, using a size of 0 or 1 or some similar small value might be
> sufficient to quiet the compiler; I'm not certain whether it is
> sufficient to satisfy the letter of the C standard.]

I do not see how this helps, first of all sizeof(*data) is illegal
(sizeof(void) doesn't make any sense, gcc defines it as 1 as an
extension), then how copying one byte will fix aliasing problems?

>
> The idea is that memmove (and memcpy, but that requres non-overlapping
> source and destination, which we don't have here) is defined by the
> aliasing rules to strip away the type of the destination, in a way
> that a simple cast does not.

I do not think it really strips away the type of the destination: in
fact it is often unsed to bit cast from integers to float and back: if
a memcpy(&some_float, &some_integer, sizeof(int32_t)) really did
change the type of some_float, it would break a lot of code (you could
no longer threat some_float as a float). What is guarateed is that
some_float will contain the same bit pattern of some_integer, while
still maintaining the float type. It assumes that the bit pattern of
the source is a valid pattern for the destination type, otherwise it
is UB.

>
> [And yes, I agree that this is rather grotesque.]
>

Aliasing rules really are a mess.

-- 
gpd