A Bug in gcc or asm/string.h ?

A Bug in gcc or asm/string.h ?

[Andreas Kies]

Hello,

When running a kernel module compiled with gcc 3.3.5, I think I've found a bug 
in either the compiler or the kernel definition of strcmp in 
asm-i386/string.h . The exact compiler version is :

Reading specs from /usr/lib/gcc-lib/i586-suse-linux/3.3.5/specs
Configured with: ../configure --enable-threads=posix --prefix=/usr 
--with-local-prefix=/usr/local --infodir=/usr/share/info 
--mandir=/usr/share/man --enable-languages=c,c++,f77,objc,java,ada 
--disable-checking --libdir=/usr/lib --enable-libgcj --with-slibdir=/lib 
--with-system-zlib --enable-shared --enable-__cxa_atexit i586-suse-linux
Thread model: posix
gcc version 3.3.5 20050117 (prerelease) (SUSE Linux)

I was able to construct a small pure usermode program.
If you compile the included test program with optimization level 1 or less it 
works, if compiled with level 2 it fails. Failing means the strcmp result 
is != 0 because char ptr has valid contents at the time strcmp is expanded.
Other platforms besides i386 might be affected, too.
In the case of a failure you get "Unrecognized" as output on stdout.

So, here are my questions :
- Is this a bug in the compiler ?
In Documentation/Changes version 2.95.x is still recommended, I guess this is 
outdated.
- Is it a bug in the definition of strcmp ? Maybe an addition volatile is 
missing.

Thanks for reading.

Andreas.

Please CC me, as I'm not on the list.

------test program-----

/* taken from <asm-i386/string.h> */
static inline int strcmp(const char * cs,const char * ct)
{
 int d0, d1;
 register int __res;
 __asm__ __volatile__(
   "1:\tlodsb\n\t"
   "scasb\n\t"
   "jne 2f\n\t"
   "testb %%al,%%al\n\t"
   "jne 1b\n\t"
   "xorl %%eax,%%eax\n\t"
   "jmp 3f\n"
   "2:\tsbbl %%eax,%%eax\n\t"
   "orb $1,%%al\n"
   "3:"
   :"=a" (__res), "=&S" (d0), "=&D" (d1)
   :"1" (cs),"2" (ct));
 return __res;
}
/* end of code from <asm-i386/string.h> */

int oem;

int main(void)
{
   char ptr[2];

   ptr[0] = 'G';
   ptr[1] = '\0';

   if (strcmp(ptr, "G") == 0) {
      oem = 0; /* this branch should always be executed*/
   } else {
      printf("Unrecognized\n");
      oem = 1;
   }
   return oem;
}

Re: A Bug in gcc or asm/string.h ?

[Paolo Ornati]

On Mon, 27 Jun 2005 01:05:28 +0200
Andreas Kies <andikies@t-online.de> wrote:

