Without -ffreestanding, gcc may replace calls to standard C library
functions by calls to other standard C library functions and/or inline
code. This may cause link errors if the replacement code calls a standard
C library function that's implemented as a macro in the kernel.
E.g. gcc turned
strncat(name, "%d", 2);
into a call to strlen() and a 16-bit store, causing a link failure, as
arch/m68k/include/asm/string.h provides strlen() using a macro:
ERROR: "strlen" [net/ipv4/ip_tunnel.ko] undefined!
In addition, this saves ca. 64 bytes of text on a typical kernel build.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
http://kisskb.ellerman.id.au/kisskb/buildresult/8462108/
QUESTION: Should we re-enable -ffreestanding in the main Makefile instead?
It was removed in
commit 6edfba1b33c701108717f4e036320fc39abe1912
Author: Andi Kleen <[email protected]>
Date: Sat Mar 25 16:29:49 2006 +0100
[PATCH] x86_64: Don't define string functions to builtin
gcc should handle this anyways, and it causes problems when
sprintf is turned into strcpy by gcc behind our backs and
the C fallback version of strcpy is actually defining __builtin_strcpy
Then drop -ffreestanding from the main Makefile because it isn't
needed anymore and implies -fno-builtin, which is wrong now.
(it was only added for x86-64, so dropping it should be safe)
Noticed by Roman Zippel
Cc: Roman Zippel <[email protected]>
Signed-off-by: Andi Kleen <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Subsequently, it got re-enabled for mips, sh, x86-32, um/x86, xtensa, and
score.
arch/m68k/Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/m68k/Makefile b/arch/m68k/Makefile
index 2f02acf..8e06a38 100644
--- a/arch/m68k/Makefile
+++ b/arch/m68k/Makefile
@@ -58,7 +58,7 @@ cpuflags-$(CONFIG_M5206e) := $(call cc-option,-mcpu=5206e,-m5200)
cpuflags-$(CONFIG_M5206) := $(call cc-option,-mcpu=5206,-m5200)
KBUILD_AFLAGS += $(cpuflags-y)
-KBUILD_CFLAGS += $(cpuflags-y) -pipe
+KBUILD_CFLAGS += $(cpuflags-y) -pipe -ffreestanding
ifdef CONFIG_MMU
# without -fno-strength-reduce the 53c7xx.c driver fails ;-(
KBUILD_CFLAGS += -fno-strength-reduce -ffixed-a2
--
1.7.0.4
(Added Andi to CC)
On 10.4.2013 11:24, Geert Uytterhoeven wrote:
[...]
> E.g. gcc turned
>
> strncat(name, "%d", 2);
>
> into a call to strlen() and a 16-bit store, causing a link failure, as
> arch/m68k/include/asm/string.h provides strlen() using a macro:
>
> ERROR: "strlen" [net/ipv4/ip_tunnel.ko] undefined!
[...]
> QUESTION: Should we re-enable -ffreestanding in the main Makefile instead?
>
> It was removed in
>
> commit 6edfba1b33c701108717f4e036320fc39abe1912
> Author: Andi Kleen <[email protected]>
> Date: Sat Mar 25 16:29:49 2006 +0100
>
> [PATCH] x86_64: Don't define string functions to builtin
My understanding is, that with -fnobuiltin, the compiler is not allowed
to make assumptions about functions if it does not see their definition,
even if they resemble standard functions. E.g. on x86_64, strlen() is
out-of-line, so gcc would have to assume, that strcmp() has side
effects. How about just naming the m68k inline function 'strlen'?
Michal
On Wed, Apr 10, 2013 at 12:18 PM, Michal Marek <[email protected]> wrote:
> (Added Andi to CC)
>
> On 10.4.2013 11:24, Geert Uytterhoeven wrote:
> [...]
>> E.g. gcc turned
>>
>> strncat(name, "%d", 2);
>>
>> into a call to strlen() and a 16-bit store, causing a link failure, as
>> arch/m68k/include/asm/string.h provides strlen() using a macro:
>>
>> ERROR: "strlen" [net/ipv4/ip_tunnel.ko] undefined!
> [...]
>> QUESTION: Should we re-enable -ffreestanding in the main Makefile instead?
>>
>> It was removed in
>>
>> commit 6edfba1b33c701108717f4e036320fc39abe1912
>> Author: Andi Kleen <[email protected]>
>> Date: Sat Mar 25 16:29:49 2006 +0100
>>
>> [PATCH] x86_64: Don't define string functions to builtin
>
> My understanding is, that with -fnobuiltin, the compiler is not allowed
> to make assumptions about functions if it does not see their definition,
> even if they resemble standard functions. E.g. on x86_64, strlen() is
> out-of-line, so gcc would have to assume, that strcmp() has side
> effects. How about just naming the m68k inline function 'strlen'?
Having an inline function named "strlen" is not sufficient, as it needs an
(exported) symbol named "strlen" at link time or module load time.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Wed, Apr 10, 2013 at 12:59:44PM +0200, Geert Uytterhoeven wrote:
> On Wed, Apr 10, 2013 at 12:18 PM, Michal Marek <[email protected]> wrote:
> > On 10.4.2013 11:24, Geert Uytterhoeven wrote:
> > My understanding is, that with -fnobuiltin, the compiler is not allowed
> > to make assumptions about functions if it does not see their definition,
> > even if they resemble standard functions. E.g. on x86_64, strlen() is
> > out-of-line, so gcc would have to assume, that strcmp() has side
> > effects. How about just naming the m68k inline function 'strlen'?
>
> Having an inline function named "strlen" is not sufficient, as it needs an
> (exported) symbol named "strlen" at link time or module load time.
Really? MIPS and Xtensa both have an inline version of strcpy() and no
exported strcpy symbol. An I would naively assume, that if the compiler
sees a static inline definition of a function, then it will not generate
a function call. Although, the C standard does require the standard
library functions be available as external functions, so the compiler
would not be necessarily wrong :-/. Anyway, can you try if this
(untested) patch fixes the issue?
Thanks,
Michal
>From ac9861ba0bf4257f14d7f7db4b9eb60311986049 Mon Sep 17 00:00:00 2001
From: Michal Marek <[email protected]>
Date: Wed, 10 Apr 2013 13:58:55 +0200
Subject: [PATCH] m68k: Do not define string functions as macros
Implement the functions under their real names, so that the compiler can
emit calls to them.
Reported-by: Geert Uytterhoeven <[email protected]>
Signed-off-by: Michal Marek <[email protected]>
---
arch/m68k/include/asm/string.h | 22 ++++++++--------------
arch/m68k/lib/Makefile | 2 +-
arch/m68k/lib/string.c | 22 ----------------------
3 files changed, 9 insertions(+), 37 deletions(-)
delete mode 100644 arch/m68k/lib/string.c
diff --git a/arch/m68k/include/asm/string.h b/arch/m68k/include/asm/string.h
index 3219845..dcd80ae 100644
--- a/arch/m68k/include/asm/string.h
+++ b/arch/m68k/include/asm/string.h
@@ -4,7 +4,7 @@
#include <linux/types.h>
#include <linux/compiler.h>
-static inline size_t __kernel_strlen(const char *s)
+static inline size_t strlen(const char *s)
{
const char *sc;
@@ -13,7 +13,7 @@ static inline size_t __kernel_strlen(const char *s)
return sc - s - 1;
}
-static inline char *__kernel_strcpy(char *dest, const char *src)
+static inline char *strcpy(char *dest, const char *src)
{
char *xdest = dest;
@@ -25,12 +25,10 @@ static inline char *__kernel_strcpy(char *dest, const char *src)
return xdest;
}
-#ifndef __IN_STRING_C
-
#define __HAVE_ARCH_STRLEN
#define strlen(s) (__builtin_constant_p(s) ? \
__builtin_strlen(s) : \
- __kernel_strlen(s))
+ strlen(s))
#define __HAVE_ARCH_STRNLEN
static inline size_t strnlen(const char *s, size_t count)
@@ -53,9 +51,7 @@ static inline size_t strnlen(const char *s, size_t count)
#define strcpy(d, s) (__builtin_constant_p(s) && \
__builtin_strlen(s) <= 32 ? \
__builtin_strcpy(d, s) : \
- __kernel_strcpy(d, s))
-#else
-#define strcpy(d, s) __kernel_strcpy(d, s)
+ strcpy(d, s))
#endif
#define __HAVE_ARCH_STRNCPY
@@ -76,10 +72,10 @@ static inline char *strncpy(char *dest, const char *src, size_t n)
}
#define __HAVE_ARCH_STRCAT
-#define strcat(d, s) ({ \
- char *__d = (d); \
- strcpy(__d + strlen(__d), (s)); \
-})
+static inline char * strcat(char *dest, const char *src)
+{
+ return strcpy(dest + strlen(dest), src);
+}
#ifndef CONFIG_COLDFIRE
#define __HAVE_ARCH_STRCMP
@@ -114,6 +110,4 @@ extern void *memset(void *, int, __kernel_size_t);
extern void *memcpy(void *, const void *, __kernel_size_t);
#define memcpy(d, s, n) __builtin_memcpy(d, s, n)
-#endif
-
#endif /* _M68K_STRING_H_ */
diff --git a/arch/m68k/lib/Makefile b/arch/m68k/lib/Makefile
index a9d782d..fcd8eb1 100644
--- a/arch/m68k/lib/Makefile
+++ b/arch/m68k/lib/Makefile
@@ -6,7 +6,7 @@
lib-y := ashldi3.o ashrdi3.o lshrdi3.o muldi3.o \
memcpy.o memset.o memmove.o
-lib-$(CONFIG_MMU) += string.o uaccess.o
+lib-$(CONFIG_MMU) += uaccess.o
lib-$(CONFIG_CPU_HAS_NO_MULDIV64) += mulsi3.o divsi3.o udivsi3.o
lib-$(CONFIG_CPU_HAS_NO_MULDIV64) += modsi3.o umodsi3.o
diff --git a/arch/m68k/lib/string.c b/arch/m68k/lib/string.c
deleted file mode 100644
index b9a57ab..0000000
--- a/arch/m68k/lib/string.c
+++ /dev/null
@@ -1,22 +0,0 @@
-/*
- * This file is subject to the terms and conditions of the GNU General Public
- * License. See the file COPYING in the main directory of this archive
- * for more details.
- */
-
-#define __IN_STRING_C
-
-#include <linux/module.h>
-#include <linux/string.h>
-
-char *strcpy(char *dest, const char *src)
-{
- return __kernel_strcpy(dest, src);
-}
-EXPORT_SYMBOL(strcpy);
-
-char *strcat(char *dest, const char *src)
-{
- return __kernel_strcpy(dest + __kernel_strlen(dest), src);
-}
-EXPORT_SYMBOL(strcat);
--
1.7.10.4
Hi Michal,
On Wed, Apr 10, 2013 at 2:26 PM, Michal Marek <[email protected]> wrote:
> On Wed, Apr 10, 2013 at 12:59:44PM +0200, Geert Uytterhoeven wrote:
>> On Wed, Apr 10, 2013 at 12:18 PM, Michal Marek <[email protected]> wrote:
>> > On 10.4.2013 11:24, Geert Uytterhoeven wrote:
>> > My understanding is, that with -fnobuiltin, the compiler is not allowed
>> > to make assumptions about functions if it does not see their definition,
>> > even if they resemble standard functions. E.g. on x86_64, strlen() is
>> > out-of-line, so gcc would have to assume, that strcmp() has side
>> > effects. How about just naming the m68k inline function 'strlen'?
>>
>> Having an inline function named "strlen" is not sufficient, as it needs an
>> (exported) symbol named "strlen" at link time or module load time.
>
> Really? MIPS and Xtensa both have an inline version of strcpy() and no
> exported strcpy symbol. An I would naively assume, that if the compiler
> sees a static inline definition of a function, then it will not generate
> a function call. Although, the C standard does require the standard
> library functions be available as external functions, so the compiler
> would not be necessarily wrong :-/. Anyway, can you try if this
> (untested) patch fixes the issue?
Thanks for the patch, but modpost still fails with
ERROR: "strlen" [net/ipv4/ip_tunnel.ko] undefined!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Wed, Apr 10, 2013 at 04:19:14PM +0200, Geert Uytterhoeven wrote:
> Thanks for the patch, but modpost still fails with
>
> ERROR: "strlen" [net/ipv4/ip_tunnel.ko] undefined!
OK OK, I am convinced. Then how about this? If you prefer speed over
size, the the other option is to add
size_t strlen(const char *s)
{
return __kernel_strlen(s);
}
EXPORT_SYMBOL(strlen);
to arch/m68k/lib/string.c. On a related note, arch/m68k/lib/string.c is
built only in the CONFIG_MMU case, whereas asm/string.h is shared.
Michal
>From 1b89666d246bcc2bfc4c9e1bdfd1dbd1dafe2e2f Mon Sep 17 00:00:00 2001
From: Michal Marek <[email protected]>
Date: Wed, 10 Apr 2013 16:45:21 +0200
Subject: [PATCH] m68k: Remove inline strlen() implementation
GCC can replace a strncat() call with constant second argument into a
strlen + store, which results in a link error:
ERROR: "strlen" [net/ipv4/ip_tunnel.ko] undefined!
The inline function is a simple for loop in C. Other architectures
either use an asm optimized variant, or use the generic function from
lib/string.c.
Reported-by: Geert Uytterhoeven <[email protected]>
Signed-off-by: Michal Marek <[email protected]>
---
arch/m68k/include/asm/string.h | 14 --------------
arch/m68k/lib/string.c | 2 +-
2 files changed, 1 insertion(+), 15 deletions(-)
diff --git a/arch/m68k/include/asm/string.h b/arch/m68k/include/asm/string.h
index 3219845..9aea9f1 100644
--- a/arch/m68k/include/asm/string.h
+++ b/arch/m68k/include/asm/string.h
@@ -4,15 +4,6 @@
#include <linux/types.h>
#include <linux/compiler.h>
-static inline size_t __kernel_strlen(const char *s)
-{
- const char *sc;
-
- for (sc = s; *sc++; )
- ;
- return sc - s - 1;
-}
-
static inline char *__kernel_strcpy(char *dest, const char *src)
{
char *xdest = dest;
@@ -27,11 +18,6 @@ static inline char *__kernel_strcpy(char *dest, const char *src)
#ifndef __IN_STRING_C
-#define __HAVE_ARCH_STRLEN
-#define strlen(s) (__builtin_constant_p(s) ? \
- __builtin_strlen(s) : \
- __kernel_strlen(s))
-
#define __HAVE_ARCH_STRNLEN
static inline size_t strnlen(const char *s, size_t count)
{
diff --git a/arch/m68k/lib/string.c b/arch/m68k/lib/string.c
index b9a57ab..4d61fa8 100644
--- a/arch/m68k/lib/string.c
+++ b/arch/m68k/lib/string.c
@@ -17,6 +17,6 @@ EXPORT_SYMBOL(strcpy);
char *strcat(char *dest, const char *src)
{
- return __kernel_strcpy(dest + __kernel_strlen(dest), src);
+ return __kernel_strcpy(dest + strlen(dest), src);
}
EXPORT_SYMBOL(strcat);
--
1.7.10.4
From: Geert Uytterhoeven <[email protected]>
Date: Wed, 10 Apr 2013 11:24:39 +0200
> Without -ffreestanding, gcc may replace calls to standard C library
> functions by calls to other standard C library functions and/or inline
> code. This may cause link errors if the replacement code calls a standard
> C library function that's implemented as a macro in the kernel.
I think you should instead implement the library functions that gcc might
emit during a build.
> My understanding is, that with -fnobuiltin, the compiler is not allowed
> to make assumptions about functions if it does not see their definition,
> even if they resemble standard functions. E.g. on x86_64, strlen() is
> out-of-line, so gcc would have to assume, that strcmp() has side
> effects. How about just naming the m68k inline function 'strlen'?
You should always supply an out of line fallback version with
the standard name. The easiest way is to define the right
define so that lib/string.c does it.
-Andi
On Wed, Apr 10, 2013 at 5:22 PM, Michal Marek <[email protected]> wrote:
> Subject: [PATCH] m68k: Remove inline strlen() implementation
>
> GCC can replace a strncat() call with constant second argument into a
> strlen + store, which results in a link error:
>
> ERROR: "strlen" [net/ipv4/ip_tunnel.ko] undefined!
>
> The inline function is a simple for loop in C. Other architectures
> either use an asm optimized variant, or use the generic function from
> lib/string.c.
>
> Reported-by: Geert Uytterhoeven <[email protected]>
> Signed-off-by: Michal Marek <[email protected]>
Thanks, this one works. Let me do some measurements...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Thu, Apr 11, 2013 at 8:04 PM, Geert Uytterhoeven
<[email protected]> wrote:
> On Wed, Apr 10, 2013 at 5:22 PM, Michal Marek <[email protected]> wrote:
>> Subject: [PATCH] m68k: Remove inline strlen() implementation
>>
>> GCC can replace a strncat() call with constant second argument into a
>> strlen + store, which results in a link error:
>>
>> ERROR: "strlen" [net/ipv4/ip_tunnel.ko] undefined!
>>
>> The inline function is a simple for loop in C. Other architectures
>> either use an asm optimized variant, or use the generic function from
>> lib/string.c.
>>
>> Reported-by: Geert Uytterhoeven <[email protected]>
>> Signed-off-by: Michal Marek <[email protected]>
>
> Thanks, this one works. Let me do some measurements...
No visible impact on size for a typical build; the increase in .text
is offset by
the decrease in .data:
- .text : 0x00001000 - 0x002aab64 (2727 KiB)
- .data : 0x002ad938 - 0x00392148 ( 915 KiB)
+ .text : 0x00001000 - 0x002aac74 (2728 KiB)
+ .data : 0x002ada48 - 0x00392148 ( 914 KiB)
On Wed, Apr 10, 2013 at 5:22 PM, Michal Marek <[email protected]> wrote:
> On Wed, Apr 10, 2013 at 04:19:14PM +0200, Geert Uytterhoeven wrote:
>> Thanks for the patch, but modpost still fails with
>>
>> ERROR: "strlen" [net/ipv4/ip_tunnel.ko] undefined!
>
> OK OK, I am convinced. Then how about this? If you prefer speed over
> size, the the other option is to add
>
> size_t strlen(const char *s)
> {
> return __kernel_strlen(s);
> }
> EXPORT_SYMBOL(strlen);
>
> to arch/m68k/lib/string.c.
Interestingly, this also doesn't work. It turns out none of the
symbols defined in
this file (currently strcpy and strcat) end up in vmlinux.
As we've been stripping code from string.c, we've been left with just
a few symbols
that are typically never referenced, so the whole string.o in
arch/m68k/lib/lib.a
is not included in the final vmlinux.
So more stringectomy to do...
> On a related note, arch/m68k/lib/string.c is
> built only in the CONFIG_MMU case, whereas asm/string.h is shared.
So due to the above, this difference is mostly moot ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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