std::aligned_storage and [[gnu::may_alias]]

[Avi Kivity]

On 06/21/2017 02:16 AM, Jonathan Wakely wrote:
> On 20 June 2017 at 22:20, Avi Kivity wrote:
>> Does std::aligned_storage need a [[gnu::may_alias]] attribute?
>>
>>
>> According to
>> http://en.cppreference.com/w/cpp/language/reinterpret_cast#Type_aliasing, it
>> is not legal to cast a char array into another type (that is not a relative
>> of char*).
> That's not what it says. The cast to a different type is always
> allowed, but you can't access the object through the resulting pointer
> (i.e. you can't dereference it) except in the specified cases.

Right. But I do want to dereference the reinterpreted pointer.

>
>> According to the example in
>> http://en.cppreference.com/w/cpp/types/aligned_storage, it's perfectly all
>> right to cast std::aligned_storage::data to some arbitrary T; and it would
>> be hard to find a use for aligned_storage without it.
> In that example (and other typical uses of aligned_storage) you store
> an object of type T, and then access that object through a pointer of
> type T*, so there's no aliasing violation.

Suppose I have

    struct a {
         char data[4];
    };

    a x;
    auto p_int = new (x.data) int(3);
    f(p_int, &x);

Now, f() received two pointers with equal representation, but it can 
assume they are not aliased.

If f is written as:

   char f(a* a_ptr, int* i_ptr) {
     *i = 5;
     return a_ptr->data[3];
   };

Then it may reorder the two accesses, even though they alias. If a::data 
was [[gnu::may_alias]], then the compiler cannot reorder the accesses.

Here's an example: https://godbolt.org/g/8pNqWL. Removing may_alias 
causes a miscompile.

Back to the explanation in cppreference, it seems to forbid 
dereferencing p_int.


>
>> std::function uses [[gnu::may_alias]] in a situation where
>> std::aligned_storage may be used.
> It uses that char buffer for a very specific situation, where the type
> being stored is trivially-copyable, which means that it can be
> mempcy'd.
>
> IIRC we added the attribute to the std::function storage so that using
> std::swap on that buffer (which just compiles to memcpying the raw
> bytes) would work, and "swap" the dynamic types contained in the
> buffers. Without that attribute the compiler considered the swap to be
> simply copy bytes, but not change the dynamic types stored in those
> bytes. I'm not sure whether the compiler has now (GCC 8) been changed,
> so that POD structs containing arrays of unsigned char and arrays of
> std::byte get special treatment, and are implicitly "may_alias". If so
> we could change the array to unsigned char and remove the attribute.
> I'd rather not mess with it, given the subtleties of the aliasing
> rules and past bugs in this area.
>

Maybe the documentation on may_alias should be updated, because it 
doesn't mention any of this.

>> Is this a problem in libstdc++, the standard, or my understanding of the
>> strict aliasing rules?
> The two cases aren't directly comparable, because the aligned_storage
> example doesn't perform a copy of the aligned_storage data, and it
> isn't only used for trivially-copyable types. If the example was
> changed to do:
>
> static_vector<std::string, 10> v2 = v1;
> v2[0];
>
> Then it would be all kinds of undefined behaviour, because v2 would
> simply copy the bytes, but would not contain "live" objects of type T
> in the buffer. So no constructors for the T objects would have been
> run (and then to make things worse, the static_vector's destructor
> would destroy non-existent T objects).
>
> Similarly, this would be undefined:
>
> static_vector<std::string, 10> v2;
> std::swap(v1, v2);
>
> To make the copy construction valid the static_vector example would
> need to be extended with a copy constructor that used placement new to
> construct objects in the buffer, and for the swap case an assignment
> operator would also be needed.
>
> Even if the may_alias attribute was added to aligned_storage, the copy
> constructor would still be necessary when it contains
> non-trivially-copyable objects, so wouldn't really help in general.
> When using aligned_storage you need to be careful what you do with
> that POD buffer, not just copy it as a POD. For std::function we can
> get away with more, because we know only trivially-copyable types are
> stored in the buffer.
>
> Somebody should probably edit the cppreference.com example to give
> static_vector a deleted copy constructor, to prevent it being used in
> undefined ways.

I agree the implicit copy constructor is broken, but I don't see how 
operator[] is legal. It seems to give a single address two dynamic types.