Liu Hao via llvm-dev
2018-Aug-10 04:30 UTC
[llvm-dev] GCC 5 and -Wstrict-aliasing in JSON.h
在 2018-08-10 06:20, Kim Gräsman 写道:> On Fri, Aug 10, 2018 at 12:02 AM, Jonathan Wakely <jwakely.gcc at gmail.com> > wrote: >> >> If GCC 4.9.3 thinks there's an aliasing violation it might >> misoptimise. It doesn't matter if it's right or not, it matters if it >> treats the code as undefined or not. >> >> And apparently GCC does think there's a violation, because it warns. >> >> Unless you're sure that not only is the code OK, but GCC is just being >> noisy and doesn't misoptimise, then I think using -fno-strict-aliasing >> is safer than just suppressing the warning. > > Good point, I can see how that would play out nicer. > > So this would probably need to be addressed in the LLVM build system, I'll > try and work up a patch tomorrow. >When I used to do such type punning in C, I got similar warnings. Then I looked for some solutions... I can't recall the principle now and I fail to find it in the C or C++ standard. Despite that, the solution is simple: Only an lvalue of a pointer to (possibly CV-qualified) `void` or a pointer to a character type (in C) / any of `char`, `unsigned char` or `std::byte` (in C++) can alias objects. That is to say, in order to eliminate the aliasing problem an intermediate lvalue pointer is required. Hence, altering ``` return *reinterpret_cast<T *>(Union.buffer); ``` to ``` auto p = static_cast<void *>(Union.buffer); return *static_cast<T *>(p); ``` will probably resolve this problem.> Thanks, > - Kim >-- Best regards, LH_Mouse
Kim Gräsman via llvm-dev
2018-Aug-10 10:53 UTC
[llvm-dev] GCC 5 and -Wstrict-aliasing in JSON.h
Hi LH_Mouse, Thanks! On Fri, Aug 10, 2018 at 6:30 AM, Liu Hao <lh_mouse at 126.com> wrote:> > When I used to do such type punning in C, I got similar warnings. Then I > looked for some solutions... I can't recall the principle now and I fail to > find it in the C or C++ standard. Despite that, the solution is simple: > > Only an lvalue of a pointer to (possibly CV-qualified) `void` or a pointer > to a character type (in C) / any of `char`, `unsigned char` or `std::byte` > (in C++) can alias objects. > > That is to say, in order to eliminate the aliasing problem an intermediate > lvalue pointer is required. > > Hence, altering > ``` > return *reinterpret_cast<T *>(Union.buffer); > ``` > to > ``` > auto p = static_cast<void *>(Union.buffer); > return *static_cast<T *>(p); > ``` > will probably resolve this problem.I'm worried that this might only serve to trick the compiler. Explicitly using `-fno-strict-aliasing` for GCC < 6 would seem more direct to me -- as Jonathan says, if the compiler classifies a strict aliasing rule violation as undefined behavior, and that is further used to optimize in an unexpected manner, it doesn't matter whether it warns or not. Then again, I guess disabling strict aliasing would also disable optimizations that are generally useful for LLVM as a whole. I'm reading up on safe aliasing techniques, but so far nothing stands out to me as applicable in this scenario. - Kim
Liu Hao via llvm-dev
2018-Aug-10 12:55 UTC
[llvm-dev] GCC 5 and -Wstrict-aliasing in JSON.h
在 2018-08-10 18:53, Kim Gräsman 写道:> I'm worried that this might only serve to trick the compiler. >It shouldn't. If it was merely a trick then `std::aligned_storage` would be completely unusable.> Explicitly using `-fno-strict-aliasing` for GCC < 6 would seem more > direct to me -- as Jonathan says, if the compiler classifies a strict > aliasing rule violation as undefined behavior, and that is further > used to optimize in an unexpected manner, it doesn't matter whether it > warns or not. Then again, I guess disabling strict aliasing would also > disable optimizations that are generally useful for LLVM as a whole. > > I'm reading up on safe aliasing techniques, but so far nothing stands > out to me as applicable in this scenario. >The C++ standard requires creation of objects in such ways to use new expressions (a.k.a. placement new). Athough [intro.object]-3 only defines /provides storage/ for arrays of a character type or `std::byte`, the specification of `aligned_storage` and `aligned_union` in [meta.trans.other] doesn't require the type used as uninitialized storage to be an array of a character type or `std::byte` - in fact it cannot be, because alignment information is not part of the nominal type system of C and will be lost when obtaining the `type` member. Focusing on the cast: As long as the compiler is unable to know whether a placement new has been made on the union (i.e. whether it is providing storage for another object), I don't think a standard-conforming compiler is ever allowed to ignore such possibility.> - Kim >-- Best regards, LH_Mouse
Andrew Haley via llvm-dev
2018-Aug-10 15:52 UTC
[llvm-dev] GCC 5 and -Wstrict-aliasing in JSON.h
On 08/10/2018 05:30 AM, Liu Hao wrote:> Only an lvalue of a pointer to (possibly CV-qualified) `void` or a > pointer to a character type (in C) / any of `char`, `unsigned char` or > `std::byte` (in C++) can alias objects.Yes.> That is to say, in order to eliminate the aliasing problem an > intermediate lvalue pointer is required.Not exactly. You can cast a pointer to a pointer to some character type or the type of the object stored in memory. It does not matter whether you use an intermediate type or not. Having not seen the test case, I can't tell whether this rule is followed. What you can't do is store as a double, cast the double pointer to a character pointer, cast that pointer to some other pointer type, and read from memory. GCC won't give you a warning for that, but it's still undefined. JSON.h seems to hope that if you cast a pointer to T to a pointer to some union type, magic will happen. It won't work, unless the object stored in the memory at that address was stored as the union type. Do not lie to the compiler or it will get its revenge. -- Andrew Haley Java Platform Lead Engineer Red Hat UK Ltd. <https://www.redhat.com> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
Sam McCall via llvm-dev
2018-Aug-10 16:08 UTC
[llvm-dev] GCC 5 and -Wstrict-aliasing in JSON.h
json::Value in JSON.h is a discriminated union. The storage is a char array of appropriate type and alignment. The storage holds one object at a time, it's initialized (and for nontrivial types, destroyed) at the right times to ensure this. The cast is only to the type of object that's already there, there's no magic here. On Fri, Aug 10, 2018, 17:52 Andrew Haley <aph at redhat.com> wrote:> On 08/10/2018 05:30 AM, Liu Hao wrote: > > Only an lvalue of a pointer to (possibly CV-qualified) `void` or a > > pointer to a character type (in C) / any of `char`, `unsigned char` or > > `std::byte` (in C++) can alias objects. > > Yes. > > > That is to say, in order to eliminate the aliasing problem an > > intermediate lvalue pointer is required. > > Not exactly. You can cast a pointer to a pointer to some character > type or the type of the object stored in memory. It does not matter > whether you use an intermediate type or not. Having not seen the test > case, I can't tell whether this rule is followed. > > What you can't do is store as a double, cast the double pointer to a > character pointer, cast that pointer to some other pointer type, and > read from memory. GCC won't give you a warning for that, but it's > still undefined. > > JSON.h seems to hope that if you cast a pointer to T to a pointer to > some union type, magic will happen. It won't work, unless the object > stored in the memory at that address was stored as the union type. > > Do not lie to the compiler or it will get its revenge. > > -- > Andrew Haley > Java Platform Lead Engineer > Red Hat UK Ltd. <https://www.redhat.com> > EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671 >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180810/ba826ae9/attachment.html>