Hi Finn,
On Fri, Dec 21, 2018 at 7:21 AM Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx> wrote:
On Thu, 20 Dec 2018, John Paul Adrian Glaubitz wrote:
On 12/20/18 12:16 AM, Finn Thain wrote:
Andreas said the error happened when the new compiler expanded a
__bultin_strcmp call to a strcmp call.
https://lore.kernel.org/lkml/87in513wbt.fsf@xxxxxxxxx/
The new compiler seems to assume that a strcmp symbol exists in the final
link. I don't see how that kind of assumption is valid here.
So I think the real bug is the lack of -ffreestanding. That omission
allows the compiler to assume that libc is available in the final link.
(At least, I imagine that's what the compiler authors had in mind.)
Anyway, the use of -ffreestanding certainly avoids this optimization.
Yeah, during the kernel history, several other architectures started using
-ffreestanding, to avoid similar replacements behind our back.
So there is definitely prior art for starting to use that.
Could someone whip up a patch so I can add it to the Debian package of
the kernel?
There are various patches to choose from.
1) As Andreas said, "strncmp doesn't make sense here." I agree, it's
inefficient to use strncmp(a, b, strlen(a)) or strncmp(&a[0], b, sizeof(a))
etc.
We can find and fix these call sites as and when they break the build. I
sent an incomplete patch to fix some of them --
https://lore.kernel.org/lkml/alpine.LNX.2.21.1807241423000.8@nippy.intranet/
2) We can use -ffreestanding, to avoid risky optimizations involving libc.
3) As Arnd said, we can provide strcmp. That's what my #if 0 workaround
did. I've not sent that upstream because a strcmp function (besides
__builtin_strcmp) is not needed given either of the above solutions.
Any or all of these approaches will avoid the link error. Option (1) seems
to be approved by maintainers. So if you want a patch that can be sent
upstream, that would be it. That solution might be a lot more maintainable
if it took the form of a Coccinelle script that could be included in the
kernel tree.
Option (2) would involve a small Makefile patch, as below.
diff --git a/arch/m68k/Makefile b/arch/m68k/Makefile
index 997c9f20ea0f..c318afd15e33 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)
ifdef CONFIG_MMU
# without -fno-strength-reduce the 53c7xx.c driver fails ;-(
KBUILD_CFLAGS += -fno-strength-reduce -ffixed-a2
@@ -69,6 +69,8 @@ KBUILD_CFLAGS += -D__uClinux__
KBUILD_AFLAGS += -D__uClinux__
endif
+KBUILD_CFLAGS += -pipe -ffreestanding
+
KBUILD_LDFLAGS := -m m68kelf
KBUILD_LDFLAGS_MODULE += -T $(srctree)/arch/m68k/kernel/module.lds
This patch seems to work fine but may have implications for both 680x0 and
Coldfire. It will make some optimizations unavailable but I haven't tried
to measure that effect. It could be a hard sell if it harms performance.
With gcc 7.3.0, it increases the kernel image size for atari_defconfig by
ca. 300 bytes (which is an 0.01% increase).
But -ffreestanding allows to switch to gcc 8.2.0, which reduces the same
kernel by ca 7.5 KiB again.
Finn, can you please submit your patch with a proper SoB?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
