m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Geert Uytterhoeven]

M68k builds with current mainline fail with

    init/built-in.o: In function `kernel_init':
    main.c:(.init.text+0x730): undefined reference to `strlen'

Reverting commit e662e1cfd434aa234b72fbc781f1d70211cb785b

    Author: Cyrill Gorcunov <gorcunov@gmail.com>
    Date:   Mon May 12 14:02:22 2008 -0700

    init: don't lose initcall return values

    There is an ability to lose an initcall return value if it happened with irq
    disabled or imbalanced preemption (and if we debug initcall).

fixes the problem. My first guess is gcc is turning sizeof() into strlen()
again.

Sample build log available at
http://kisskb.ellerman.id.au/kisskb/buildresult/27596/

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Andreas Schwab]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> My first guess is gcc is turning sizeof() into strlen() again.

Definitely not.  sizeof is a compile-time constant, strlen is not.  More
likely the strlen call is embedded in the expansion of strncat.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Cyrill Gorcunov]

On Wed, May 14, 2008 at 12:37 PM, Andreas Schwab <schwab@suse.de> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>
>  > My first guess is gcc is turning sizeof() into strlen() again.
>
>  Definitely not.  sizeof is a compile-time constant, strlen is not.  More
>  likely the strlen call is embedded in the expansion of strncat.
>
>  Andreas.
>
>  --
>  Andreas Schwab, SuSE Labs, schwab@suse.de
>  SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
>  PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
>  "And now for something completely different."
>

I've missed to add

#include <linux/string.h>

sorry, will make the patch today evening (or maybe someone could
make it). Thanks for report!

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Cyrill Gorcunov]

On Wed, May 14, 2008 at 1:57 PM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> On Wed, May 14, 2008 at 12:37 PM, Andreas Schwab <schwab@suse.de> wrote:
>  > Geert Uytterhoeven <geert@linux-m68k.org> writes:
>  >
>  >  > My first guess is gcc is turning sizeof() into strlen() again.
>  >
>  >  Definitely not.  sizeof is a compile-time constant, strlen is not.  More
>  >  likely the strlen call is embedded in the expansion of strncat.
>  >
>  >  Andreas.
>  >
>  >  --
>  >  Andreas Schwab, SuSE Labs, schwab@suse.de
>  >  SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
>  >  PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
>  >  "And now for something completely different."
>  >
>
>  I've missed to add
>
>  #include <linux/string.h>
>
>  sorry, will make the patch today evening (or maybe someone could
>  make it). Thanks for report!
>

hmm, string.h is aready there, so there are two reason for fail I could
imagine at first glance: or gcc does something strange, or the problem
with linking
string.o. Will check it.

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Adrian Bunk]

On Wed, May 14, 2008 at 10:37:57AM +0200, Andreas Schwab wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
> 
> > My first guess is gcc is turning sizeof() into strlen() again.
> 
> Definitely not.  sizeof is a compile-time constant, strlen is not.  More
> likely the strlen call is embedded in the expansion of strncat.

m68k is one of the architectures not using -ffreestanding, so this 
kind of problems is somehow expected...

We could add -ffreestanding on m68k.

Or replace all the strlen stuff in include/asm-m68k/string.h with a 
function prototype, which lets gcc choose itself whether it wants to use 
the builtin or the version from lib/string.c, and makes an out-of-line 
version of the function available for caes like the one we face here.

> Andreas.

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

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Andreas Schwab]

Adrian Bunk <bunk@kernel.org> writes:

> Or replace all the strlen stuff in include/asm-m68k/string.h with a 

I don't think the strlen macros has any influence in this case (it
already uses __builtin_strlen anyway).  I'd rather guess that gcc is
expanding strncat internally to something involving strlen with
non-constant argument, although I cannot see how that can happen from a
quick look.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Cyrill Gorcunov]

[Andreas Schwab - Wed, May 14, 2008 at 04:28:24PM +0200]
| Adrian Bunk <bunk@kernel.org> writes:
| 
| > Or replace all the strlen stuff in include/asm-m68k/string.h with a 
| 
| I don't think the strlen macros has any influence in this case (it
| already uses __builtin_strlen anyway).  I'd rather guess that gcc is
| expanding strncat internally to something involving strlen with
| non-constant argument, although I cannot see how that can happen from a
| quick look.
| 
| Andreas.
| 
| -- 
| Andreas Schwab, SuSE Labs, schwab@suse.de
| SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
| PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
| "And now for something completely different."
| 

