Hi Andi,
some months ago, you invented a IN_STRING_C with the following comment:
<-- snip -->
gcc 3.4 optimizes sprintf(foo,"%s",string) into strcpy.
Unfortunately that isn't seen by the inliner and linux/i386 has no
out-of-line strcpy so you end up with a linker error.
This patch adds out of line copies for most string functions to avoid
this.
...
<-- snip -->
I tried 2.6.10-rc1-mm3 with gcc 3.4.2 and the patch below and didn't
observe the problems you described.
Can you still reproduce this problem?
If not, I'll suggest to apply the patch below which saves a few kB in
lib/string.o .
Signed-off-by: Adrian Bunk <[email protected]>
--- linux-2.6.10-rc1-mm3-full/include/asm-i386/string.h.old 2004-11-07 13:27:44.000000000 +0100
+++ linux-2.6.10-rc1-mm3-full/include/asm-i386/string.h 2004-11-07 13:28:47.000000000 +0100
@@ -25,7 +25,6 @@
/* AK: in fact I bet it would be better to move this stuff all out of line.
*/
-#if !defined(IN_STRING_C)
#define __HAVE_ARCH_STRCPY
static inline char * strcpy(char * dest,const char *src)
@@ -180,8 +179,6 @@
return __res;
}
-#endif
-
#define __HAVE_ARCH_STRLEN
static inline size_t strlen(const char * s)
{
--- linux-2.6.10-rc1-mm3-full/lib/string.c.old 2004-11-07 13:29:00.000000000 +0100
+++ linux-2.6.10-rc1-mm3-full/lib/string.c 2004-11-07 13:29:05.000000000 +0100
@@ -19,8 +19,6 @@
* - Kissed strtok() goodbye
*/
-#define IN_STRING_C 1
-
#include <linux/types.h>
#include <linux/string.h>
#include <linux/ctype.h>
> Can you still reproduce this problem?
> If not, I'll suggest to apply the patch below which saves a few kB in
> lib/string.o .
I would prefer to keep it because there is no guarantee in gcc
that it always inlines all string functions unless you pass
-minline-all-stringops. And with that the code would
be bloated much more than the few out of lined fallback
string functions.
Even if it works for you with your configuration it's just by luck.
-Andi
On Mon, Nov 08, 2004 at 02:44:49PM +0100, Andi Kleen wrote:
> > Can you still reproduce this problem?
> > If not, I'll suggest to apply the patch below which saves a few kB in
> > lib/string.o .
>
> I would prefer to keep it because there is no guarantee in gcc
> that it always inlines all string functions unless you pass
> -minline-all-stringops. And with that the code would
> be bloated much more than the few out of lined fallback
> string functions.
If I understand your changelog entry correctly, this wasn't the problem
(the asm string functions are "static inline").
Rethinking it, I don't even understand the sprintf example in your
changelog entry - shouldn't an inclusion of kernel.h always get it
right?
> Even if it works for you with your configuration it's just by luck.
The two configurations I tried are one configuration with _everything_
except modules enablesd, and the other was _everything_ modular.
That's why I'd like to have an example where it fails to understand the
problem.
> -Andi
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Mon, Nov 08, 2004 at 05:19:35PM +0100, Andi Kleen wrote:
> > Rethinking it, I don't even understand the sprintf example in your
> > changelog entry - shouldn't an inclusion of kernel.h always get it
> > right?
>
> Newer gcc rewrites sprintf(buf,"%s",str) to strcpy(buf,str) transparently.
Which gcc is "Newer"?
My gcc 3.4.2 didn't show this problem.
> -Andi
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
> Rethinking it, I don't even understand the sprintf example in your
> changelog entry - shouldn't an inclusion of kernel.h always get it
> right?
Newer gcc rewrites sprintf(buf,"%s",str) to strcpy(buf,str) transparently.
-Andi
On Mon, Nov 08, 2004 at 05:31:01PM +0100, Adrian Bunk wrote:
> On Mon, Nov 08, 2004 at 05:19:35PM +0100, Andi Kleen wrote:
> > > Rethinking it, I don't even understand the sprintf example in your
> > > changelog entry - shouldn't an inclusion of kernel.h always get it
> > > right?
> >
> > Newer gcc rewrites sprintf(buf,"%s",str) to strcpy(buf,str) transparently.
>
> Which gcc is "Newer"?
I saw it with 3.3-hammer, which had additional optimizations in this
area at some point. Note that 3.3-hammer is widely used. I don't
know if 3.4 does it in the same way.
-Andi
On Monday 08 of November 2004 17:31, you wrote:
> On Mon, Nov 08, 2004 at 05:19:35PM +0100, Andi Kleen wrote:
> > > Rethinking it, I don't even understand the sprintf example in your
> > > changelog entry - shouldn't an inclusion of kernel.h always get it
> > > right?
> >
> > Newer gcc rewrites sprintf(buf,"%s",str) to strcpy(buf,str)
> > transparently.
>
> Which gcc is "Newer"?
>
> My gcc 3.4.2 didn't show this problem.
#include <stdio.h>
#include <string.h>
char buf[128];
void test(char *str)
{
sprintf(buf, "%s", str);
}
gcc -Wall sp.c -S -O2 -fomit-frame-pointer -mregparm=3
.file "sp.c"
.text
.p2align 4,,15
.globl test
.type test, @function
test:
movl %eax, %edx
movl $buf, %eax
jmp strcpy
.size test, .-test
.comm buf,128,32
.section .note.GNU-stack,"",@progbits
.ident "GCC: (GNU) 3.4.3 (PLD Linux)"
--
/* Copyright (C) 2003, SCO, Inc. This is valuable Intellectual Property. */
#define say(x) lie(x)
On Mon, 8 Nov 2004, Andi Kleen wrote:
>> Rethinking it, I don't even understand the sprintf example in your
>> changelog entry - shouldn't an inclusion of kernel.h always get it
>> right?
>
> Newer gcc rewrites sprintf(buf,"%s",str) to strcpy(buf,str) transparently.
>
> -Andi
Hmmm, how does it get the correct return-value and type? I don't
think that a compiler is allowed to change the function(s) called.
If gcc is doing this now, there are many potential problems and
it needs to be fixed. For instance, one can't qualify a
'C' runtime library and then have a compiler decide that it's
not going to call the pre-qualified function.
Cheers,
Dick Johnson
Penguin : Linux version 2.6.9 on an i686 machine (5537.79 BogoMips).
Notice : All mail here is now cached for review by John Ashcroft.
98.36% of all statistics are fiction.
On Mon, Nov 08, 2004 at 06:51:20PM +0100, Andi Kleen wrote:
> On Mon, Nov 08, 2004 at 05:31:01PM +0100, Adrian Bunk wrote:
> > On Mon, Nov 08, 2004 at 05:19:35PM +0100, Andi Kleen wrote:
> > > > Rethinking it, I don't even understand the sprintf example in your
> > > > changelog entry - shouldn't an inclusion of kernel.h always get it
> > > > right?
> > >
> > > Newer gcc rewrites sprintf(buf,"%s",str) to strcpy(buf,str) transparently.
> >
> > Which gcc is "Newer"?
>
> I saw it with 3.3-hammer, which had additional optimizations in this
> area at some point. Note that 3.3-hammer is widely used. I don't
> know if 3.4 does it in the same way.
Is this a -hammer specific problem?
If yes, does a -no-builtin-sprintf fix it?
Or is the problem a missing #include <linux/kernel.h> at the top of
include/linux/string.h?
> -Andi
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Mon, Nov 08, 2004 at 07:04:13PM +0100, Pawe?? Sikora wrote:
> On Monday 08 of November 2004 17:31, you wrote:
> > On Mon, Nov 08, 2004 at 05:19:35PM +0100, Andi Kleen wrote:
> > > > Rethinking it, I don't even understand the sprintf example in your
> > > > changelog entry - shouldn't an inclusion of kernel.h always get it
> > > > right?
> > >
> > > Newer gcc rewrites sprintf(buf,"%s",str) to strcpy(buf,str)
> > > transparently.
> >
> > Which gcc is "Newer"?
> >
> > My gcc 3.4.2 didn't show this problem.
>
> #include <stdio.h>
> #include <string.h>
> char buf[128];
> void test(char *str)
> {
> sprintf(buf, "%s", str);
> }
>...
> jmp strcpy
>...
This is the userspace example.
The kernel example is:
#include <linux/string.h>
#include <linux/kernel.h>
char buf[128];
void test(char *str)
{
sprintf(buf, "%s", str);
}
This results with gcc-3.4 (GCC) 3.4.2 (Debian 3.4.2-3) in:
.file "test.c"
.section .rodata.str1.1,"aMS",@progbits,1
.LC0:
.string "%s"
.text
.p2align 4,,15
.globl test
.type test, @function
test:
pushl %eax
pushl $.LC0
pushl $buf
call sprintf
addl $12, %esp
ret
.size test, .-test
.globl buf
.bss
.align 32
.type buf, @object
.size buf, 128
buf:
.zero 128
.section .note.GNU-stack,"",@progbits
.ident "GCC: (GNU) 3.4.2 (Debian 3.4.2-3)"
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Monday 08 of November 2004 19:31, you wrote:
> On Mon, Nov 08, 2004 at 07:04:13PM +0100, Pawe?? Sikora wrote:
> > On Monday 08 of November 2004 17:31, you wrote:
> > > On Mon, Nov 08, 2004 at 05:19:35PM +0100, Andi Kleen wrote:
> > > > > Rethinking it, I don't even understand the sprintf example in your
> > > > > changelog entry - shouldn't an inclusion of kernel.h always get it
> > > > > right?
> > > >
> > > > Newer gcc rewrites sprintf(buf,"%s",str) to strcpy(buf,str)
> > > > transparently.
> > >
> > > Which gcc is "Newer"?
> > >
> > > My gcc 3.4.2 didn't show this problem.
> >
> > #include <stdio.h>
> > #include <string.h>
> > char buf[128];
> > void test(char *str)
> > {
> > sprintf(buf, "%s", str);
> > }
> >...
> > jmp strcpy
> >...
>
> This is the userspace example.
>
> The kernel example is:
>
> #include <linux/string.h>
> #include <linux/kernel.h>
>
> char buf[128];
> void test(char *str)
> {
> sprintf(buf, "%s", str);
> }
>
>
> This results with gcc-3.4 (GCC) 3.4.2 (Debian 3.4.2-3) in:
>
> .file "test.c"
> .section .rodata.str1.1,"aMS",@progbits,1
> .LC0:
> .string "%s"
> .text
> .p2align 4,,15
> .globl test
> .type test, @function
> test:
> pushl %eax
> pushl $.LC0
> pushl $buf
> call sprintf
> addl $12, %esp
> ret
> .size test, .-test
> .globl buf
> .bss
> .align 32
> .type buf, @object
> .size buf, 128
> buf:
> .zero 128
> .section .note.GNU-stack,"",@progbits
> .ident "GCC: (GNU) 3.4.2 (Debian 3.4.2-3)"
[~/rpm/BUILD] # cat sp.c
#include <linux/string.h>
#include <linux/kernel.h>
char buf[128];
void test(char *str)
{
sprintf(buf, "%s", str);
}
[~/rpm/BUILD] # gcc -Wall sp.c -S -O2 -fomit-frame-pointer -mregparm=3
-nostdinc -isystem /usr/src/linux/include
sp.c: In function `test':
sp.c:7: warning: implicit declaration of function `sprintf'
[~/rpm/BUILD] # cat sp.s
.file "sp.c"
.text
.p2align 4,,15
.globl test
.type test, @function
test:
movl %eax, %edx
movl $buf, %eax
jmp strcpy
.size test, .-test
.comm buf,128,32
.section .note.GNU-stack,"",@progbits
.ident "GCC: (GNU) 3.4.3 (PLD Linux)"
What now?
--
/* Copyright (C) 2003, SCO, Inc. This is valuable Intellectual Property. */
#define say(x) lie(x)
On Monday 08 of November 2004 19:52, you wrote:
> On Mon, Nov 08, 2004 at 07:42:38PM +0100, Pawe?? Sikora wrote:
> >...
> > [~/rpm/BUILD] # gcc -Wall sp.c -S -O2 -fomit-frame-pointer -mregparm=3
> > -nostdinc -isystem /usr/src/linux/include
> >
> > sp.c: In function `test':
> > sp.c:7: warning: implicit declaration of function `sprintf'
> >
> > [~/rpm/BUILD] # cat sp.s
> >
> > .file "sp.c"
> > .text
> > .p2align 4,,15
> > .globl test
> > .type test, @function
> > test:
> > movl %eax, %edx
> > movl $buf, %eax
> > jmp strcpy
> > .size test, .-test
> > .comm buf,128,32
> > .section .note.GNU-stack,"",@progbits
> > .ident "GCC: (GNU) 3.4.3 (PLD Linux)"
> >
> >
> > What now?
>
> Do a "make V=1" and use the complete gcc call you see there.
[~/rpm/BUILD/linux-2.6.10-rc1] #
gcc -nostdinc -iwithprefix include -D__KERNEL__ -Iinclude -Wall
-Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -O2
-Wdeclaration-after-statement -pipe -msoft-float -mpreferred-stack-boundary=2
-fno-unit-at-a-time -march=pentium3 -Iinclude/asm-i386/mach-default -S sp.c
.file "sp.c"
.text
.p2align 4,,15
.globl test
.type test, @function
test:
pushl %ebp
movl %esp, %ebp
subl $8, %esp
movl $buf, (%esp)
movl 8(%ebp), %eax
movl %eax, 4(%esp)
call strcpy
leave
ret
.size test, .-test
.p2align 4,,15
.type strcpy, @function
strcpy:
pushl %ebp
movl %esp, %ebp
subl $12, %esp
movl %esi, -8(%ebp)
movl 8(%ebp), %edx
movl 12(%ebp), %esi
movl %edi, -4(%ebp)
movl %edx, %edi
#APP
1: lodsb
stosb
testb %al,%al
jne 1b
#NO_APP
movl -8(%ebp), %esi
movl %edx, %eax
movl -4(%ebp), %edi
movl %ebp, %esp
popl %ebp
ret
.size strcpy, .-strcpy
.globl buf
.bss
.align 32
.type buf, @object
.size buf, 128
buf:
.zero 128
.section .note.GNU-stack,"",@progbits
.ident "GCC: (GNU) 3.4.3 (PLD Linux)"
--
/* Copyright (C) 2003, SCO, Inc. This is valuable Intellectual Property. */
#define say(x) lie(x)
On Mon, 8 Nov 2004, Adrian Bunk wrote:
> On Mon, Nov 08, 2004 at 07:04:13PM +0100, Pawe?? Sikora wrote:
>> On Monday 08 of November 2004 17:31, you wrote:
>>> On Mon, Nov 08, 2004 at 05:19:35PM +0100, Andi Kleen wrote:
>>>>> Rethinking it, I don't even understand the sprintf example in your
>>>>> changelog entry - shouldn't an inclusion of kernel.h always get it
>>>>> right?
>>>>
>>>> Newer gcc rewrites sprintf(buf,"%s",str) to strcpy(buf,str)
>>>> transparently.
>>>
>>> Which gcc is "Newer"?
>>>
>>> My gcc 3.4.2 didn't show this problem.
>>
>> #include <stdio.h>
>> #include <string.h>
>> char buf[128];
>> void test(char *str)
>> {
>> sprintf(buf, "%s", str);
>> }
>> ...
>> jmp strcpy
>> ...
>
>
> This is the userspace example.
>
> The kernel example is:
>
> #include <linux/string.h>
> #include <linux/kernel.h>
>
> char buf[128];
> void test(char *str)
> {
> sprintf(buf, "%s", str);
> }
>
>
> This results with gcc-3.4 (GCC) 3.4.2 (Debian 3.4.2-3) in:
>
> .file "test.c"
> .section .rodata.str1.1,"aMS",@progbits,1
> .LC0:
> .string "%s"
> .text
> .p2align 4,,15
> .globl test
> .type test, @function
> test:
> pushl %eax
> pushl $.LC0
> pushl $buf
> call sprintf
> addl $12, %esp
> ret
> .size test, .-test
> .globl buf
> .bss
> .align 32
> .type buf, @object
> .size buf, 128
> buf:
> .zero 128
> .section .note.GNU-stack,"",@progbits
> .ident "GCC: (GNU) 3.4.2 (Debian 3.4.2-3)"
>
>
>
> cu
> Adrian
>
On this compiler 3.3.3, -O2 will cause it to use strcpy().
Cheers,
Dick Johnson
Penguin : Linux version 2.6.9 on an i686 machine (5537.79 BogoMips).
Notice : All mail here is now cached for review by John Ashcroft.
98.36% of all statistics are fiction.
On Mon, Nov 08, 2004 at 07:34:49PM +0100, Adrian Bunk wrote:
> On Mon, Nov 08, 2004 at 06:51:20PM +0100, Andi Kleen wrote:
> > On Mon, Nov 08, 2004 at 05:31:01PM +0100, Adrian Bunk wrote:
> > > On Mon, Nov 08, 2004 at 05:19:35PM +0100, Andi Kleen wrote:
> > > > > Rethinking it, I don't even understand the sprintf example in your
> > > > > changelog entry - shouldn't an inclusion of kernel.h always get it
> > > > > right?
> > > >
> > > > Newer gcc rewrites sprintf(buf,"%s",str) to strcpy(buf,str) transparently.
> > >
> > > Which gcc is "Newer"?
> >
> > I saw it with 3.3-hammer, which had additional optimizations in this
> > area at some point. Note that 3.3-hammer is widely used. I don't
> > know if 3.4 does it in the same way.
>
> Is this a -hammer specific problem?
No, I just checked a 4.0 mainline gcc and it does it too.
Note I saw it on x86-64, don't know if it occurs on i386 too.
-Andi
On Monday 08 November 2004 10:22, linux-os wrote:
> Hmmm, how does it get the correct return-value and type?
Presumably it does it only for sprintf() calls where the return value is
ignored.
> I don't
> think that a compiler is allowed to change the function(s) called.
GCC is making the assumption that the functions it's replacing comply with the
C standard. I personally don't think that's too insane, especially since it
can be turned off (see below).
> If gcc is doing this now, there are many potential problems and
> it needs to be fixed. For instance, one can't qualify a
> 'C' runtime library and then have a compiler decide that it's
> not going to call the pre-qualified function.
-fno-builtin
-Ryan
On Mon, Nov 08, 2004 at 08:11:46PM +0100, Pawe?? Sikora wrote:
>
> gcc -nostdinc -iwithprefix include -D__KERNEL__ -Iinclude -Wall
> -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -O2
> -Wdeclaration-after-statement -pipe -msoft-float -mpreferred-stack-boundary=2
> -fno-unit-at-a-time -march=pentium3 -Iinclude/asm-i386/mach-default -S sp.c
>
>...
> call strcpy
>...
> strcpy:
> pushl %ebp
>...
That's no function call. :-)
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Mon, Nov 08, 2004 at 02:12:18PM -0500, linux-os wrote:
>
> On this compiler 3.3.3, -O2 will cause it to use strcpy().
Not for me:
.file "test.c"
.section .rodata.str1.1,"aMS",@progbits,1
.LC0:
.string "%s"
.text
.p2align 4,,15
.globl test
.type test, @function
test:
pushl %ebp
movl %esp, %ebp
subl $12, %esp
movl %eax, 8(%esp)
movl $.LC0, %eax
movl %eax, 4(%esp)
movl $buf, (%esp)
call sprintf
movl %ebp, %esp
popl %ebp
ret
.size test, .-test
.globl buf
.bss
.align 32
.type buf, @object
.size buf, 128
buf:
.zero 128
.section .note.GNU-stack,"",@progbits
.ident "GCC: (GNU) 3.3.5 (Debian 1:3.3.5-2)"
Are you using exactly my example file?
Are you using the complete gcc command line as shown by "make V=1"?
Which gcc 3.3.3 are you using?
> Cheers,
> Dick Johnson
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Mon, 8 Nov 2004, Adrian Bunk wrote:
> On Mon, Nov 08, 2004 at 02:12:18PM -0500, linux-os wrote:
>>
>> On this compiler 3.3.3, -O2 will cause it to use strcpy().
>
> Not for me:
>
> .file "test.c"
> .section .rodata.str1.1,"aMS",@progbits,1
> .LC0:
> .string "%s"
> .text
> .p2align 4,,15
> .globl test
> .type test, @function
> test:
> pushl %ebp
> movl %esp, %ebp
> subl $12, %esp
> movl %eax, 8(%esp)
> movl $.LC0, %eax
> movl %eax, 4(%esp)
> movl $buf, (%esp)
> call sprintf
> movl %ebp, %esp
> popl %ebp
> ret
> .size test, .-test
> .globl buf
> .bss
> .align 32
> .type buf, @object
> .size buf, 128
> buf:
> .zero 128
> .section .note.GNU-stack,"",@progbits
> .ident "GCC: (GNU) 3.3.5 (Debian 1:3.3.5-2)"
>
>
>
> Are you using exactly my example file?
> Are you using the complete gcc command line as shown by "make V=1"?
> Which gcc 3.3.3 are you using?
>
No, I am using (no headers):
-------------------
extern int sprintf(char *, const char *,...);
extern int puts(const char *);
static const char hello[]="Hello";
int xxx(void);
int xxx(){
char buf[0x100];
sprintf(buf, "%s", hello);
puts(buf);
return 0;
}
--------------------
Compiled as:
gcc -O2 -Wall -S -o xxx xxx.c
I get:
.file "xxx.c"
.section .rodata
.type hello, @object
.size hello, 6
hello:
.string "Hello"
.text
.p2align 2,,3
.globl xxx
.type xxx, @function
xxx:
pushl %ebp
movl %esp, %ebp
pushl %ebx
subl $268, %esp
pushl $hello
leal -264(%ebp), %ebx
pushl %ebx
call strcpy
movl %ebx, (%esp)
call puts
xorl %eax, %eax
movl -4(%ebp), %ebx
leave
ret
.size xxx, .-xxx
.section .note.GNU-stack,"",@progbits
.ident "GCC: (GNU) 3.3.3 20040412 (Red Hat Linux 3.3.3-7)"
If I don't use -O2, I get the sprintf() call. If I use
the constant string "Hello" instead of the allocated string,
it just bypasses everything and calls puts() directly.
If I use -fno-builtin, then it doesn't bypass sprintf().
However, there is no ISO C standard of "built-in" so
I don't think any compiler should default to something
that is undefined and decide to do whatever its current
whims are. Certainly, a compiler that has some capabilities,
not defined by a standard, should be able to use those
capabilities, but it certainly shouldn't decide to do
these things on its own.
Reviewing `man gcc` I see where I should be able to
find out what the built-in commands are:
gcc -dumpspecs
However, this man page is wrong because I don't get a
listing of any built-in functions, only the linking
and compiler defaults.
The compiler is a tool. It should be just like other
tools. Any killer options should be something that
take a special effort to turn ON. It shouldn't default
to firing the bullet out both ends of the barrel!
Cheers,
Dick Johnson
Penguin : Linux version 2.6.9 on an i686 machine (5537.79 BogoMips).
Notice : All mail here is now cached for review by John Ashcroft.
98.36% of all statistics are fiction.
On Mon, Nov 08, 2004 at 05:15:14PM -0500, linux-os wrote:
> >...
> >Are you using exactly my example file?
> >Are you using the complete gcc command line as shown by "make V=1"?
> >Which gcc 3.3.3 are you using?
> >
>
> No, I am using (no headers):
> -------------------
> extern int sprintf(char *, const char *,...);
> extern int puts(const char *);
> static const char hello[]="Hello";
> int xxx(void);
> int xxx(){
> char buf[0x100];
> sprintf(buf, "%s", hello);
> puts(buf);
> return 0;
> }
> --------------------
>
> Compiled as:
>
> gcc -O2 -Wall -S -o xxx xxx.c
>...
If you don't compile the code as it would be compiled inside the kernel,
that's your fault...
Please reply only if you can reproduce this with
#include <linux/string.h>, #include <linux/kernel.h> _and_ a gcc command
line as it would be in the kernel - everything else is useless.
> Cheers,
> Dick Johnson
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Mon, 8 Nov 2004, Adrian Bunk wrote:
>
> If you don't compile the code as it would be compiled inside the kernel,
> that's your fault...
>
> Please reply only if you can reproduce this with
> #include <linux/string.h>, #include <linux/kernel.h> _and_ a gcc command
> line as it would be in the kernel - everything else is useless.
I attached a script that duplicates the gcc command line for the
compile and also generates the 'C' code.
Script started on Mon 08 Nov 2004 05:50:54 PM EST
# sh -v xxx.sh
#!/bin/bash
cat >xxx.c <<EOF
KERN=/usr/src/linux-`uname -r`
uname -r
touch xxx.o.d
gcc -Wp,-MD,xxx.o.d -nostdinc -iwithprefix include -D__KERNEL__ -I${KERN}/include -Wall -Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -O2 -fomit-frame-pointer -Wdeclaration-after-statement -pipe -msoft-float -mpreferred-stack-boundary=2 -march=pentium4 -I${KERN}/include/asm-i386/mach-generic -I${KERN}/include/asm-i386/mach-default -DKVER6 -DMAJOR_NR=178 -DTRUE=1 -DFALSE=0 -DMODULE -DKBUILD_BASENAME=xxx -DKBUILD_MODNAME=Junk -S -o xxx xxx.c
cat xxx
.file "xxx.c"
.section .rodata
.type hello, @object
.size hello, 6
hello:
.string "Hello"
.text
.globl xxx
.type xxx, @function
xxx:
subl $268, %esp
movl %ebx, 264(%esp)
movl $hello, 4(%esp)
leal 8(%esp), %ebx
movl %ebx, (%esp)
call strcpy
movl %ebx, (%esp)
call printk
movl 264(%esp), %ebx
xorl %eax, %eax
addl $268, %esp
ret
.size xxx, .-xxx
.type strcpy, @function
strcpy:
subl $8, %esp
movl 12(%esp), %ecx
movl %esi, (%esp)
movl %edi, 4(%esp)
movl 16(%esp), %esi
movl %ecx, %edi
#APP
1: lodsb
stosb
testb %al,%al
jne 1b
#NO_APP
movl %ecx, %eax
movl (%esp), %esi
movl 4(%esp), %edi
addl $8, %esp
ret
.size strcpy, .-strcpy
.section .note.GNU-stack,"",@progbits
.ident "GCC: (GNU) 3.3.3 20040412 (Red Hat Linux 3.3.3-7)"
# exit
Script done on Mon 08 Nov 2004 05:51:03 PM EST
It clearly invents strcpy, having never been referenced in the
source.
Cheers,
Dick Johnson
Penguin : Linux version 2.6.9 on an i686 machine (5537.79 BogoMips).
Notice : All mail here is now cached for review by John Ashcroft.
98.36% of all statistics are fiction.
On Mon, Nov 08, 2004 at 05:57:12PM -0500, linux-os wrote:
>...
> call strcpy
>...
> strcpy:
> subl $8, %esp
>...
> It clearly invents strcpy, having never been referenced in the
> source.
The asm code you sent does _not_ call a global strcpy function.
It calls an asm procedure named "strcpy" it ships itself.
BTW: You are the second person in this thread I have to explain this to...
> Cheers,
> Dick Johnson
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Mon, Nov 08, 2004 at 08:01:30PM +0100, Andi Kleen wrote:
> On Mon, Nov 08, 2004 at 07:34:49PM +0100, Adrian Bunk wrote:
> > On Mon, Nov 08, 2004 at 06:51:20PM +0100, Andi Kleen wrote:
> > > On Mon, Nov 08, 2004 at 05:31:01PM +0100, Adrian Bunk wrote:
> > > > On Mon, Nov 08, 2004 at 05:19:35PM +0100, Andi Kleen wrote:
> > > > > > Rethinking it, I don't even understand the sprintf example in your
> > > > > > changelog entry - shouldn't an inclusion of kernel.h always get it
> > > > > > right?
> > > > >
> > > > > Newer gcc rewrites sprintf(buf,"%s",str) to strcpy(buf,str) transparently.
> > > >
> > > > Which gcc is "Newer"?
> > >
> > > I saw it with 3.3-hammer, which had additional optimizations in this
> > > area at some point. Note that 3.3-hammer is widely used. I don't
> > > know if 3.4 does it in the same way.
> >
> > Is this a -hammer specific problem?
>
> No, I just checked a 4.0 mainline gcc and it does it too.
>
> Note I saw it on x86-64, don't know if it occurs on i386 too.
OK, I see the difference:
After removing -fno-unit-at-a-time, I see this problem, too.
Why doesn't the kernel use -ffreestanding which should prevent all such
problems?
> -Andi
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
> Why doesn't the kernel use -ffreestanding which should prevent all such
> problems?
Because we want most of these optimizations. Also with -ffreestanding
you would need to supply the out of line string functions anyways
because gcc wouldn't inline them.
-Andi
On Tue, 9 Nov 2004, Adrian Bunk wrote:
> On Mon, Nov 08, 2004 at 05:57:12PM -0500, linux-os wrote:
>> ...
>> call strcpy
>> ...
>> strcpy:
>> subl $8, %esp
>> ...
>> It clearly invents strcpy, having never been referenced in the
>> source.
>
> The asm code you sent does _not_ call a global strcpy function.
> It calls an asm procedure named "strcpy" it ships itself.
>
> BTW: You are the second person in this thread I have to explain this to...
>
>> Cheers,
>> Dick Johnson
>
> cu
> Adrian
Explain WHAT? There is NO strcpy in the code. No such procedure
should have been called. Period. The generated code is defective.
Cheers,
Dick Johnson
Penguin : Linux version 2.6.9 on an i686 machine (5537.79 BogoMips).
Notice : All mail here is now cached for review by John Ashcroft.
98.36% of all statistics are fiction.
On Tue, 9 Nov 2004, linux-os wrote:
> On Tue, 9 Nov 2004, Adrian Bunk wrote:
>
>> On Mon, Nov 08, 2004 at 05:57:12PM -0500, linux-os wrote:
>>> ...
>>> call strcpy
>>> ...
>>> strcpy:
>>> subl $8, %esp
>>> ...
>>> It clearly invents strcpy, having never been referenced in the
>>> source.
>>
>> The asm code you sent does _not_ call a global strcpy function.
>> It calls an asm procedure named "strcpy" it ships itself.
>>
>> BTW: You are the second person in this thread I have to explain this to...
>>
>>> Cheers,
>>> Dick Johnson
>>
>> cu
>> Adrian
>
> Explain WHAT? There is NO strcpy in the code. No such procedure
> should have been called. Period. The generated code is defective.
>
Here is more information showing the defective code generation.
Output of `nm`
00000000 r hello
U printk
00000039 t strcpy
00000000 T xxx
Clearly shows a strcpy symbol when strcpy() was never referenced
in the 'C' code.
Output of objdump
xxx.o: file format elf32-i386
Disassembly of section .text:
00000000 <xxx>:
0: 81 ec 0c 01 00 00 sub $0x10c,%esp
6: 89 9c 24 08 01 00 00 mov %ebx,0x108(%esp)
d: c7 44 24 04 00 00 00 movl $0x0,0x4(%esp)
14: 00
15: 8d 5c 24 08 lea 0x8(%esp),%ebx
19: 89 1c 24 mov %ebx,(%esp)
1c: e8 18 00 00 00 call 39 <strcpy>
21: 89 1c 24 mov %ebx,(%esp)
24: e8 fc ff ff ff call 25 <xxx+0x25>
29: 8b 9c 24 08 01 00 00 mov 0x108(%esp),%ebx
30: 31 c0 xor %eax,%eax
32: 81 c4 0c 01 00 00 add $0x10c,%esp
38: c3 ret
00000039 <strcpy>:
39: 83 ec 08 sub $0x8,%esp
3c: 8b 4c 24 0c mov 0xc(%esp),%ecx
40: 89 34 24 mov %esi,(%esp)
43: 89 7c 24 04 mov %edi,0x4(%esp)
47: 8b 74 24 10 mov 0x10(%esp),%esi
4b: 89 cf mov %ecx,%edi
4d: ac lods %ds:(%esi),%al
4e: aa stos %al,%es:(%edi)
4f: 84 c0 test %al,%al
51: 75 fa jne 4d <strcpy+0x14>
53: 89 c8 mov %ecx,%eax
55: 8b 34 24 mov (%esp),%esi
58: 8b 7c 24 04 mov 0x4(%esp),%edi
5c: 83 c4 08 add $0x8,%esp
5f: c3 ret
[SNIPPED...]
Clearly shows a GLOBAL procedure called strcpy when
no such procedure was ever referenced in the code.
Now I will show that the strcpy() procedure is NOT local.
It's global and can screw up anything:
Another program....
char printk; // Just quiet the reference
// ... just a label, never called.
extern int puts(const char *); // No header funnies
extern char *strcpy(char *, const char *);
int main(){
char buf[0x100];
strcpy(buf, "WTF?"); // Going to call it!
puts(buf);
return 0;
}
Show code generation.
$ gcc -S -o aaa zzz.c
.file "zzz.c"
.section .rodata
.LC0:
.string "WTF?"
.text
.globl main
.type main, @function
main:
pushl %ebp
movl %esp, %ebp
subl $264, %esp
andl $-16, %esp
movl $0, %eax
subl %eax, %esp
subl $8, %esp
pushl $.LC0
leal -264(%ebp), %eax
pushl %eax
call strcpy # Clearly calls it.
addl $16, %esp
subl $12, %esp
leal -264(%ebp), %eax
pushl %eax
call puts
addl $16, %esp
movl $0, %eax
leave
ret
.size main, .-main
.comm printk,1,1
.section .note.GNU-stack,"",@progbits
.ident "GCC: (GNU) 3.3.3 20040412 (Red Hat Linux 3.3.3-7)"
Now compile and link with the file containing ROGUE strcpy().
$ gcc -o zzz zzz.c xxx.o
$ ./zzz
WTF?
Executes, calling a ROGUE function that MUST NOT exist.
Cheers,
Dick Johnson
Penguin : Linux version 2.6.9 on an i686 machine (5537.79 BogoMips).
Notice : All mail here is now cached for review by John Ashcroft.
98.36% of all statistics are fiction.
> On Mon, Nov 08, 2004 at 02:44:49PM +0100, Andi Kleen wrote:
>
> > > Can you still reproduce this problem?
> > > If not, I'll suggest to apply the patch below which saves a few kB in
> > > lib/string.o .
> >
> > I would prefer to keep it because there is no guarantee in gcc
> > that it always inlines all string functions unless you pass
> > -minline-all-stringops. And with that the code would
> > be bloated much more than the few out of lined fallback
> > string functions.
>
> If I understand your changelog entry correctly, this wasn't the problem
> (the asm string functions are "static inline").
Actually, shouldn't the string functions be "extern inline" then?
That would mean we use the copy from lib/string.c instead of generating
a new copy for each file in which gcc decides not to inline the function.
It would also let you get rid of the IN_STRING_C hack that is needed
to avoid the clash of the "static inline" and the "extern" version.
Arnd <><
On Tue, Nov 09, 2004 at 06:01:07AM +0100, Andi Kleen wrote:
> > Why doesn't the kernel use -ffreestanding which should prevent all such
> > problems?
>
> Because we want most of these optimizations. Also with -ffreestanding
Do we really want the compiler to silently replace in-kernel functions
with built-ins?
You can still do an explicit
#define strlen __builtin_strlen
if you want to use a gcc built-in function.
> you would need to supply the out of line string functions anyways
> because gcc wouldn't inline them.
At least with gcc 3.4.2 on i386 adding -ffreestanding and your
(i386-specific) IN_STRING_C hack removed compiles fine.
> -Andi
I'm open for examples why this actually doesn't work, but after my
(limited) testin I'd suggest the patch below for inclusion in the next
-mm.
Signed-off-by: Adrian Bunk <[email protected]>
--- linux-2.6.10-rc1-mm4-full-ffreestanding/Makefile.old 2004-11-09 22:27:06.000000000 +0100
+++ linux-2.6.10-rc1-mm4-full-ffreestanding/Makefile 2004-11-09 22:27:47.000000000 +0100
@@ -349,7 +349,8 @@
CPPFLAGS := -D__KERNEL__ $(LINUXINCLUDE)
CFLAGS := -Wall -Wstrict-prototypes -Wno-trigraphs \
- -fno-strict-aliasing -fno-common
+ -fno-strict-aliasing -fno-common \
+ -ffreestanding
AFLAGS := -D__ASSEMBLY__
export VERSION PATCHLEVEL SUBLEVEL EXTRAVERSION LOCALVERSION KERNELRELEASE \
On Wed, 10 Nov 2004, Adrian Bunk wrote:
>
> I'm open for examples why this actually doesn't work, but after my
> (limited) testin I'd suggest the patch below for inclusion in the next
> -mm.
When was -ffreestanding introduced? Iow, it might not work with all
compiler versions.. Apart from that, I think it makes sense.
Linus
On Tue, Nov 09, 2004 at 05:51:41PM -0800, Linus Torvalds wrote:
>
> On Wed, 10 Nov 2004, Adrian Bunk wrote:
> >
> > I'm open for examples why this actually doesn't work, but after my
> > (limited) testin I'd suggest the patch below for inclusion in the next
> > -mm.
>
> When was -ffreestanding introduced? Iow, it might not work with all
> compiler versions.. Apart from that, I think it makes sense.
It's supported in gcc 2.95 (which is the oldest compiler supported by
kernel 2.6).
> Linus
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Tue, Nov 09, 2004 at 02:58:34PM +0100, Arnd Bergmann wrote:
> > On Mon, Nov 08, 2004 at 02:44:49PM +0100, Andi Kleen wrote:
> >
> > > > Can you still reproduce this problem?
> > > > If not, I'll suggest to apply the patch below which saves a few kB in
> > > > lib/string.o .
> > >
> > > I would prefer to keep it because there is no guarantee in gcc
> > > that it always inlines all string functions unless you pass
> > > -minline-all-stringops. And with that the code would
> > > be bloated much more than the few out of lined fallback
> > > string functions.
> >
> > If I understand your changelog entry correctly, this wasn't the problem
> > (the asm string functions are "static inline").
>
> Actually, shouldn't the string functions be "extern inline" then?
> That would mean we use the copy from lib/string.c instead of generating
> a new copy for each file in which gcc decides not to inline the function.
>...
In the kernel, we #define inline to __attribute__((always_inline)) [1]
leaving gcc no room for a decision to not inline it.
> Arnd <><
cu
Adrian
[1] only for gcc >= 3.1, but I don't remember problems with older gcc
versions
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
Linus Torvalds wrote:
>
> On Wed, 10 Nov 2004, Adrian Bunk wrote:
>
>>I'm open for examples why this actually doesn't work, but after my
>>(limited) testin I'd suggest the patch below for inclusion in the next
>>-mm.
>
>
> When was -ffreestanding introduced? Iow, it might not work with all
> compiler versions.. Apart from that, I think it makes sense.
From RH 7.3:
gcc version 2.96 20000731 (Red Hat Linux 7.3 2.96-113)
--
-bill davidsen ([email protected])
"The secret to procrastination is to put things off until the
last possible moment - but no longer" -me