[PATCH 2] fix broken strict aliasing

Marc Lehmann schmorp at schmorp.de
Mon Feb 22 20:45:19 CET 2010


On Mon, Feb 22, 2010 at 12:59:08PM -0600, Brandon Black <blblack at gmail.com> wrote:
> On Sun, Feb 21, 2010 at 3:39 AM, Marc Lehmann <schmorp at schmorp.de> wrote:
> > On Sat, Feb 20, 2010 at 09:41:35AM -0600, Brandon Black <blblack at gmail.com> wrote:
> >> I've seen this strict-aliasing issues with libev + gcc 4.4 as well.  I
> > Facts please...
> 
> I wasn't asserting that there was an aliasing issue in libev.  I was

Well, you asserted a "strict-aliasing issue", which to me sounds a lot
like an aliasing issue...

> asserting that there is a strict aliasing warning with gcc + libev,

That's not what you asserted, but maybe what you meant. You even quoted
it...

But let's not quarrel about words - you mistyped something and didn't even
notice it on quoting it, that's ok.

> The real issue here is that I apparently don't understand, precisely
> and in-depth, exactly what the C standard says about aliasing, and
> exactly what gcc says about the same, and whether they agree or not.

The warning is correct as worded. It's like saying "hitting a wall at
120km/h is deadly for humans". But as the code isn't doing what it is
warning about, all is fine.

Again, and we had that many times in the past, warnings are just that: the
compiler warns you that doing something might result in something.

Most compilers have lots of warnings, many of them bogus in many cases, many
of them useful. Many not enabled unless you explicitly ask for them.

It's ok to enable some for yourself, but a) not understanding a warning
and b) then trying to enforce weird coding rules on other people's code,
based on those warnings, strikes me as rather unconstructive. Do you not
agree?

Take, for example, -Wformat-y2k, -Wformat-nonliteral, -Wparentheses, -Wseq
uence-point, to pick some random examples. They all flag completely valid
code and most of those cannot even be avoided.

It is ok to enable those for your code. It's not ok to force your
pseudo-language standard on libev.

We had this discussion in the past, and I am really getting annoyed at the
continued attempts of forcing "but my compiler warning is bogus and needs
this workaround" stuff.

This has nothing to do with quality, it's just mindless "I am too stupid
to understand compiler warnings but I want to force you to change your
code, even at the expense of spreading FUD about your code".

I just can't listen to it anymore, so go to the list archives and find
something new to talk about.

> Apparently I'm not the only one, as evidenced not only by this thread,
> but by Google searches turning up many similar threads on many other

Searching for that exact warning and "bogus" will - guess what - get you
many, many hits as well. Your point being...?

Of course, this is just spreading more doubt.

Fact is, you enable a warning you don't understand (your claim) and want
me to change my code (I claim to understand aliasing issues).

Is this convincing? Should I _really_ make a change based on that
premise? Do you _really_ want this kind of development model for libev?

Sure, other library authors might haxor their software to please a few
wannabe-dictators that want to enable every compiler warning their
compiler got, often causing elaborate workarounds around compiler
problems.

Neither gcc nor libev are buggy here, if you don't like the compiler
warning, if you don't even understand it, switch it off! It is of zero
help to you!

I like to repeat that I try to produce quality software. This means not
applying each and every patch that hacks around to avoid an issue with
some misconigured OS, a problematic compiler, or the warning-set-du-jour.
It is sometimes hard, sure, but I haven't found a good way to simultaneously
say "the patch is buggy" and "but you are encouraged to contribute" - most
people send in low quality patches to fix their problem, and expect me to do
all the quality work. Sometimes I do, especially if there is a real issue.

Sometimes I don't, especially if it's just some bogus compiler warning
that isn't enabled by default even.

Blindly applying such patches doesn't lead to high quality software
(personal experience, you might disagree), only to a big unmaintainable
mess.

To your code example:

> blblack at xpc:~$ gcc -std=c99 -O3 -Wall -c test.c
> test.c: In function ‘get_a’:
> test.c:13: warning: dereferencing type-punned pointer will break
> strict-aliasing rules

The warning is bogus UNLESS you actually do what the warning says.

But the example doesn't, so the warning is bogus.

> The code above is a simplified version of a line at the beginning of
> evpipe_init() which generates the same warning.  The original line is
> this:

Except that it leaves out some essential ingredients, such as the union
in ev.h, but that doesn't change anything in this simplified example. Of
course, the union exists just for standard C correctness, no compiler
I know actually relies on this, but since this discussion is about
standards, sorry, you left out essentials.

Unfortunately, your example is so simplified it is completely useless
to discuss the issue at hand, because it cannot have an aliasing issue,
because nothing is there to alias with anything else - it doesn't even
execute any code, which is crucial for aliasing to occur!

> In the definition of "struct ev_loop", pipe_w is a member of type
> "struct ev_io" (not a pointer, a direct sub-struct).  ev_io and
> ev_watcher have a common first member, "int active".

(The member has the same offset and layout in all structs, of course.)

Now, there is some disagreement how to apply the aliasing rules to
this case (whether the struct type is included in the type or only the
underlying type (int) counts), but the libev code, afaik, is correct under
both interpretations, as the "active" member is only accessed in the
ev_watcher structure, which is an exact subset of the ev_io structure.

This is what the casts accomplish - without them, perversely enough,
you will NOT get the warnings, and your code will be miscompiled with
sufficiently high optimisations.

But hey, it's just a warning with a known-high bogus ratio...

> Can someone who understands the relevant standards and whatever modern
> gcc is doing with aliasing explain what the standards say about the
> test case snippet above, and why gcc is (possibly erroneously?)

Nothing is errornous except your insistence that a gcc warning indicates an
issue in the code:

- This ain't so. That's (one reason) why they are warnings.

- Warnings, for the umpteemth time, are just that - warnings.

- This specific warning is not strictly wrong, it just doesn't apply to
  the code. Note that to makes no claims about the code in question, either.
  It is just bogus.

- This is a quality of implementation issue in gcc, nothing more, nothing
  less. I understand what gcc does there, and why it fails, and why it
  can be a useful warning, and why it is not useful here.

- It's not included in the default warning set for a reason (namely it
  creates a lot of bogus false positives... go figure).

Executive summary: if you don't understand a compiler warning, don't
enable it. If you enable it anyways, make sure you understand it BEFORE
torturing code until it is silent - you might actually break things.

Case in point, I get a patch to silence a specific warning in
rxvt-unicode, not enabled by default, once per year that silences the
warning - and breaks the code.

Hard rule #1: YOU NEED TO UNDERSTAND WHAT YOU ARE DOING. Also when
programming. Sorry, using magic tools will not magically make you a better
programmer.

Can we settle this please, or _finally_ bring some evidence?

I spent hours on this bogus thread this time. This is time lost for
other projects. You are one such time killer. If you had researched your
subject, we might have had an intellectual discussion. As it is, it's just
rehashing of the same old arguments (and non-arguments...).

-- 
                The choice of a       Deliantra, the free code+content MORPG
      -----==-     _GNU_              http://www.deliantra.net
      ----==-- _       generation
      ---==---(_)__  __ ____  __      Marc Lehmann
      --==---/ / _ \/ // /\ \/ /      schmorp at schmorp.de
      -=====/_/_//_/\_,_/ /_/\_\



More information about the libev mailing list