Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

[Erik Hensema]

I've been looking into the compile problems of net/ipv6/ndisc.c in
2.5.1-pre2 and I found that the asm generated by gcc (2.95.3) is wrong:

This is a small part of a diff betweem two asm files generated by gcc, note
the missing \n's in the wrong code:

-	
-		addl 0(%ebp), %edx
-		adcl 4(%ebp), %edx
-		adcl 8(%ebp), %edx
-		adcl 12(%ebp), %edx
-		adcl 0(%ecx), %edx
-		adcl 4(%ecx), %edx
-		adcl 8(%ecx), %edx
-		adcl 12(%ecx), %edx
-		adcl %edi, %edx
-		adcl %eax, %edx
-		adcl $0, %edx
-		
+	addl 0(%ebp), %edxadcl 4(%ebp), %edxadcl 8(%ebp), %edxadcl 12(%ebp), %edxadcl 0(%ecx), %edxadcl 4(%ecx), %edxadcl 8(%ecx), %edxadcl 12(%ecx), %edxadcl %edi, %edxadcl %eax, %edxadcl $0, %edx

I'll be looking into this further tonight.

-- 
Erik Hensema (erik@hensema.net)
I'm on the list, no need to Cc: me, though I appreciate one if your
a
mailer doesn't support the References header.

Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

[Richard B. Johnson]

On 28 Nov 2001, Erik Hensema wrote:

> 
> I've been looking into the compile problems of net/ipv6/ndisc.c in
> 2.5.1-pre2 and I found that the asm generated by gcc (2.95.3) is wrong:
> 
> This is a small part of a diff betweem two asm files generated by gcc, note
> the missing \n's in the wrong code:
> 
> -	
> -		addl 0(%ebp), %edx
> -		adcl 4(%ebp), %edx
> -		adcl 8(%ebp), %edx
> -		adcl 12(%ebp), %edx
> -		adcl 0(%ecx), %edx
> -		adcl 4(%ecx), %edx
> -		adcl 8(%ecx), %edx
> -		adcl 12(%ecx), %edx
> -		adcl %edi, %edx
> -		adcl %eax, %edx
> -		adcl $0, %edx
> -		
> +	addl 0(%ebp), %edxadcl 4(%ebp), %edxadcl 8(%ebp), %edxadcl 12(%ebp), %edxadcl 0(%ecx), %edxadcl 4(%ecx), %edxadcl 8(%ecx), %edxadcl 12(%ecx), %edxadcl %edi, %edxadcl %eax, %edxadcl $0, %edx


This is probably just some loop unrolling, not some as you say "wrong
code".


Cheers,
Dick Johnson

Penguin : Linux version 2.4.1 on an i686 machine (799.53 BogoMips).

    I was going to compile a list of innovations that could be
    attributed to Microsoft. Once I realized that Ctrl-Alt-Del
    was handled in the BIOS, I found that there aren't any.

Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

[David Relson]

At 02:09 PM 11/28/01, you wrote:
>On 28 Nov 2001, Erik Hensema wrote:
>
> >
> > I've been looking into the compile problems of net/ipv6/ndisc.c in
> > 2.5.1-pre2 and I found that the asm generated by gcc (2.95.3) is wrong:
> >
> > This is a small part of a diff betweem two asm files generated by gcc, note
> > the missing \n's in the wrong code:
> >
> > -
> > -             addl 0(%ebp), %edx
> > -             adcl 4(%ebp), %edx
> > -             adcl 8(%ebp), %edx
> > -             adcl 12(%ebp), %edx
> > -             adcl 0(%ecx), %edx
> > -             adcl 4(%ecx), %edx
> > -             adcl 8(%ecx), %edx
> > -             adcl 12(%ecx), %edx
> > -             adcl %edi, %edx
> > -             adcl %eax, %edx
> > -             adcl $0, %edx
> > -
> > +     addl 0(%ebp), %edxadcl 4(%ebp), %edxadcl 8(%ebp), %edxadcl 
> 12(%ebp), %edxadcl 0(%ecx), %edxadcl 4(%ecx), %edxadcl 8(%ecx), %edxadcl 
> 12(%ecx), %edxadcl %edi, %edxadcl %eax, %edxadcl $0, %edx
>
>
>This is probably just some loop unrolling, not some as you say "wrong
>code".

The lack of \n's is wrong.  Adding \n's and some whitespace, gives 
identical code.  Looks like a defective printf() in gcc.

David

Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

[Martin Eriksson]