I think it would help to see ..tmp_vmlinux1.cmd to ensure for inclusion
of lib/lib.a. strlen was there without my patch as Andreas already pointed,
I've just added strncat wich is coming from lib/string.o for this arch.

		- Cyrill -

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Cyrill Gorcunov]

[Andreas Schwab - Wed, May 14, 2008 at 04:28:24PM +0200]
| Adrian Bunk <bunk@kernel.org> writes:
| 
| > Or replace all the strlen stuff in include/asm-m68k/string.h with a 
| 
| I don't think the strlen macros has any influence in this case (it
| already uses __builtin_strlen anyway).  I'd rather guess that gcc is
| expanding strncat internally to something involving strlen with
| non-constant argument, although I cannot see how that can happen from a
| quick look.
| 
| Andreas.
| 
| -- 
| Andreas Schwab, SuSE Labs, schwab@suse.de
| SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
| PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
| "And now for something completely different."
| 

actually, i can rewrite it using memcpy if strncat causes such a problems

		- Cyrill -

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Roman Zippel]

Hi,

On Wed, 14 May 2008, Geert Uytterhoeven wrote:

> M68k builds with current mainline fail with
> 
>     init/built-in.o: In function `kernel_init':
>     main.c:(.init.text+0x730): undefined reference to `strlen'

If it's generated by gcc, we can simply provide an uninlined version of it 
in arch/m68k/lib/string.c similiar to strcpy (and there is already a 
__kernel_strlen).

bye, Roman

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Andreas Schwab]

Cyrill Gorcunov <gorcunov@gmail.com> writes:

> I think it would help to see ..tmp_vmlinux1.cmd to ensure for inclusion
> of lib/lib.a. strlen was there without my patch as Andreas already pointed,
> I've just added strncat wich is coming from lib/string.o for this arch.

Actually the way strncat is used here is broken anyway, it does not
prevent array overrun.  The third argument of strncat only limits the
amount of characters copied, without taking into account the length of
the string already in the buffer.  Consequently gcc has optimized the
call to strncat into a simple call to strcat, since none of the copied
strings are longer than sizeof(msgbuf).  This strcat call is then
expanded to include a call to strlen.

So a better fix would probably be to make msgbuf big enough and use
strcat instead.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Cyrill Gorcunov]

[Andreas Schwab - Wed, May 14, 2008 at 04:55:37PM +0200]
| Cyrill Gorcunov <gorcunov@gmail.com> writes:
| 
| > I think it would help to see ..tmp_vmlinux1.cmd to ensure for inclusion
| > of lib/lib.a. strlen was there without my patch as Andreas already pointed,
| > I've just added strncat wich is coming from lib/string.o for this arch.
| 
| Actually the way strncat is used here is broken anyway, it does not
| prevent array overrun.  The third argument of strncat only limits the
| amount of characters copied, without taking into account the length of
| the string already in the buffer.  Consequently gcc has optimized the
| call to strncat into a simple call to strcat, since none of the copied
| strings are longer than sizeof(msgbuf).  This strcat call is then
| expanded to include a call to strlen.
| 
| So a better fix would probably be to make msgbuf big enough and use
| strcat instead.
| 
| Andreas.
| 
| -- 
| Andreas Schwab, SuSE Labs, schwab@suse.de
| SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
| PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
| "And now for something completely different."
| 

Thanks Andreas, I'll fix it.

		- Cyrill -

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Cyrill Gorcunov]

[Andreas Schwab - Wed, May 14, 2008 at 04:55:37PM +0200]
| Cyrill Gorcunov <gorcunov@gmail.com> writes:
| 
| > I think it would help to see ..tmp_vmlinux1.cmd to ensure for inclusion
| > of lib/lib.a. strlen was there without my patch as Andreas already pointed,
| > I've just added strncat wich is coming from lib/string.o for this arch.
| 
| Actually the way strncat is used here is broken anyway, it does not
| prevent array overrun.  The third argument of strncat only limits the
| amount of characters copied, without taking into account the length of
| the string already in the buffer.  Consequently gcc has optimized the
| call to strncat into a simple call to strcat, since none of the copied
| strings are longer than sizeof(msgbuf).  This strcat call is then
| expanded to include a call to strlen.
| 
| So a better fix would probably be to make msgbuf big enough and use
| strcat instead.
| 
| Andreas.
| 
| 