> Hello,
> 
> When running a kernel module compiled with gcc 3.3.5, I think I've found a
> bug  in either the compiler or the kernel definition of strcmp in 
> asm-i386/string.h . The exact compiler version is :
> 
> Reading specs from /usr/lib/gcc-lib/i586-suse-linux/3.3.5/specs
> Configured with: ../configure --enable-threads=posix --prefix=/usr 
> --with-local-prefix=/usr/local --infodir=/usr/share/info 
> --mandir=/usr/share/man --enable-languages=c,c++,f77,objc,java,ada 
> --disable-checking --libdir=/usr/lib --enable-libgcj --with-slibdir=/lib 
> --with-system-zlib --enable-shared --enable-__cxa_atexit i586-suse-linux
> Thread model: posix
> gcc version 3.3.5 20050117 (prerelease) (SUSE Linux)
> 
> I was able to construct a small pure usermode program.
> If you compile the included test program with optimization level 1 or less
> it  works, if compiled with level 2 it fails. Failing means the strcmp
> result  is != 0 because char ptr has valid contents at the time strcmp is
> expanded. Other platforms besides i386 might be affected, too.
> In the case of a failure you get "Unrecognized" as output on stdout.
> 
> So, here are my questions :
> - Is this a bug in the compiler ?
> In Documentation/Changes version 2.95.x is still recommended, I guess this
> is  outdated.
> - Is it a bug in the definition of strcmp ? Maybe an addition volatile is 
> missing.
> 
> Thanks for reading.
> 
...
> 
> int oem;
> 
> int main(void)
> {
>    char ptr[2];

what happens if you change "char ptr[2]" to "volatile char ptr[2]" ?

--
	Paolo Ornati
	Linux 2.6.12.1 on x86_64

Re: A Bug in gcc or asm/string.h ?

[Paolo Ornati]

PS: I've readded LKML to CC, since I think that this is a problem with the
ASM template


On Mon, 27 Jun 2005 20:59:20 +0200
Andreas Kies <andikies@t-online.de> wrote:

> Them it works, but that's not a solution at all. "volatile" destroys more
> or  less all optimizations.

Yes... I know, it was just to see what was the problem.


The problem is that GCC is caching in registers the value of "ptr[0]" and/or
"ptr[1]" and/or "ptr[2]".

A little better workaround would be to add "memory" to clobbered registers
in the asm template:

static inline int strcmp(const char * cs,const char * ct)
{
int d0, d1;
register int __res;
__asm__ __volatile__(
        "1:\tlodsb\n\t"
        "scasb\n\t"
        "jne 2f\n\t"
        "testb %%al,%%al\n\t"
        "jne 1b\n\t"
        "xorl %%eax,%%eax\n\t"
        "jmp 3f\n"
        "2:\tsbbl %%eax,%%eax\n\t"
        "orb $1,%%al\n"
        "3:"
        :"=a" (__res), "=&S" (d0), "=&D" (d1)
                     :"1" (cs),"2" (ct)
                     : "memory");	// <--- workaround
return __res;
}


In this way GCC puts everything is cached in register back to memory when
you call strcmp()... but you can argue that this isn't optimal.


I don't know if there is a better way... basically you need to tell GCC to
NOT cache these values.

I think that nobody hits this bug because the typical usage is different...
something like this:

	...
	char *str = "Hello!";		// or even char str[] = "Hello!";
	...
	strcmp (str, other_str);
	...

In this way "Hello!" _IS_ allocated in memory and "str" points to it.... GCC
optimizations can't hurt here (I hope ;).


CONCLUSION: I think that it should be fixed... but adding "memory" doesn't
seems The Right Thing to do.

--
	Paolo Ornati
	Linux 2.6.12.1 on x86_64

Re: A Bug in gcc or asm/string.h ?

[Nick Warne]

Please forgive posting to this thread, but I had this issue a few weeks back.

I was messing about with my bespoke Quake2 Xatrix server code, and have added 
a lot of standard string functions  for speed and stuff.  I used a few from 
the linux kernel string functions.

Read the thread for what went on:

http://www.quakesrc.org/forums/viewtopic.php?t=5144

I don't expect a reason here, just I couldn't get it to work at all - but the 
assembler one did...

Nick
-- 
"When you're chewing on life's gristle,
Don't grumble, Give a whistle..."

Re: A Bug in gcc or asm/string.h ?

[Andreas Kies]

On Monday 27 June 2005 21:43, Paolo Ornati wrote:
> PS: I've readded LKML to CC, since I think that this is a problem with the
> ASM template

Yes, you are right, it is not a compiler bug.
My apologies to the GCC team, in case anyone has read it.

[...]

> A little better workaround would be to add "memory" to clobbered registers
> in the asm template:
>
> static inline int strcmp(const char * cs,const char * ct)
> {
> int d0, d1;
> register int __res;
> __asm__ __volatile__(
>         "1:\tlodsb\n\t"
>         "scasb\n\t"
>         "jne 2f\n\t"
>         "testb %%al,%%al\n\t"
>         "jne 1b\n\t"
>         "xorl %%eax,%%eax\n\t"
>         "jmp 3f\n"
>         "2:\tsbbl %%eax,%%eax\n\t"
>         "orb $1,%%al\n"
>         "3:"
>
>         :"=a" (__res), "=&S" (d0), "=&D" (d1)
>         :
>                      :"1" (cs),"2" (ct)
>                      : "memory"); // <--- workaround
>
> return __res;
> }
>
>
> In this way GCC puts everything is cached in register back to memory when
> you call strcmp()... but you can argue that this isn't optimal.

Indeed the compiler has to assume that any memory location has changed.

> I don't know if there is a better way... basically you need to tell GCC to
> NOT cache these values.

There is one, it says that cs and ct address structures with 4 gigabyte size.
This is anyway not 64 bit clean.

static inline int strcmp(const char * cs,const char * ct)
{
        int d0, d1;
        register int __res;
        __asm__ __volatile__(
                        "1:\tlodsb\n\t"
                        "scasb\n\t"
                        "jne 2f\n\t"
                        "testb %%al,%%al\n\t"
                        "jne 1b\n\t"
                        "xorl %%eax,%%eax\n\t"
                        "jmp 3f\n"
                        "2:\tsbbl %%eax,%%eax\n\t"
                        "orb $1,%%al\n"
                        "3:"
                        :"=a" (__res), "=&S" (d0), "=&D" (d1)
                        :"1" (cs),"2" (ct),
                        "m" ( *(struct { char __x[0xfffffff]; } *)cs),
                        "m" ( *(struct { char __x[0xfffffff]; } *)ct));
        return __res;
}


Now, how do i formally submit this as a bug report ?

Andreas.

Re: A Bug in gcc or asm/string.h ?

[Paolo Ornati]

On Tue, 28 Jun 2005 01:53:04 +0200
Andreas Kies <andikies@t-online.de> wrote:

> Now, how do i formally submit this as a bug report ?

Not needed.

Linus has just fix it by adding "memory" to clobbered registers:

	http://marc.theaimsgroup.com/?l=bk-commits-head&m=111965412113339&w=2

--
	Paolo Ornati
	Linux 2.6.12.1 on x86_64