----- Original Message -----
From: "Richard B. Johnson" <root@chaos.analogic.com>
To: <erik@hensema.net>
Cc: <linux-kernel@vger.kernel.org>
Sent: Wednesday, November 28, 2001 8:09 PM
Subject: Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?


> On 28 Nov 2001, Erik Hensema wrote:
>
> >
> > I've been looking into the compile problems of net/ipv6/ndisc.c in
> > 2.5.1-pre2 and I found that the asm generated by gcc (2.95.3) is wrong:
> >
> > This is a small part of a diff betweem two asm files generated by gcc,
note
> > the missing \n's in the wrong code:
> >
> > -
> > - addl 0(%ebp), %edx
> > - adcl 4(%ebp), %edx
> > - adcl 8(%ebp), %edx
> > - adcl 12(%ebp), %edx
> > - adcl 0(%ecx), %edx
> > - adcl 4(%ecx), %edx
> > - adcl 8(%ecx), %edx
> > - adcl 12(%ecx), %edx
> > - adcl %edi, %edx
> > - adcl %eax, %edx
> > - adcl $0, %edx
> > -
> > + addl 0(%ebp), %edxadcl 4(%ebp), %edxadcl 8(%ebp), %edxadcl 12(%ebp),
%edxadcl 0(%ecx), %edxadcl 4(%ecx), %edxadcl 8(%ecx), %edxadcl 12(%ecx),
%edxadcl %edi, %edxadcl %eax, %edxadcl $0, %edx
>
>
> This is probably just some loop unrolling, not some as you say "wrong
> code".

Correct me if I'm wrong, but I don't think "%edxadcl" really assembles....

_____________________________________________________
|  Martin Eriksson <nitrax@giron.wox.org>
|  MSc CSE student, department of Computing Science
|  Umeå University, Sweden

Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

[Richard B. Johnson]

On Wed, 28 Nov 2001, Martin Eriksson wrote:

> ----- Original Message -----
> From: "Richard B. Johnson" <root@chaos.analogic.com>
> To: <erik@hensema.net>
> Cc: <linux-kernel@vger.kernel.org>
> Sent: Wednesday, November 28, 2001 8:09 PM
> Subject: Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?
> 
> 
> > On 28 Nov 2001, Erik Hensema wrote:
> >
> > >
> > > I've been looking into the compile problems of net/ipv6/ndisc.c in
> > > 2.5.1-pre2 and I found that the asm generated by gcc (2.95.3) is wrong:
> > >
> > > This is a small part of a diff betweem two asm files generated by gcc,
> note
> > > the missing \n's in the wrong code:
> > >
> > > -
> > > - addl 0(%ebp), %edx
> > > - adcl 4(%ebp), %edx
> > > - adcl 8(%ebp), %edx
> > > - adcl 12(%ebp), %edx
> > > - adcl 0(%ecx), %edx
> > > - adcl 4(%ecx), %edx
> > > - adcl 8(%ecx), %edx
> > > - adcl 12(%ecx), %edx
> > > - adcl %edi, %edx
> > > - adcl %eax, %edx
> > > - adcl $0, %edx
> > > -
> > > + addl 0(%ebp), %edxadcl 4(%ebp), %edxadcl 8(%ebp), %edxadcl 12(%ebp),
> %edxadcl 0(%ecx), %edxadcl 4(%ecx), %edxadcl 8(%ecx), %edxadcl 12(%ecx),
> %edxadcl %edi, %edxadcl %eax, %edxadcl $0, %edx
> >
> >
> > This is probably just some loop unrolling, not some as you say "wrong
> > code".
> 
> Correct me if I'm wrong, but I don't think "%edxadcl" really assembles....

No, but an edited 'diff' of the assembly output of a 'C' compiler doesn't
really tell much. Note, no line numbers, no clue as to what the diff
was about. The actual defective section of code would be much more
instructive. For instance, is this as a result of an in-line macro
expansion; the result of a <CR><LF> 'dos' file; the true output of
a 'C' file with no __inline__ __asm__?

Cheers,
Dick Johnson

Penguin : Linux version 2.4.1 on an i686 machine (799.53 BogoMips).

    I was going to compile a list of innovations that could be
    attributed to Microsoft. Once I realized that Ctrl-Alt-Del
    was handled in the BIOS, I found that there aren't any.

Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

[Brian Gerst]

