Hi Emmanuel, On Mon, 7 Aug 2017, Emmanuel Deloget wrote:
> On Mon, Aug 7, 2017 at 12:25 PM, Lauri Kasanen <[email protected]> wrote: > > On Mon, 7 Aug 2017 12:09:45 +0200 (CEST) > > Johannes Schindelin <[email protected]> wrote: > >> Meaning: GCC gets this all wrong and should not complain. So let's force > >> it to be quiet for a couple of lines. > > > > I believe this is both wrong and dangerous. If gcc warns about it, it > > will also very likely try to mis-optimize the same case. > > > > - Lauri > > I tried to carefully read the code, and I don't see where the aliasing > issue migh be. > * buf is an uin8_t [] > * the code either use it as is, or promote it to const uint8_t *, or > cast it to (const) char *. > > All of these types are compatible w.r.t. aliasing (and the C99 > standard). I may have missed something though and my eyes might be > rusted. > > Now, it seems that some might have seen a regression in GCC with > respect to aliasing rules [1][2]. This might be an instance of the bug > (according to the bug report, it's no longer reproducible in GCC > trunk). I dug a little deeper, and it seems this issue may indeed have been reported as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80593 (and been fixed in the meantime). However... > Anyway, it might be a good idea to avoid unsetting the warning for one > buggy GCC version :) ... I get this warning *also* with the default GCC version of Ubuntu 16.04.3 LTS, gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609. So it is not just one buggy GCC version. I do agree, however, that it is a super-ugly patch, even if it does the job for the described purpose. So I set out to find a more elegant solution. More elegant, even, than removing the get_le32() macro as Denys did in 76b65624b (unxz: get_le32 macro is obviously wrong, 2017-08-07): the macro was there for a reason, namely to avoid accessing the (purposefully aligned) data using get_unaligned_le32() which is slow and unlikely to get optimized. It turns out that GCC simply has a hard time with the cast, without assigning the pointer explicitly to a variable. Turning get_le32() into an inline function lets GCC figure out that everything is groovy, and it also adds a bit more of concrete compile-time safety. I will send out v2 in a moment, assuming that Denys will agree that direct, provably aligned 32-bit access is better than an unnecessary assembly-from-individual-bytes. Ciao, Johannes _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
