Hi Geert,
I am seeing a few errors when cross-building m68k on x86_64, using the
toolchain at https://mirrors.edge.kernel.org/pub/tools/crosstool/ (thanks, Arnd).
(so this is gcc 8.1.0)
block/partitions/ldm.o: In function `ldm_partition':
ldm.c:(.text+0x1900): undefined reference to `strcmp'
ldm.c:(.text+0x1964): undefined reference to `strcmp'
drivers/rtc/rtc-proc.o: In function `is_rtc_hctosys':
rtc-proc.c:(.text+0x290): undefined reference to `strcmp'
drivers/watchdog/watchdog_pretimeout.o: In function `watchdog_register_governor':
(.text+0x142): undefined reference to `strcmp'
Adding #include <linux/string.h> does not help.
Is this a toolchain problem or drivers or something else?
help?
thanks,
--
~Randy
On Thu, 19 Jul 2018, Randy Dunlap wrote:
> Hi Geert,
>
> I am seeing a few errors when cross-building m68k on x86_64, using the
> toolchain at https://mirrors.edge.kernel.org/pub/tools/crosstool/
> (thanks, Arnd). (so this is gcc 8.1.0)
>
> block/partitions/ldm.o: In function `ldm_partition':
> ldm.c:(.text+0x1900): undefined reference to `strcmp'
> ldm.c:(.text+0x1964): undefined reference to `strcmp'
> drivers/rtc/rtc-proc.o: In function `is_rtc_hctosys':
> rtc-proc.c:(.text+0x290): undefined reference to `strcmp'
> drivers/watchdog/watchdog_pretimeout.o: In function `watchdog_register_governor':
> (.text+0x142): undefined reference to `strcmp'
>
>
> Adding #include <linux/string.h> does not help.
>
> Is this a toolchain problem or drivers or something else?
>
This gcc build was apparently configured like so:
/home/arnd/git/gcc/configure --target=m68k-linux --enable-targets=all
--prefix=/home/arnd/cross/x86_64/gcc-8.1.0-nolibc/m68k-linux
--enable-languages=c --without-headers --disable-bootstrap --disable-nls
--disable-threads --disable-shared --disable-libmudflap --disable-libssp
--disable-libgomp --disable-decimal-float --disable-libquadmath
--disable-libatomic --disable-libcc1 --disable-libmpx
--enable-checking=release
In my own cross toolchain builds strcmp comes from glibc but this
toolchain has no libc at all.
> help?
>
Linux will use the strcmp in lib/string.c unless __HAVE_ARCH_STRCMP is
defined in the arch headers. Grep suggests that m68k, mips, x86, xtensa,
arc, sh, arm64, s390 all define that macro. But maybe you could just patch
out that definition for build testing.
--
> thanks,
>
On Jul 19 2018, Randy Dunlap <[email protected]> wrote:
> block/partitions/ldm.o: In function `ldm_partition':
> ldm.c:(.text+0x1900): undefined reference to `strcmp'
> ldm.c:(.text+0x1964): undefined reference to `strcmp'
> drivers/rtc/rtc-proc.o: In function `is_rtc_hctosys':
> rtc-proc.c:(.text+0x290): undefined reference to `strcmp'
> drivers/watchdog/watchdog_pretimeout.o: In function `watchdog_register_governor':
> (.text+0x142): undefined reference to `strcmp'
GCC has optimized strncmp to strcmp, but at a stage where macros are no
longer available. I think the right fix is to use strcmp directly,
since strncmp doesn't make sense here.
Andreas.
--
Andreas Schwab, [email protected]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
On 07/19/2018 10:17 PM, Finn Thain wrote:
> On Thu, 19 Jul 2018, Randy Dunlap wrote:
>
>> Hi Geert,
>>
>> I am seeing a few errors when cross-building m68k on x86_64, using the
>> toolchain at https://mirrors.edge.kernel.org/pub/tools/crosstool/
>> (thanks, Arnd). (so this is gcc 8.1.0)
>>
>> block/partitions/ldm.o: In function `ldm_partition':
>> ldm.c:(.text+0x1900): undefined reference to `strcmp'
>> ldm.c:(.text+0x1964): undefined reference to `strcmp'
>> drivers/rtc/rtc-proc.o: In function `is_rtc_hctosys':
>> rtc-proc.c:(.text+0x290): undefined reference to `strcmp'
>> drivers/watchdog/watchdog_pretimeout.o: In function `watchdog_register_governor':
>> (.text+0x142): undefined reference to `strcmp'
>>
>>
>> Adding #include <linux/string.h> does not help.
>>
>> Is this a toolchain problem or drivers or something else?
>>
>
> This gcc build was apparently configured like so:
>
> /home/arnd/git/gcc/configure --target=m68k-linux --enable-targets=all
> --prefix=/home/arnd/cross/x86_64/gcc-8.1.0-nolibc/m68k-linux
> --enable-languages=c --without-headers --disable-bootstrap --disable-nls
> --disable-threads --disable-shared --disable-libmudflap --disable-libssp
> --disable-libgomp --disable-decimal-float --disable-libquadmath
> --disable-libatomic --disable-libcc1 --disable-libmpx
> --enable-checking=release
>
> In my own cross toolchain builds strcmp comes from glibc but this
> toolchain has no libc at all.
>
>> help?
>>
>
> Linux will use the strcmp in lib/string.c unless __HAVE_ARCH_STRCMP is
> defined in the arch headers. Grep suggests that m68k, mips, x86, xtensa,
> arc, sh, arm64, s390 all define that macro. But maybe you could just patch
> out that definition for build testing.
Sure, that works. Thanks.
--
~Randy
On 07/20/2018 12:20 AM, Andreas Schwab wrote:
> On Jul 19 2018, Randy Dunlap <[email protected]> wrote:
>
>> block/partitions/ldm.o: In function `ldm_partition':
>> ldm.c:(.text+0x1900): undefined reference to `strcmp'
>> ldm.c:(.text+0x1964): undefined reference to `strcmp'
>> drivers/rtc/rtc-proc.o: In function `is_rtc_hctosys':
>> rtc-proc.c:(.text+0x290): undefined reference to `strcmp'
>> drivers/watchdog/watchdog_pretimeout.o: In function `watchdog_register_governor':
>> (.text+0x142): undefined reference to `strcmp'
>
> GCC has optimized strncmp to strcmp, but at a stage where macros are no
> longer available. I think the right fix is to use strcmp directly,
> since strncmp doesn't make sense here.
Hi Andreas,
I don't see that all of these string compare fields are null-terminated.
How does one convert strncmp() users to strcmp()?
thanks,
--
~Randy
On Mon, 23 Jul 2018, Randy Dunlap wrote:
> On 07/20/2018 12:20 AM, Andreas Schwab wrote:
> > On Jul 19 2018, Randy Dunlap <[email protected]> wrote:
> >
> >> block/partitions/ldm.o: In function `ldm_partition':
> >> ldm.c:(.text+0x1900): undefined reference to `strcmp'
> >> ldm.c:(.text+0x1964): undefined reference to `strcmp'
> >> drivers/rtc/rtc-proc.o: In function `is_rtc_hctosys':
> >> rtc-proc.c:(.text+0x290): undefined reference to `strcmp'
> >> drivers/watchdog/watchdog_pretimeout.o: In function `watchdog_register_governor':
> >> (.text+0x142): undefined reference to `strcmp'
> >
> > GCC has optimized strncmp to strcmp, but at a stage where macros are no
> > longer available. I think the right fix is to use strcmp directly,
> > since strncmp doesn't make sense here.
>
> Hi Andreas,
>
> I don't see that all of these string compare fields are null-terminated.
>
Some of the strncmp calls in ldm.c are null-terminated, some are not.
That would imply that the compiler will emit both strcmp and strncmp
calls.
A strncmp call isn't a problem, because m68k doesn't define
__HAVE_ARCH_STRNCMP and so the one from lib/string.c gets built.
> How does one convert strncmp() users to strcmp()?
>
> thanks,
>
The untested patch below may work. It seems that it may be relevant to
both arc and m68k:
$ diff -u <(egrep -rlw __HAVE_ARCH_STRCMP *) <(egrep -rlw __HAVE_ARCH_STRNCMP *)
--- /dev/fd/63 2018-07-24 14:22:56.180014584 +1000
+++ /dev/fd/62 2018-07-24 14:22:56.180014584 +1000
@@ -1,11 +1,12 @@
arch/mips/include/asm/string.h
arch/x86/lib/string_32.c
arch/x86/include/asm/string_32.h
-arch/m68k/include/asm/string.h
arch/xtensa/include/asm/string.h
arch/sh/include/asm/string_32.h
+arch/powerpc/include/asm/string.h
arch/s390/include/asm/string.h
arch/arm64/include/asm/string.h
-arch/arc/include/asm/string.h
+arch/sparc/include/asm/string.h
+drivers/firmware/efi/libstub/string.c
include/linux/string.h
lib/string.c
How can all those __HAVE_ARCH_FOO feature macros work properly if the
compiler makes assumptions about libc availability? Isn't that assumption
invalidated by -ffreestanding?
Surely the strcmp optimization is only valid when there is a
__builtin_strcmp?
--
diff --git a/block/partitions/ldm.c b/block/partitions/ldm.c
index 0417937dfe99..4b9927344593 100644
--- a/block/partitions/ldm.c
+++ b/block/partitions/ldm.c
@@ -150,8 +150,7 @@ static bool ldm_parse_tocblock (const u8 *data, struct tocblock *toc)
toc->bitmap1_start = get_unaligned_be64(data + 0x2E);
toc->bitmap1_size = get_unaligned_be64(data + 0x36);
- if (strncmp (toc->bitmap1_name, TOC_BITMAP1,
- sizeof (toc->bitmap1_name)) != 0) {
+ if (strcmp(toc->bitmap1_name, TOC_BITMAP1) != 0) {
ldm_crit ("TOCBLOCK's first bitmap is '%s', should be '%s'.",
TOC_BITMAP1, toc->bitmap1_name);
return false;
@@ -160,8 +159,7 @@ static bool ldm_parse_tocblock (const u8 *data, struct tocblock *toc)
toc->bitmap2_name[sizeof (toc->bitmap2_name) - 1] = 0;
toc->bitmap2_start = get_unaligned_be64(data + 0x50);
toc->bitmap2_size = get_unaligned_be64(data + 0x58);
- if (strncmp (toc->bitmap2_name, TOC_BITMAP2,
- sizeof (toc->bitmap2_name)) != 0) {
+ if (strcmp(toc->bitmap2_name, TOC_BITMAP2) != 0) {
ldm_crit ("TOCBLOCK's second bitmap is '%s', should be '%s'.",
TOC_BITMAP2, toc->bitmap2_name);
return false;
diff --git a/drivers/rtc/rtc-proc.c b/drivers/rtc/rtc-proc.c
index a9dd9218fae2..5f9a5a720b4d 100644
--- a/drivers/rtc/rtc-proc.c
+++ b/drivers/rtc/rtc-proc.c
@@ -30,7 +30,7 @@ static bool is_rtc_hctosys(struct rtc_device *rtc)
if (size > NAME_SIZE)
return false;
- return !strncmp(name, CONFIG_RTC_HCTOSYS_DEVICE, NAME_SIZE);
+ return !strcmp(name, CONFIG_RTC_HCTOSYS_DEVICE);
}
#else
static bool is_rtc_hctosys(struct rtc_device *rtc)
diff --git a/drivers/watchdog/watchdog_pretimeout.c b/drivers/watchdog/watchdog_pretimeout.c
index 9db07bfb4334..d4797452b011 100644
--- a/drivers/watchdog/watchdog_pretimeout.c
+++ b/drivers/watchdog/watchdog_pretimeout.c
@@ -136,8 +136,7 @@ int watchdog_register_governor(struct watchdog_governor *gov)
priv->gov = gov;
list_add(&priv->entry, &governor_list);
- if (!strncmp(gov->name, WATCHDOG_PRETIMEOUT_DEFAULT_GOV,
- WATCHDOG_GOV_NAME_MAXLEN)) {
+ if (!strcmp(gov->name, WATCHDOG_PRETIMEOUT_DEFAULT_GOV)) {
spin_lock_irq(&pretimeout_lock);
default_gov = gov;
On Jul 23 2018, Randy Dunlap <[email protected]> wrote:
> I don't see that all of these string compare fields are null-terminated.
That gcc has converted the strncmp calls to strcmp is a pretty strong
evidence that they are.
Andreas.
--
Andreas Schwab, [email protected]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
On 07/23/2018 09:49 PM, Finn Thain wrote:
> On Mon, 23 Jul 2018, Randy Dunlap wrote:
>
>> On 07/20/2018 12:20 AM, Andreas Schwab wrote:
>>> On Jul 19 2018, Randy Dunlap <[email protected]> wrote:
>>>
>>>> block/partitions/ldm.o: In function `ldm_partition':
>>>> ldm.c:(.text+0x1900): undefined reference to `strcmp'
>>>> ldm.c:(.text+0x1964): undefined reference to `strcmp'
>>>> drivers/rtc/rtc-proc.o: In function `is_rtc_hctosys':
>>>> rtc-proc.c:(.text+0x290): undefined reference to `strcmp'
>>>> drivers/watchdog/watchdog_pretimeout.o: In function `watchdog_register_governor':
>>>> (.text+0x142): undefined reference to `strcmp'
>>>
>>> GCC has optimized strncmp to strcmp, but at a stage where macros are no
>>> longer available. I think the right fix is to use strcmp directly,
>>> since strncmp doesn't make sense here.
>>
>> Hi Andreas,
>>
>> I don't see that all of these string compare fields are null-terminated.
>>
>
> Some of the strncmp calls in ldm.c are null-terminated, some are not.
>
> That would imply that the compiler will emit both strcmp and strncmp
> calls.
>
> A strncmp call isn't a problem, because m68k doesn't define
> __HAVE_ARCH_STRNCMP and so the one from lib/string.c gets built.
>
>> How does one convert strncmp() users to strcmp()?
>>
>> thanks,
>>
>
> The untested patch below may work. It seems that it may be relevant to
> both arc and m68k:
I got back to looking at the build errors. I agree with Andreas that
all of the fields in question are null-terminated, so Finn's patch looks
good to me.
--
~Randy
On Thu, 26 Jul 2018, Randy Dunlap wrote:
> >
> > The untested patch below may work. It seems that it may be relevant to
> > both arc and m68k:
>
>
> I got back to looking at the build errors. I agree with Andreas that
> all of the fields in question are null-terminated, so Finn's patch looks
> good to me.
>
>
That patch is insufficient:
ERROR: "strcmp" [fs/hfsplus/hfsplus.ko] undefined!
And I haven't even attempted "make ARCH=m68k ... allyesconfig" let alone
"make ARCH=arc ... allyesconfig".
And we have to anticipate an ongoing game of whack-a-mole here, as new
library calls get added to linux and new optimizations get added to gcc.
The real problem here seems to be a compiler bug: converting strncmp to
strcmp is bogus. Some implicit assumptions in that code transformation:
1. that strcmp is equivalient to strncmp in this case
2. that strcmp is faster than strncmp in this case
3. that strcmp is actually available to the linker
Why doesn't gcc convert strncmp to __builtin_strcmp? That would require
fewer assumptions, and it would actually link, and we wouldn't have to
keep patching things...
--
On Jul 27 2018, Finn Thain <[email protected]> wrote:
> Why doesn't gcc convert strncmp to __builtin_strcmp?
It does. In fact, it first converts strncmp to __builtin_strncmp, then
optimizes it to __builtin_strcmp. Finally, __bultin_strcmp is expanded
to a call to strcmp.
Andreas.
--
Andreas Schwab, [email protected]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
On Fri, 27 Jul 2018, Andreas Schwab wrote:
> On Jul 27 2018, Finn Thain <[email protected]> wrote:
>
> > Why doesn't gcc convert strncmp to __builtin_strcmp?
>
> It does. In fact, it first converts strncmp to __builtin_strncmp, then
> optimizes it to __builtin_strcmp. Finally, __bultin_strcmp is expanded
> to a call to strcmp.
>
So either the static inline strcmp routine has to be dropped from
arch/m68k/include/asm/string.h (along with #define __HAVE_ARCH_STRCMP), or
else a static inline strncmp has to be added (along with #define
__HAVE_ARCH_STRNCMP)?
--
> Andreas.
>
>
Hi Andreas,
On Tue, Jul 24, 2018 at 7:35 PM Andreas Schwab <[email protected]> wrote:
> On Jul 23 2018, Randy Dunlap <[email protected]> wrote:
> > I don't see that all of these string compare fields are null-terminated.
>
> That gcc has converted the strncmp calls to strcmp is a pretty strong
> evidence that they are.
Isn't gcc-8 just assuming that any char array must contain a valid
NUL-terminated string, unless it is tagged with the nonstring attribute?
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 Sep 26 2018, Geert Uytterhoeven <[email protected]> wrote:
> Isn't gcc-8 just assuming that any char array must contain a valid
> NUL-terminated string, unless it is tagged with the nonstring attribute?
No, only if you call strncpy on it and make it potentially
non-terminated.
Andreas.
--
Andreas Schwab, [email protected]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."