A recent "libcall optimization" addition to LLVM will emit libcalls to
stpcpy, which the kernel doesn't provide an implementation, breaking
almost all kernel builds with ToT Clang. Disable it for clang.
We discussed providing an implementation, but the interface is generally
unsafe as it provides no bounds checking.
-fno-builtin-foo doesn't prevent GCC from emitting calls to foo, and GCC
doesn't currently do the same libcall optimizations. If it ever does,
then we can resurrect these implementations, but right now, YAGNI. So we
only add these flags to CLANG_FLAGS to solve a Clang specific issue.
The first patch is critical, I'm hoping Masahiro will pick it for the
Kbuild tree and help us to get the fix in 5.9.
The rest are cleanups; sending them for feedback/review/testing. Once
the first hits mainline, I plan to resend the rest to the x86
maintainers for inclusion in tip.
Nick Desaulniers (5):
Makefile: add -fno-builtin-stpcpy
Makefile: add -fno-builtin-bcmp
Revert "lib/string.c: implement a basic bcmp"
x86/boot: use -fno-builtin-bcmp
x86: don't build CONFIG_X86_32 as -ffreestanding
Makefile | 2 ++
arch/x86/Makefile | 3 ---
arch/x86/boot/Makefile | 1 +
arch/x86/boot/string.c | 8 --------
include/linux/string.h | 3 ---
lib/string.c | 20 --------------------
6 files changed, 3 insertions(+), 34 deletions(-)
--
2.28.0.297.g1956fa8f8d-goog
LLVM implemented a recent "libcall optimization" that lowers calls to
`sprintf(dest, "%s", str)` where the return value is used to
`stpcpy(dest, str) - dest`. This generally avoids the machinery involved
in parsing format strings. This optimization was introduced into
clang-12. Because the kernel does not provide an implementation of
stpcpy, we observe linkage failures for almost all targets when building
with ToT clang.
The interface is unsafe as it does not perform any bounds checking.
Disable this "libcall optimization" via `-fno-builtin-stpcpy`.
Cc: [email protected] # 4.4
Link: https://bugs.llvm.org/show_bug.cgi?id=47162
Link: https://github.com/ClangBuiltLinux/linux/issues/1126
Link: https://reviews.llvm.org/D85963
Reported-by: Sami Tolvanen <[email protected]>
Suggested-by: Dávid Bolvanský <[email protected]>
Suggested-by: Kees Cook <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/Makefile b/Makefile
index 9cac6fde3479..e523dc8d30e0 100644
--- a/Makefile
+++ b/Makefile
@@ -578,6 +578,7 @@ ifneq ($(LLVM_IAS),1)
CLANG_FLAGS += -no-integrated-as
endif
CLANG_FLAGS += -Werror=unknown-warning-option
+CLANG_FLAGS += -fno-builtin-stpcpy
KBUILD_CFLAGS += $(CLANG_FLAGS)
KBUILD_AFLAGS += $(CLANG_FLAGS)
export CLANG_FLAGS
--
2.28.0.297.g1956fa8f8d-goog
-ffreestanding typically inhibits "libcall optimizations" where calls to
certain library functions can be replaced by the compiler in certain
cases to calls to other library functions that may be more efficient.
This can be problematic for embedded targets that don't provide full
libc implementations.
-ffreestanding inhibits all optimizations, which
is the safe choice, but generally we want the optimizations that are
performed. The Linux kernel does implement a fair amount of libc
routines. Instead of -ffreestanding (which makes more sense in smaller
images like kexec's purgatory image), prefer -fno-builtin-* flags to
disable the compiler from emitting calls to functions which may not be
defined.
If you see a linkage failure due to a missing symbol that's typically
defined in a libc, and not explicitly called from the source code, then
the compiler may have done such a transform. You can either implement
such a function (ie. in lib/string.c) or disable the transform outright
via -fno-builtin-* flag (where * is the name of the library routine, ie.
-fno-builtin-bcmp).
i386_defconfig build+boot tested with GCC and Clang. Removes a pretty
old TODO from the codebase.
Fixes: 6edfba1b33c7 ("x86_64: Don't define string functions to builtin")
Suggested-by: Arvind Sankar <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
Might be nice to have
https://lore.kernel.org/lkml/CAKwvOdn-mv1D1GEk3pWnPYsyzQRRk5qZFhSi0CYn6tRDo1O_iw@mail.gmail.com/T/#u
first.
arch/x86/Makefile | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 4346ffb2e39f..2383a96cf4fd 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -80,9 +80,6 @@ ifeq ($(CONFIG_X86_32),y)
# CPU-specific tuning. Anything which can be shared with UML should go here.
include arch/x86/Makefile_32.cpu
KBUILD_CFLAGS += $(cflags-y)
-
- # temporary until string.h is fixed
- KBUILD_CFLAGS += -ffreestanding
else
BITS := 64
UTS_MACHINE := x86_64
--
2.28.0.297.g1956fa8f8d-goog
The issue with using `-fno-builtin-*` flags was that they were not
retained during an LTO link with LLVM. This was fixed in clang-11 by
https://reviews.llvm.org/D71193
(0508c994f0b14144041f2cfd3ba9f9a80f03de08), which is also the minimum
supported version of clang for LTO. Use `-fno-builtin-bcmp` instead.
With this applid, we can cleanly revert
commit 5f074f3e192f ("lib/string.c: implement a basic bcmp")
Reviewed-by: Kees Cook <[email protected]>
Suggested-by: Nathan Chancellor <[email protected]>
Signed-off-by: Nick Desaulniers <[email protected]>
---
Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/Makefile b/Makefile
index e523dc8d30e0..def590b743a9 100644
--- a/Makefile
+++ b/Makefile
@@ -579,6 +579,7 @@ CLANG_FLAGS += -no-integrated-as
endif
CLANG_FLAGS += -Werror=unknown-warning-option
CLANG_FLAGS += -fno-builtin-stpcpy
+CLANG_FLAGS += -fno-builtin-bcmp
KBUILD_CFLAGS += $(CLANG_FLAGS)
KBUILD_AFLAGS += $(CLANG_FLAGS)
export CLANG_FLAGS
--
2.28.0.297.g1956fa8f8d-goog
On Wed, Aug 19, 2020 at 12:16:50PM -0700, Nick Desaulniers wrote:
> LLVM implemented a recent "libcall optimization" that lowers calls to
> `sprintf(dest, "%s", str)` where the return value is used to
> `stpcpy(dest, str) - dest`. This generally avoids the machinery involved
> in parsing format strings. This optimization was introduced into
> clang-12. Because the kernel does not provide an implementation of
> stpcpy, we observe linkage failures for almost all targets when building
> with ToT clang.
>
> The interface is unsafe as it does not perform any bounds checking.
> Disable this "libcall optimization" via `-fno-builtin-stpcpy`.
>
> Cc: [email protected] # 4.4
> Link: https://bugs.llvm.org/show_bug.cgi?id=47162
> Link: https://github.com/ClangBuiltLinux/linux/issues/1126
> Link: https://reviews.llvm.org/D85963
> Reported-by: Sami Tolvanen <[email protected]>
> Suggested-by: D?vid Bolvansk? <[email protected]>
> Suggested-by: Kees Cook <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>
> ---
> Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Makefile b/Makefile
> index 9cac6fde3479..e523dc8d30e0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -578,6 +578,7 @@ ifneq ($(LLVM_IAS),1)
> CLANG_FLAGS += -no-integrated-as
> endif
> CLANG_FLAGS += -Werror=unknown-warning-option
> +CLANG_FLAGS += -fno-builtin-stpcpy
> KBUILD_CFLAGS += $(CLANG_FLAGS)
> KBUILD_AFLAGS += $(CLANG_FLAGS)
> export CLANG_FLAGS
> --
> 2.28.0.297.g1956fa8f8d-goog
>
On Wed, Aug 19, 2020 at 12:16:51PM -0700, Nick Desaulniers wrote:
> The issue with using `-fno-builtin-*` flags was that they were not
> retained during an LTO link with LLVM. This was fixed in clang-11 by
> https://reviews.llvm.org/D71193
> (0508c994f0b14144041f2cfd3ba9f9a80f03de08), which is also the minimum
> supported version of clang for LTO. Use `-fno-builtin-bcmp` instead.
>
> With this applid, we can cleanly revert
> commit 5f074f3e192f ("lib/string.c: implement a basic bcmp")
>
> Reviewed-by: Kees Cook <[email protected]>
> Suggested-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Nick Desaulniers <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
> ---
> Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Makefile b/Makefile
> index e523dc8d30e0..def590b743a9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -579,6 +579,7 @@ CLANG_FLAGS += -no-integrated-as
> endif
> CLANG_FLAGS += -Werror=unknown-warning-option
> CLANG_FLAGS += -fno-builtin-stpcpy
> +CLANG_FLAGS += -fno-builtin-bcmp
> KBUILD_CFLAGS += $(CLANG_FLAGS)
> KBUILD_AFLAGS += $(CLANG_FLAGS)
> export CLANG_FLAGS
> --
> 2.28.0.297.g1956fa8f8d-goog
>