Andreas, could you test/review the following patch
(what is worse - I *have* introduced buffer overflow with my
patch, so I think we should fix it asap)
---

Index: linux-2.6.git/init/main.c
===================================================================
--- linux-2.6.git.orig/init/main.c	2008-05-14 17:55:10.000000000 +0400
+++ linux-2.6.git/init/main.c	2008-05-14 19:11:18.000000000 +0400
@@ -702,7 +702,7 @@ static void __init do_initcalls(void)
 
 	for (call = __initcall_start; call < __initcall_end; call++) {
 		ktime_t t0, t1, delta;
-		char msgbuf[40];
+		char msgbuf[64];
 		int result;
 
 		if (initcall_debug) {
@@ -729,11 +729,11 @@ static void __init do_initcalls(void)
 			sprintf(msgbuf, "error code %d ", result);
 
 		if (preempt_count() != count) {
-			strncat(msgbuf, "preemption imbalance ", sizeof(msgbuf));
+			strcat(msgbuf, "preemption imbalance ");
 			preempt_count() = count;
 		}
 		if (irqs_disabled()) {
-			strncat(msgbuf, "disabled interrupts ", sizeof(msgbuf));
+			strcat(msgbuf, "disabled interrupts ");
 			local_irq_enable();
 		}
 		if (msgbuf[0]) {

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Andreas Schwab]

Cyrill Gorcunov <gorcunov@gmail.com> writes:

> Andreas, could you test/review the following patch
> (what is worse - I *have* introduced buffer overflow with my
> patch, so I think we should fix it asap)

I've compiled the file with the various cross compilers I have laying
around here, no reference to strlen any more.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Adrian Bunk]

On Wed, May 14, 2008 at 04:28:24PM +0200, Andreas Schwab wrote:
> Adrian Bunk <bunk@kernel.org> writes:
> 
> > Or replace all the strlen stuff in include/asm-m68k/string.h with a 
> 
> I don't think the strlen macros has any influence in this case (it
> already uses __builtin_strlen anyway).

The problem is that it sets __HAVE_ARCH_STRLEN, and therefore the 
out-of-line function in lib/string.c is not built.

That breaks when gcc replaces a call to a different function with a call 
to strlen().

> I'd rather guess that gcc is
> expanding strncat internally to something involving strlen with
> non-constant argument,

We agree on this one.

> although I cannot see how that can happen from a
> quick look.

It isn't the first time we have these problems in the kernel...

> Andreas.

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

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Cyrill Gorcunov]

[Andreas Schwab - Wed, May 14, 2008 at 05:27:35PM +0200]
| Cyrill Gorcunov <gorcunov@gmail.com> writes:
| 
| > Andreas, could you test/review the following patch
| > (what is worse - I *have* introduced buffer overflow with my
| > patch, so I think we should fix it asap)
| 
| I've compiled the file with the various cross compilers I have laying
| around here, no reference to strlen any more.
| 
| Andreas.
| 
| -- 
| Andreas Schwab, SuSE Labs, schwab@suse.de
| SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
| PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
| "And now for something completely different."
| 

Thanks a lot, Andreas! I'll sent the patch to Linus, 'case having potential buffer overflow
is not good for mainline

		- Cyrill -

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Cyrill Gorcunov]

[Adrian Bunk - Wed, May 14, 2008 at 06:30:59PM +0300]
| On Wed, May 14, 2008 at 04:28:24PM +0200, Andreas Schwab wrote:
| > Adrian Bunk <bunk@kernel.org> writes:
| > 
| > > Or replace all the strlen stuff in include/asm-m68k/string.h with a 
| > 
| > I don't think the strlen macros has any influence in this case (it
| > already uses __builtin_strlen anyway).
| 
| The problem is that it sets __HAVE_ARCH_STRLEN, and therefore the 
| out-of-line function in lib/string.c is not built.
| 
| That breaks when gcc replaces a call to a different function with a call 
| to strlen().
| 
| > I'd rather guess that gcc is
| > expanding strncat internally to something involving strlen with
| > non-constant argument,
| 
| We agree on this one.
| 
| > although I cannot see how that can happen from a
| > quick look.
| 
| It isn't the first time we have these problems in the kernel...
| 
| > Andreas.
| 
| 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
| 
| 

