Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Fri, 20 Jun 2014 20:34:48 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Re: broken gcc optimization for facilitynames

On Fri, Jun 20, 2014 at 10:09:17PM +0000, Clément Vasseur wrote:
> On 2014-06-19, Rich Felker <dalias@...c.org> wrote:
> > On Wed, Jun 18, 2014 at 04:31:58PM +0000, Clément Vasseur wrote:
> >> Hello,
> >> 
> >> I might have found a gcc code generation problem with the facilitynames
> >> implementation used in musl. See the following reduced test case:
> >> 
> >> $ cat test-facilitynames.c
> >> #define SYSLOG_NAMES
> >> #include <syslog.h>
> >> int main(void)
> >> {
> >>     for (int i = 0; facilitynames[i].c_name; i++)
> >>         if (facilitynames[i].c_name)
> >>             return facilitynames[i].c_val;
> >> }
> >> 
> >> $ musl-gcc -std=gnu99 -O1 test-facilitynames.c && ./a.out; echo $?
> >> 32
> >> 
> >> $ musl-gcc -std=gnu99 -O2 test-facilitynames.c && ./a.out; echo $?
> >> 1
> >> 
> >> I see similar results with gcc versions 4.6.1, 4.8.3 and 4.9.0 (with a
> >> different return value with -O2).
> >
> > I can verify that I see this one on gcc 4.7.3. I don't see any UB in
> > the code so I'm pretty sure this is a gcc bug.
> >
> > Note that we should really fix this horrible definition of
> > facilitynames (it bloats busybox by quite a bit), but it's a nice
> > demonstration of the gcc bug which we should also report and try to
> > get fixed..
> >
> > Can you make a self-contained test case (copy the macros from musl's
> > syslog.h into the source file) and file a gcc bug report?
> 
> I made some progress towards a standalone test case. Here it is:
> 
> struct s1 { int v; };
> struct s2 { int v; };
> 
> #define a ((struct s2 *)(struct s1 []){{ 42 }})
> 
> int main(void)
> {
> 	for (int i = 0; a[i].v; i++)
> 		if (a[i].v)
> 			return a[i].v;
> }
> 
> I also found out which optimization flag causes the broken output, it's
> -fstrict-aliasing. I guess the issue here is whether casting `struct s1
> []` to `struct s2 *` violates the strict aliasing rules or not. Indeed,
> compiling with -Wstrict-aliasing=1 shows a warning at this location.

It's indeed an aliasing violation. And I don't see what the point of
it was in the first place. Whether c_name has type char * or const
char * has no bearing on whether the data can reside in .text/.rodata.

> Can someone pinpoint the exact location in the ISO C spec where there is
> an explanation about the aliasing rules with this kind of pointer
> compatibility?

C99 6.5 paragraph 7.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.