Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 9 Jul 2007 10:55:47 +0800
From:      zhouyi zhou <zhouzhouyi@FreeBSD.org>
To:        freebsd-hackers@FreeBSD.org
Cc:        freebsd-current@FreeBSD.org
Subject:   rewrite src/sys/i386/i386/in_cksum.c
Message-ID:  <20070709105547.71827eb8.zhouzhouyi@FreeBSD.org>

next in thread | raw e-mail | index | archive | help
Hey,
  Since FreeBSD update gcc from 3.4.x to 4.2.0, the FreeBSD gdb remote debugger in i386 platform
will find the TCP/UPD checksum will not be computed right, when calling macro in_cksum
which calls in_cksum_skip in src/sys/i386/i386/in_cksum.c in case of getting rid of -O flag
supplied to gcc when compiling(The optimize will interfere with comfortable gdb debugging).

  The reason is for example in src/sys/i386/i386/in_cksum.c:
    126   if (mlen >= 16) {
    127    __asm volatile ("addl %1, %0" : "+r" (sum) : "g" (((const unsigned int *)w)[0 / 4]));
    128    __asm volatile ("adcl %1, %0" : "+r" (sum) : "g" (((const unsigned int *)w)[4 / 4]));
    129    __asm volatile ("adcl %1, %0" : "+r" (sum) : "g" (((const unsigned int *)w)[8 / 4]));
    130    __asm volatile ("adcl %1, %0" : "+r" (sum) : "g" (((const unsigned int *)w)[12 / 4]));
    131    __asm volatile ("adcl         $0, %0" : "+r" (sum));
    132    w += 8;
    133    mlen -= 16;
    134   }
 
 This is OK with gcc-3.4.x, because the compiler will not insert add instructions between the _asm expressions,
but in gcc-4.x.x case, the compiler will insert add instruction between the _asm expressions which will let carrage bit in eflags cleared.
0x0804877c <in_cksum_skip+780>: cmpl   $0xf,0xffffffec(%ebp)
0x08048780 <in_cksum_skip+784>: jle    0x80487d0 <in_cksum_skip+864>
0x08048782 <in_cksum_skip+786>: mov    0xffffffe4(%ebp),%eax
0x08048785 <in_cksum_skip+789>: mov    (%eax),%eax
0x08048787 <in_cksum_skip+791>: mov    0xffffffe8(%ebp),%edx
0x0804878a <in_cksum_skip+794>: add    %eax,%edx
0x0804878c <in_cksum_skip+796>: mov    %edx,0xffffffe8(%ebp)
0x0804878f <in_cksum_skip+799>: mov    0xffffffe4(%ebp),%eax
---Type <return> to continue, or q <return> to quit---
0x08048792 <in_cksum_skip+802>: add    $0x4,%eax
0x08048795 <in_cksum_skip+805>: mov    (%eax),%eax
0x08048797 <in_cksum_skip+807>: mov    0xffffffe8(%ebp),%edx
0x0804879a <in_cksum_skip+810>: adc    %eax,%edx
0x0804879c <in_cksum_skip+812>: mov    %edx,0xffffffe8(%ebp)
0x0804879f <in_cksum_skip+815>: mov    0xffffffe4(%ebp),%eax
0x080487a2 <in_cksum_skip+818>: add    $0x8,%eax
0x080487a5 <in_cksum_skip+821>: mov    (%eax),%eax
0x080487a7 <in_cksum_skip+823>: mov    0xffffffe8(%ebp),%edx
0x080487aa <in_cksum_skip+826>: adc    %eax,%edx
0x080487ac <in_cksum_skip+828>: mov    %edx,0xffffffe8(%ebp)
0x080487af <in_cksum_skip+831>: mov    0xffffffe4(%ebp),%eax
0x080487b2 <in_cksum_skip+834>: add    $0xc,%eax
0x080487b5 <in_cksum_skip+837>: mov    (%eax),%eax
0x080487b7 <in_cksum_skip+839>: mov    0xffffffe8(%ebp),%edx
0x080487ba <in_cksum_skip+842>: adc    %eax,%edx
0x080487bc <in_cksum_skip+844>: mov    %edx,0xffffffe8(%ebp)
  
   So, I suggest rewrite src/sys/i386/i386/in_cksum.c in following style, for example:
--- in_cksum.c  2007-07-06 22:45:52.000000000 +0000
+++ in_cksum1.c 2007-07-07 00:18:25.000000000 +0000
@@ -124,11 +124,14 @@
    mlen -= 32;
   }
   if (mlen >= 16) {
-   __asm volatile ("addl %1, %0" : "+r" (sum) : "g" (((const unsigned int *)w)[0 / 4]));
-   __asm volatile ("adcl %1, %0" : "+r" (sum) : "g" (((const unsigned int *)w)[4 / 4]));
-   __asm volatile ("adcl %1, %0" : "+r" (sum) : "g" (((const unsigned int *)w)[8 / 4]));
-   __asm volatile ("adcl %1, %0" : "+r" (sum) : "g" (((const unsigned int *)w)[12 / 4]));
-   __asm volatile ("adcl         $0, %0" : "+r" (sum));
+    __asm volatile ("addl %1, %0 \n"
+                   "adcl %2, %0 \n"
+                   "adcl %3, %0 \n"
+                   "adcl %4, %0 \n"
+                   "adcl $0, %0": "+r"(sum): "g" (((const unsigned int *)w)[0 / 4]),
+                    "g" (((const unsigned int *)w)[4 / 4]),
+                   "g" (((const unsigned int *)w)[8 / 4]),
+                    "g" (((const unsigned int *)w)[12 / 4]));
    w += 8;
    mlen -= 16;
   }

   
   If someone is interested in it, he can lend me a hand to rewrite all of the similiar cases.

Sincerely yours
Zhouyi Zhou



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070709105547.71827eb8.zhouzhouyi>