[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH] Silence misguided GCC warning about alignment issues
From:       Johannes Schindelin <Johannes.Schindelin () gmx ! de>
Date:       2017-08-09 22:30:50
Message-ID: alpine.DEB.2.21.1.1708100027330.11175 () virtualbox
[Download RAW message or body]

Hi Emmanuel,

On Tue, 8 Aug 2017, Emmanuel Deloget wrote:

> On Tue, Aug 8, 2017 at 1:19 PM, Johannes Schindelin <
> Johannes.Schindelin@gmx.de> wrote:
>
> > On Mon, 7 Aug 2017, Emmanuel Deloget wrote:
> >
> > > And yes, its seems that the get_le32() macro in xz_private.h is a
> > > bit illegal with respect to strict aliasing, as it casts a uint8_t *
> > > into a const uint32_t *.
> >
> > It would seem so.
> >
> > Until you realize that it is used only on s->temp.buf (or `s->temp.buf + 4
> > * <number>`), and that the `buf` field was carefully placed after two
> > fields of type `size_t` (i.e. naturally aligned *at least* to a 32-bit
> > boundary).
> >
> > If you are worried about that magic, you can even mark the `buf` field
> > using ALIGN4. But that *still* does not fix GCC's compiler warning. What
> > fixes it is my workaround of defining a static ALWAYS_INLINE function.
> >
> >
> ​No, I'm not worried by the magic :)
> 
> Yet the problem is not the aliasing per see, it's the way the compiler
> handles it and what it means to him.

Please do not refer to the compiler as a "he". You are a "he", I assume,
and I am a "he" (I am certain about the latter). But the compiler is an
"it".

Writing "he" for the compiler makes me stumble every single time, which
means I had a much harder time understanding your otherwise excellent
explanation.

I was under the impression that the C standard explicitly forbade the
duplication/reordering you mentioned when it comes to struct fields. But
maybe I was wrong on that.

Having been educated about this, I agree that the easier fix is to simply
drop the optimization, even if it is a pity to give up on performance.

Ciao,
Johannes

_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[prev in list] [next in list] [prev in thread] [next in thread]