"Richard B. Johnson" wrote:
> 
> On Wed, 28 Nov 2001, Martin Eriksson wrote:
> 
> > ----- Original Message -----
> > From: "Richard B. Johnson" <root@chaos.analogic.com>
> > To: <erik@hensema.net>
> > Cc: <linux-kernel@vger.kernel.org>
> > Sent: Wednesday, November 28, 2001 8:09 PM
> > Subject: Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?
> >
> >
> > > On 28 Nov 2001, Erik Hensema wrote:
> > >
> > > >
> > > > I've been looking into the compile problems of net/ipv6/ndisc.c in
> > > > 2.5.1-pre2 and I found that the asm generated by gcc (2.95.3) is wrong:
> > > >
> > > > This is a small part of a diff betweem two asm files generated by gcc,
> > note
> > > > the missing \n's in the wrong code:
> > > >
> > > > -
> > > > - addl 0(%ebp), %edx
> > > > - adcl 4(%ebp), %edx
> > > > - adcl 8(%ebp), %edx
> > > > - adcl 12(%ebp), %edx
> > > > - adcl 0(%ecx), %edx
> > > > - adcl 4(%ecx), %edx
> > > > - adcl 8(%ecx), %edx
> > > > - adcl 12(%ecx), %edx
> > > > - adcl %edi, %edx
> > > > - adcl %eax, %edx
> > > > - adcl $0, %edx
> > > > -
> > > > + addl 0(%ebp), %edxadcl 4(%ebp), %edxadcl 8(%ebp), %edxadcl 12(%ebp),
> > %edxadcl 0(%ecx), %edxadcl 4(%ecx), %edxadcl 8(%ecx), %edxadcl 12(%ecx),
> > %edxadcl %edi, %edxadcl %eax, %edxadcl $0, %edx
> > >
> > >
> > > This is probably just some loop unrolling, not some as you say "wrong
> > > code".
> >
> > Correct me if I'm wrong, but I don't think "%edxadcl" really assembles....
> 
> No, but an edited 'diff' of the assembly output of a 'C' compiler doesn't
> really tell much. Note, no line numbers, no clue as to what the diff
> was about. The actual defective section of code would be much more
> instructive. For instance, is this as a result of an in-line macro
> expansion; the result of a <CR><LF> 'dos' file; the true output of
> a 'C' file with no __inline__ __asm__?

It's from include/asm-i386/checksum.h, specifically csum_ipv6_magic(). 
The asm statements in this header file need semicolons or explicit
newlines after each opcode.

--

				Brian Gerst

PATCH: Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

[Erik Hensema]

Brian Gerst (bgerst@didntduck.org) wrote:
>"Richard B. Johnson" wrote:
>> 
>> On Wed, 28 Nov 2001, Martin Eriksson wrote:
>> > From: "Richard B. Johnson" <root@chaos.analogic.com>
>> > > On 28 Nov 2001, Erik Hensema wrote:
>> > > > I've been looking into the compile problems of net/ipv6/ndisc.c in
>> > > > 2.5.1-pre2 and I found that the asm generated by gcc (2.95.3) is wrong:
>> > > >
>> > > > This is a small part of a diff betweem two asm files generated by gcc,
>> > note
>> > > > the missing \n's in the wrong code:
>> > >
[code]
>> > > This is probably just some loop unrolling, not some as you say "wrong
>> > > code".
>> >
>> > Correct me if I'm wrong, but I don't think "%edxadcl" really assembles....
>> 
>> No, but an edited 'diff' of the assembly output of a 'C' compiler doesn't
>> really tell much. Note, no line numbers, no clue as to what the diff
>> was about. The actual defective section of code would be much more
>> instructive. For instance, is this as a result of an in-line macro
>> expansion; the result of a <CR><LF> 'dos' file; the true output of
>> a 'C' file with no __inline__ __asm__?
>
>It's from include/asm-i386/checksum.h, specifically csum_ipv6_magic(). 
>The asm statements in this header file need semicolons or explicit
>newlines after each opcode.

You're right, that was the problem.

This patch fixes it:

--- linux/include/asm-i386/checksum.h.orig	Wed Nov 28 21:49:00 2001
+++ linux/include/asm-i386/checksum.h	Wed Nov 28 21:52:28 2001
@@ -156,17 +156,17 @@
 						     unsigned int sum) 
 {
 	__asm__(
-		"addl 0(%1), %0"
-		"adcl 4(%1), %0"
-		"adcl 8(%1), %0"
-		"adcl 12(%1), %0"
-		"adcl 0(%2), %0"
-		"adcl 4(%2), %0"
-		"adcl 8(%2), %0"
-		"adcl 12(%2), %0"
-		"adcl %3, %0"
-		"adcl %4, %0"
-		"adcl $0, %0"
+		"addl 0(%1), %0		;\n"
+		"adcl 4(%1), %0		;\n"
+		"adcl 8(%1), %0		;\n"
+		"adcl 12(%1), %0	;\n"
+		"adcl 0(%2), %0		;\n"
+		"adcl 4(%2), %0		;\n"
+		"adcl 8(%2), %0		;\n"
+		"adcl 12(%2), %0	;\n"
+		"adcl %3, %0		;\n"
+		"adcl %4, %0		;\n"
+		"adcl $0, %0		;\n"
 		: "=&r" (sum)
 		: "r" (saddr), "r" (daddr), 
 		  "r"(htonl(len)), "r"(htonl(proto)), "0"(sum));

-- 
Erik Hensema (erik@hensema.net)
I'm on the list, no need to Cc: me, though I appreciate one if your
mailer doesn't support the References header.

Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

[Richard B. Johnson]

On Wed, 28 Nov 2001, Brian Gerst wrote:

> "Richard B. Johnson" wrote:
> > 
> > On Wed, 28 Nov 2001, Martin Eriksson wrote:
> > 
> > > ----- Original Message -----
> > > From: "Richard B. Johnson" <root@chaos.analogic.com>
> > > To: <erik@hensema.net>
> > > Cc: <linux-kernel@vger.kernel.org>
> > > Sent: Wednesday, November 28, 2001 8:09 PM
> > > Subject: Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?
> > >
> > >
> > > > On 28 Nov 2001, Erik Hensema wrote:
> > > >
> > > > >
> > > > > I've been looking into the compile problems of net/ipv6/ndisc.c in
> > > > > 2.5.1-pre2 and I found that the asm generated by gcc (2.95.3) is wrong:
> > > > >
> > > > > This is a small part of a diff betweem two asm files generated by gcc,
> > > note
> > > > > the missing \n's in the wrong code:
> > > > >
> > > > > -
> > > > > - addl 0(%ebp), %edx
> > > > > - adcl 4(%ebp), %edx
> > > > > - adcl 8(%ebp), %edx
> > > > > - adcl 12(%ebp), %edx
> > > > > - adcl 0(%ecx), %edx
> > > > > - adcl 4(%ecx), %edx
> > > > > - adcl 8(%ecx), %edx
> > > > > - adcl 12(%ecx), %edx
> > > > > - adcl %edi, %edx
> > > > > - adcl %eax, %edx
> > > > > - adcl $0, %edx
> > > > > -
> > > > > + addl 0(%ebp), %edxadcl 4(%ebp), %edxadcl 8(%ebp), %edxadcl 12(%ebp),
> > > %edxadcl 0(%ecx), %edxadcl 4(%ecx), %edxadcl 8(%ecx), %edxadcl 12(%ecx),
> > > %edxadcl %edi, %edxadcl %eax, %edxadcl $0, %edx
> > > >
> > > >
> > > > This is probably just some loop unrolling, not some as you say "wrong
> > > > code".
> > >
> > > Correct me if I'm wrong, but I don't think "%edxadcl" really assembles....
> > 
> > No, but an edited 'diff' of the assembly output of a 'C' compiler doesn't
> > really tell much. Note, no line numbers, no clue as to what the diff
> > was about. The actual defective section of code would be much more
> > instructive. For instance, is this as a result of an in-line macro
> > expansion; the result of a <CR><LF> 'dos' file; the true output of
> > a 'C' file with no __inline__ __asm__?
> 
> It's from include/asm-i386/checksum.h, specifically csum_ipv6_magic(). 
> The asm statements in this header file need semicolons or explicit
> newlines after each opcode.
> 

Yes. It isn't a 'C' compiler problem. Here is a patch. I remember
some comment about "multi-line string literals are depreciated..."
as a diagnostic message from some 'C' compiler version. Somebody
should check this out. This patch is "for the way we used to do it..."

SNIP
--- /usr/src/linux/include/asm-i386/checksum.h	Tue Feb  1 02:41:14 2000
+++ checksum.h	Thu Nov 29 08:56:11 2001
@@ -69,25 +69,24 @@
 					  unsigned int ihl) {
 	unsigned int sum;
 
-	__asm__ __volatile__("
-	    movl (%1), %0
-	    subl $4, %2
-	    jbe 2f
-	    addl 4(%1), %0
-	    adcl 8(%1), %0
-	    adcl 12(%1), %0
-1:	    adcl 16(%1), %0
-	    lea 4(%1), %1
-	    decl %2
-	    jne	1b
-	    adcl $0, %0
-	    movl %0, %2
-	    shrl $16, %0
-	    addw %w2, %w0
-	    adcl $0, %0
-	    notl %0
-2:
-	    "
+	__asm__ __volatile__(
+	"\n\tmovl (%1), %0\n"
+	"\tsubl $4, %2\n"
+	"\tjbe 2f\n"
+	"\taddl 4(%1), %0\n"
+	"\tadcl 8(%1), %0\n"
+	"\tadcl 12(%1), %0\n"
+	"1:\tadcl 16(%1), %0\n"
+	"\tlea 4(%1), %1\n"
+	"\tdecl %2\n"
+	"\tjne 1b\n"
+	"\tadcl $0, %0\n"
+	"\tmovl %0, %2\n"
+	"\tshrl $16, %0\n"
+	"\taddw %w2, %w0\n"
+	"\tadcl $0, %0\n"
+	"\tnotl %0\n"
+	"2:\n"
 	/* Since the input registers which are loaded with iph and ipl
 	   are modified, we must also specify them as outputs, or gcc
 	   will assume they contain their original values. */
@@ -102,10 +101,9 @@
 
 static inline unsigned int csum_fold(unsigned int sum)
 {
-	__asm__("
-		addl %1, %0
-		adcl $0xffff, %0
-		"
+	__asm__(
+		"\n\taddl %1, %0\n"
+		"\tadcl $0xffff, %0\n"
 		: "=r" (sum)
 		: "r" (sum << 16), "0" (sum & 0xffff0000)
 	);
@@ -118,12 +116,11 @@
 						   unsigned short proto,
 						   unsigned int sum) 
 {
-    __asm__("
-	addl %1, %0
-	adcl %2, %0
-	adcl %3, %0
-	adcl $0, %0
-	"
+    __asm__(
+	"\n\taddl %1, %0\n"
+	"\tadcl %2, %0\n"
+	"\tadcl %3, %0\n"
+	"\tadcl $0, %0\n"
 	: "=r" (sum)
 	: "g" (daddr), "g"(saddr), "g"((ntohs(len)<<16)+proto*256), "0"(sum));
     return sum;
@@ -158,19 +155,18 @@
 						     unsigned short proto,
 						     unsigned int sum) 
 {
-	__asm__("
-		addl 0(%1), %0
-		adcl 4(%1), %0
-		adcl 8(%1), %0
-		adcl 12(%1), %0
-		adcl 0(%2), %0
-		adcl 4(%2), %0
-		adcl 8(%2), %0
-		adcl 12(%2), %0
-		adcl %3, %0
-		adcl %4, %0
-		adcl $0, %0
-		"
+	__asm__(
+		"\n\taddl 0(%1), %0\n"
+		"\tadcl 4(%1), %0\n"
+		"\tadcl 8(%1), %0\n"
+		"\tadcl 12(%1), %0\n"
+		"\tadcl 0(%2), %0\n"
+		"\tadcl 4(%2), %0\n"
+		"\tadcl 8(%2), %0\n"
+		"\tadcl 12(%2), %0\n"
+		"\tadcl %3, %0\n"
+		"\tadcl %4, %0\n"
+		"\tadcl $0, %0\n"
 		: "=&r" (sum)
 		: "r" (saddr), "r" (daddr), 
 		  "r"(htonl(len)), "r"(htonl(proto)), "0"(sum));
SNIP


Here is something to test it with:

#define asmlinkage
#define NULL (void *)0
#define DUMMY 0
#define VERIFY_WRITE DUMMY 
#define EFAULT DUMMY
typedef unsigned int __u32;
struct in6_addr{ int dummy; };

#include "checksum.h"
void foo()
{
   char bar[0x100];
   ip_fast_csum(bar, sizeof(bar));
   csum_ipv6_magic(NULL, NULL, sizeof(bar), DUMMY, DUMMY);
   csum_tcpudp_nofold(DUMMY, DUMMY, sizeof(bar), DUMMY, DUMMY);
   csum_fold(DUMMY);
}


Just do `gcc -S -o xxx xxx.c` to see the code generated and verify
that the stuborn 'C' compilers like these strings okay.


Cheers,
Dick Johnson

Penguin : Linux version 2.4.1 on an i686 machine (799.53 BogoMips).

    I was going to compile a list of innovations that could be
    attributed to Microsoft. Once I realized that Ctrl-Alt-Del
    was handled in the BIOS, I found that there aren't any.