2001-11-28 19:00:27

by spamtrap

[permalink] [raw]
Subject: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?


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 ([email protected])
I'm on the list, no need to Cc: me, though I appreciate one if your
a
mailer doesn't support the References header.


2001-11-28 19:13:18

by Richard B. Johnson

[permalink] [raw]
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".


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.


2001-11-28 19:29:29

by David Relson

[permalink] [raw]
Subject: Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

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


2001-11-28 19:43:09

by Martin Eriksson

[permalink] [raw]
Subject: Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

----- Original Message -----
From: "Richard B. Johnson" <[email protected]>
To: <[email protected]>
Cc: <[email protected]>
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 <[email protected]>
| MSc CSE student, department of Computing Science
| Ume? University, Sweden


2001-11-28 19:51:39

by Richard B. Johnson

[permalink] [raw]
Subject: Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

On Wed, 28 Nov 2001, Martin Eriksson wrote:

> ----- Original Message -----
> From: "Richard B. Johnson" <[email protected]>
> To: <[email protected]>
> Cc: <[email protected]>
> 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.


2001-11-28 20:14:49

by Brian Gerst

[permalink] [raw]
Subject: Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

"Richard B. Johnson" wrote:
>
> On Wed, 28 Nov 2001, Martin Eriksson wrote:
>
> > ----- Original Message -----
> > From: "Richard B. Johnson" <[email protected]>
> > To: <[email protected]>
> > Cc: <[email protected]>
> > 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

2001-11-28 21:22:14

by spamtrap

[permalink] [raw]
Subject: PATCH: Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

Brian Gerst ([email protected]) wrote:
>"Richard B. Johnson" wrote:
>>
>> On Wed, 28 Nov 2001, Martin Eriksson wrote:
>> > From: "Richard B. Johnson" <[email protected]>
>> > > 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 ([email protected])
I'm on the list, no need to Cc: me, though I appreciate one if your
mailer doesn't support the References header.

2001-11-29 14:07:32

by Richard B. Johnson

[permalink] [raw]
Subject: Re: Compilation problem in ndisc.c / 2.5.1-pre2 : possible gcc bug?

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" <[email protected]>
> > > To: <[email protected]>
> > > Cc: <[email protected]>
> > > 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.