m68k allmodconfig build errors

m68k allmodconfig build errors

[Randy Dunlap]

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

Re: m68k allmodconfig build errors

[Finn Thain]

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,
> 

Re: m68k allmodconfig build errors

[Randy Dunlap]

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

Re: m68k allmodconfig build errors

[Andreas Schwab]

On Jul 19 2018, Randy Dunlap <rdunlap@infradead.org> 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, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: m68k allmodconfig build errors

[Randy Dunlap]

On 07/20/2018 12:20 AM, Andreas Schwab wrote:
> On Jul 19 2018, Randy Dunlap <rdunlap@infradead.org> 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

Re: m68k allmodconfig build errors

[Finn Thain]

On Mon, 23 Jul 2018, Randy Dunlap wrote:

> On 07/20/2018 12:20 AM, Andreas Schwab wrote:
> > On Jul 19 2018, Randy Dunlap <rdunlap@infradead.org> 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;
 

Re: m68k allmodconfig build errors

[Randy Dunlap]

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 <rdunlap@infradead.org> 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

Re: m68k allmodconfig build errors

[Finn Thain]

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...

-- 

Re: m68k allmodconfig build errors

[Andreas Schwab]

On Jul 27 2018, Finn Thain <fthain@telegraphics.com.au> 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, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: m68k allmodconfig build errors

[Finn Thain]

On Fri, 27 Jul 2018, Andreas Schwab wrote:

> On Jul 27 2018, Finn Thain <fthain@telegraphics.com.au> 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.
> 
> 

Re: m68k allmodconfig build errors

[Andreas Schwab]

On Jul 23 2018, Randy Dunlap <rdunlap@infradead.org> 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, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: m68k allmodconfig build errors

[Geert Uytterhoeven]

Hi Andreas,

On Tue, Jul 24, 2018 at 7:35 PM Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Jul 23 2018, Randy Dunlap <rdunlap@infradead.org> 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 -- 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 allmodconfig build errors

[Andreas Schwab]

On Sep 26 2018, Geert Uytterhoeven <geert@linux-m68k.org> 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, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."