On 22 June 2017 at 19:06, Avi Kivity <avi@xxxxxxxxxxxx> wrote: > > > 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. If you do that and there isn't an object of that dynamic type at that location, it's undefined. >>> 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]; > }; If you're using the char[4] array as untyped storage to hold an object of type int, why are you also accessing it as char? Make your mind up. You can do this sort of thing using a union (in compilers that support type-punning as a non-standard extension) but you can't do it like this with non-unions. After the placement new-expression there is an object of type int at that location. *i = 5 is OK. If you then try to access that memory location as type char then you are reading uninitialized bytes, because you didn't initialize any char object at that location. Since reading the value of an indeterminate char is undefined, the compiler can assume you're not doing that, and that a_ptr->data[3] is an initialized char object. > 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. Without the attribute it's undefined behaviour. > Back to the explanation in cppreference, it seems to forbid dereferencing > p_int. Either it doesn't say that, or it's wrong. The placement new-expression begins the lifetime of an object of type int at that location, and you can access that object through 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. It's not the expected use of may_alias. It was added to std::function's storage to solve a problem without needing an ABI change. It happens to work, and we can rely on undocumented GCC-specific behaviour in libstdc++, just because we can. >>> 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. No it doesn't. There's only one dynamic type at a time.