Adrian, I've just sent the updated patch, and accidentally forget
to CC you - please take a look on http://lkml.org/lkml/2008/5/14/267

		- Cyrill -

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Adrian Bunk]

On Wed, May 14, 2008 at 07:48:01PM +0400, Cyrill Gorcunov wrote:
> [Adrian Bunk - Wed, May 14, 2008 at 06:30:59PM +0300]
> | On Wed, May 14, 2008 at 04:28:24PM +0200, Andreas Schwab wrote:
> | > Adrian Bunk <bunk@kernel.org> writes:
> | > 
> | > > Or replace all the strlen stuff in include/asm-m68k/string.h with a 
> | > 
> | > I don't think the strlen macros has any influence in this case (it
> | > already uses __builtin_strlen anyway).
> | 
> | The problem is that it sets __HAVE_ARCH_STRLEN, and therefore the 
> | out-of-line function in lib/string.c is not built.
> | 
> | That breaks when gcc replaces a call to a different function with a call 
> | to strlen().
> | 
> | > I'd rather guess that gcc is
> | > expanding strncat internally to something involving strlen with
> | > non-constant argument,
> | 
> | We agree on this one.
> | 
> | > although I cannot see how that can happen from a
> | > quick look.
> | 
> | It isn't the first time we have these problems in the kernel...
> | 
> | > Andreas.
> | 
> | cu
> | Adrian
> 
> Adrian, I've just sent the updated patch, and accidentally forget
> to CC you - please take a look on http://lkml.org/lkml/2008/5/14/267

It removes the strncat() calls and should therefore remove the current 
compile error.

But the underlying problem that actually causes the compile error is not 
fixed and might occur again in other places (but that's nothing you 
have to worry about).

> 		- Cyrill -

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

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Cyrill Gorcunov]

[Adrian Bunk - Wed, May 14, 2008 at 06:56:11PM +0300]
| On Wed, May 14, 2008 at 07:48:01PM +0400, Cyrill Gorcunov wrote:
| > [Adrian Bunk - Wed, May 14, 2008 at 06:30:59PM +0300]
| > | On Wed, May 14, 2008 at 04:28:24PM +0200, Andreas Schwab wrote:
| > | > Adrian Bunk <bunk@kernel.org> writes:
| > | > 
| > | > > Or replace all the strlen stuff in include/asm-m68k/string.h with a 
| > | > 
| > | > I don't think the strlen macros has any influence in this case (it
| > | > already uses __builtin_strlen anyway).
| > | 
| > | The problem is that it sets __HAVE_ARCH_STRLEN, and therefore the 
| > | out-of-line function in lib/string.c is not built.
| > | 
| > | That breaks when gcc replaces a call to a different function with a call 
| > | to strlen().
| > | 
| > | > I'd rather guess that gcc is
| > | > expanding strncat internally to something involving strlen with
| > | > non-constant argument,
| > | 
| > | We agree on this one.
| > | 
| > | > although I cannot see how that can happen from a
| > | > quick look.
| > | 
| > | It isn't the first time we have these problems in the kernel...
| > | 
| > | > Andreas.
| > | 
| > | cu
| > | Adrian
| > 
| > Adrian, I've just sent the updated patch, and accidentally forget
| > to CC you - please take a look on http://lkml.org/lkml/2008/5/14/267
| 
| It removes the strncat() calls and should therefore remove the current 
| compile error.
| 
| But the underlying problem that actually causes the compile error is not 
| fixed and might occur again in other places (but that's nothing you 
| have to worry about).
| 
| > 		- Cyrill -
| 
| 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
| 

Yes, this problem is still here but I was really frowned 'cause
it's my patch introduced potential memory overflow :(

		- Cyrill -

Re: m68k: main.c:(.init.text+0x730): undefined reference to `strlen'

[Geert Uytterhoeven]

On Wed, 14 May 2008, Andreas Schwab wrote:
> Cyrill Gorcunov <gorcunov@gmail.com> writes:
> > Andreas, could you test/review the following patch
> > (what is worse - I *have* introduced buffer overflow with my
> > patch, so I think we should fix it asap)
> 
> I've compiled the file with the various cross compilers I have laying
> around here, no reference to strlen any more.

Same here. Builds and runs fine (on ARAnyM). Thanks!

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds