[bug] gcc-4.[23] miscompiling causing networking to bugger up

[bug] gcc-4.[23] miscompiling causing networking to bugger up

[Kyle McMartin]

So I've narrowed the regression down to a miscompilation (presumably) of
net/ipv4/ip_output.o, in the sense that

	cp linux-2.6-gcc34/net/ipv4/ip_output.o
		linux-2.6-gcc43/net/ipv4/ip_output.o
	touch linux-2.6-gcc43/net/ipv4/ip_output.o
	make vmlinux

fixes the issue.

Unfortunately, the file is basically entirely different when
objdump'd...

The generated assembly is not much more help either.

I'm going to try again with gcc 4.1 & 4.2, and hope they're similar
enough that they generate closer assembly fragments...

but, getting closer to a fix, I hope.

Re: [bug] gcc-4.[23] miscompiling causing networking to bugger up

[Kyle McMartin]

On Wed, May 28, 2008 at 03:09:46PM -0400, Kyle McMartin wrote:
> So I've narrowed the regression down to a miscompilation (presumably) of
> net/ipv4/ip_output.o, in the sense that
> 
> 	cp linux-2.6-gcc34/net/ipv4/ip_output.o
> 		linux-2.6-gcc43/net/ipv4/ip_output.o
> 	touch linux-2.6-gcc43/net/ipv4/ip_output.o
> 	make vmlinux
> 
> fixes the issue.
> 

Apparently I was on glue and gcc-4.2 is not problematic.

Re: [bug] gcc-4.[23] miscompiling causing networking to bugger up

[Carlos O'Donell]

On Wed, May 28, 2008 at 4:00 PM, Kyle McMartin <kyle@mcmartin.ca> wrote:
> On Wed, May 28, 2008 at 03:09:46PM -0400, Kyle McMartin wrote:
>> So I've narrowed the regression down to a miscompilation (presumably) of
>> net/ipv4/ip_output.o, in the sense that
>>
>>       cp linux-2.6-gcc34/net/ipv4/ip_output.o
>>               linux-2.6-gcc43/net/ipv4/ip_output.o
>>       touch linux-2.6-gcc43/net/ipv4/ip_output.o
>>       make vmlinux
>>
>> fixes the issue.
>>
>
> Apparently I was on glue and gcc-4.2 is not problematic.

If you compile ip_output.c with -O0 does it work with gcc-4.3?

Cheers,
Carlos.

Re: [bug] gcc-4.[23] miscompiling causing networking to bugger up

[Kyle McMartin]

On Wed, May 28, 2008 at 06:55:29PM -0400, Carlos O'Donell wrote:
> On Wed, May 28, 2008 at 4:00 PM, Kyle McMartin <kyle@mcmartin.ca> wrote:
> > On Wed, May 28, 2008 at 03:09:46PM -0400, Kyle McMartin wrote:
> >> So I've narrowed the regression down to a miscompilation (presumably) of
> >> net/ipv4/ip_output.o, in the sense that
> >>
> >>       cp linux-2.6-gcc34/net/ipv4/ip_output.o
> >>               linux-2.6-gcc43/net/ipv4/ip_output.o
> >>       touch linux-2.6-gcc43/net/ipv4/ip_output.o
> >>       make vmlinux
> >>
> >> fixes the issue.
> >>
> >
> > Apparently I was on glue and gcc-4.2 is not problematic.
> 
> If you compile ip_output.c with -O0 does it work with gcc-4.3?
> 

Building the kernel without optimization is very tetchy... if you can
guess what passes to remove, I can try... one of the optimization
effects that must go is DCE though, otherwise we'll have undefined
references to things (ie: kmalloc_too_much in a switch that should have
been eliminated except the constant case.)

r, Kyle

Re: [bug] gcc-4.[23] miscompiling causing networking to bugger up

[Carlos O'Donell]

[-- Attachment #1: Type: text/plain, Size: 815 bytes --]

On Wed, May 28, 2008 at 7:00 PM, Kyle McMartin <kyle@mcmartin.ca> wrote:
> Building the kernel without optimization is very tetchy... if you can
> guess what passes to remove, I can try... one of the optimization
> effects that must go is DCE though, otherwise we'll have undefined
> references to things (ie: kmalloc_too_much in a switch that should have
> been eliminated except the constant case.)

You *need* to build the kernel with -fno-dce and -fno-tree-dce?
Are you working around another compiler bug?

Take a look at the attached scripts I use.

./o2list.sh > o2list.txt
./permute-options.sh o2list.txt > run.sh
chmod u+x
run.sh

Normally I can simply run the results on a target and
know which is good and which is bad. Unfortunately
you have to link each into a kernel and try to boot.

Cheers,
Carlos.

[-- Attachment #2: o2list.sh --]
[-- Type: application/x-sh, Size: 803 bytes --]

[-- Attachment #3: permute-build.sh --]
[-- Type: application/x-sh, Size: 1539 bytes --]

Re: [bug] gcc-4.[23] miscompiling causing networking to bugger up

[Kyle McMartin]

On Wed, May 28, 2008 at 07:37:14PM -0400, Carlos O'Donell wrote:
> On Wed, May 28, 2008 at 7:00 PM, Kyle McMartin <kyle@mcmartin.ca> wrote:
> > Building the kernel without optimization is very tetchy... if you can
> > guess what passes to remove, I can try... one of the optimization
> > effects that must go is DCE though, otherwise we'll have undefined
> > references to things (ie: kmalloc_too_much in a switch that should have
> > been eliminated except the constant case.)
> 
> You *need* to build the kernel with -fno-dce and -fno-tree-dce?
> Are you working around another compiler bug?
> 
> Take a look at the attached scripts I use.
> 
> ./o2list.sh > o2list.txt
> ./permute-options.sh o2list.txt > run.sh
> chmod u+x
> run.sh
> 
> Normally I can simply run the results on a target and
> know which is good and which is bad. Unfortunately
> you have to link each into a kernel and try to boot.
> 

Other way around... we *need* dce (but apparnetly not on this file)
because of inline constructs like

	switch (sizeof(some constant)) {
	1:
		blah;
	default:
		some_undefined_reference();
	}

Ah well, anyway, it works built at -O0, and buggers up at -O1.

Want a copy of each of the .S?

Re: [bug] gcc-4.[23] miscompiling causing networking to bugger up

[John David Anglin]

> Ah well, anyway, it works built at -O0, and buggers up at -O1.
> 
> Want a copy of each of the .S?

I think it would be useful to know if this is still present in 4.3 branch
head.  Unless we know which routine the problem is in, comparing .s files
is tough.

I'm busy until just before the summit.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: [bug] gcc-4.[23] miscompiling causing networking to bugger up

[Kyle McMartin]

On Wed, May 28, 2008 at 08:00:58PM -0400, John David Anglin wrote:
> > Ah well, anyway, it works built at -O0, and buggers up at -O1.
> > 
> > Want a copy of each of the .S?
> 
> I think it would be useful to know if this is still present in 4.3 branch
> head.  Unless we know which routine the problem is in, comparing .s files
> is tough.
> 
> I'm busy until just before the summit.
> 

I tried 4.4 from svn a few days (more like 2 weeks maybe) ago. Is that
recent enough or should I try 4.3-head as well (no problem to do.)

r, Kyle

Re: [bug] gcc-4.[23] miscompiling causing networking to bugger up

[Carlos O'Donell]

On Wed, May 28, 2008 at 8:02 PM, Kyle McMartin <kyle@mcmartin.ca> wrote:
> I tried 4.4 from svn a few days (more like 2 weeks maybe) ago. Is that
> recent enough or should I try 4.3-head as well (no problem to do.)

Yes, please try 4.3-head.

Cheers,
Carlos.

Re: [bug] gcc-4.[23] miscompiling causing networking to bugger up

[Kyle McMartin]

On Wed, May 28, 2008 at 08:07:00PM -0400, Carlos O'Donell wrote:
> On Wed, May 28, 2008 at 8:02 PM, Kyle McMartin <kyle@mcmartin.ca> wrote:
> > I tried 4.4 from svn a few days (more like 2 weeks maybe) ago. Is that
> > recent enough or should I try 4.3-head as well (no problem to do.)
> 
> Yes, please try 4.3-head.
> 

Ok, the Debian version is pretty recent though.

kyle@shortfin:~/linux-2.6-gcc42$ hppa64-linux-gnu-gcc-4.3 -v
Using built-in specs.
Target: hppa64-linux-gnu
Configured with: ../src/configure --enable-languages=c --prefix=/usr
--libexecdir=/usr/lib --disable-shared --disable-nls --disable-threads
--disable-libgomp --disable-libmudflap --disable-libssp
--with-as=/usr/bin/hppa64-linux-gnu-as
--with-ld=/usr/bin/hppa64-linux-gnu-ld
--includedir=/usr/hppa64-linux-gnu/include --host=hppa-linux-gnu
--build=hppa-linux-gnu --target=hppa64-linux-gnu
Thread model: single
gcc version 4.3.1 20080523 (prerelease) (GCC) 
kyle@shortfin:~/linux-2.6-gcc42$ 

I'll build my xcompiler using 4.3-HEAD instead of HEAD and we'll see.
(the xcompiler fails the same way, so I'm confident it will be fine
building on my i386... all builds in this thread discussed so far have
been natively built.)

r, kyle

Re: [bug] gcc-4.3 miscompiling causing networking to bugger up

[Kyle McMartin]

On Wed, May 28, 2008 at 08:07:00PM -0400, Carlos O'Donell wrote:
> On Wed, May 28, 2008 at 8:02 PM, Kyle McMartin <kyle@mcmartin.ca> wrote:
> > I tried 4.4 from svn a few days (more like 2 weeks maybe) ago. Is that
> > recent enough or should I try 4.3-head as well (no problem to do.)
> 
> Yes, please try 4.3-head.
> 

I've found the solution to the bug, it seems something is going wonky
because of ip_fast_csum missing a "memory" clobber.

This should fix it, but I have no idea whether this is a legitimate bug,
or masking a problem with gcc due to too much inlining.

diff --git a/include/asm-parisc/checksum.h b/include/asm-parisc/checksum.h
index cc3ec1b..1916ebe 100644
--- a/include/asm-parisc/checksum.h
+++ b/include/asm-parisc/checksum.h
@@ -65,7 +65,7 @@ static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
 "2:\n"
 	: "=r" (sum), "=r" (iph), "=r" (ihl)
 	: "1" (iph), "2" (ihl)
-	: "r19", "r20", "r21" );
+	: "r19", "r20", "r21", "memory" );
 
 	return (__force __sum16)sum;
 }

Re: [bug] gcc-4.3 miscompiling causing networking to bugger up

[Matthew Wilcox]

On Sat, May 31, 2008 at 12:10:10AM -0400, Kyle McMartin wrote:
> I've found the solution to the bug, it seems something is going wonky
> because of ip_fast_csum missing a "memory" clobber.
> 
> This should fix it, but I have no idea whether this is a legitimate bug,
> or masking a problem with gcc due to too much inlining.

*veeery* interesting.  It's entirely possible that this is the correct
fix:

 If your assembler instructions access memory in an unpredictable
 fashion, add `memory' to the list of clobbered registers.  This will
 cause GCC to not keep memory values cached in registers across the
 assembler instruction and not optimize stores or loads to that memory.

So if GCC has something in a register, it might not bother to write it
back to ram before this asm if we don't have the memory clobber.

Fantastic work, Kyle.  Thanks for spending so much time on this.

> diff --git a/include/asm-parisc/checksum.h b/include/asm-parisc/checksum.h
> index cc3ec1b..1916ebe 100644
> --- a/include/asm-parisc/checksum.h
> +++ b/include/asm-parisc/checksum.h
> @@ -65,7 +65,7 @@ static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
>  "2:\n"
>  	: "=r" (sum), "=r" (iph), "=r" (ihl)
>  	: "1" (iph), "2" (ihl)
> -	: "r19", "r20", "r21" );
> +	: "r19", "r20", "r21", "memory" );
>  
>  	return (__force __sum16)sum;
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [bug] gcc-4.3 miscompiling causing networking to bugger up

[Grant Grundler]

On Sat, May 31, 2008 at 04:58:58AM -0600, Matthew Wilcox wrote:
...
> So if GCC has something in a register, it might not bother to write it
> back to ram before this asm if we don't have the memory clobber.

Willy, thanks for explaining...I had to reread this a few times to
"get it". This would also explain why we saw it on ip_output where
the ipv4 header was being written.

> Fantastic work, Kyle.  Thanks for spending so much time on this.

Agreed. I owe you a bar of chocolate.

> > diff --git a/include/asm-parisc/checksum.h b/include/asm-parisc/checksum.h
> > index cc3ec1b..1916ebe 100644
> > --- a/include/asm-parisc/checksum.h
> > +++ b/include/asm-parisc/checksum.h
> > @@ -65,7 +65,7 @@ static inline __sum16 ip_fast_csum(const void *iph, unsigned int ihl)
> >  "2:\n"
> >  	: "=r" (sum), "=r" (iph), "=r" (ihl)
> >  	: "1" (iph), "2" (ihl)
> > -	: "r19", "r20", "r21" );
> > +	: "r19", "r20", "r21", "memory" );
> >  
> >  	return (__force __sum16)sum;
> >  }

Does csum_ipv6_magic() also need the same treatment?

Kyle suspects csum_ipv6_magic() might be ok because it's passing
in a "struct ipv6_addr *".  If that's correct, maybe we should ask
davem/et al to change ip_fast_csum() to take "struct iphdr *" as well?

thanks,
grant

Re: [bug] gcc-4.3 miscompiling causing networking to bugger up

[Carlos O'Donell]

On Sat, May 31, 2008 at 12:10 AM, Kyle McMartin <kyle@mcmartin.ca> wrote:
> I've found the solution to the bug, it seems something is going wonky
> because of ip_fast_csum missing a "memory" clobber.

Great news!

The asm does advance the iph pointer, and reads the header data
without telling gcc. The input/output operands only state the pointer
may be read and modified not the memory at the pointer.

The compiler may have cached iph writes in registers and they may not
be in memory yet. Under normal circumstances I would say "Add a memory
input operand for the header data e.g. "m" (*header_struct_pointer)",
but the relationship with ihl and iph is not known the compiler. In
this case it is probably correct to use the "memory" clobber as a
catch all.

Cheers,
Carlos.

Re: [bug] gcc-4.[23] miscompiling causing networking to bugger up

[John David Anglin]

> I tried 4.4 from svn a few days (more like 2 weeks maybe) ago. Is that
> recent enough or should I try 4.3-head as well (no problem to do.)

4.4 has new bugs.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)