2022-05-28 04:34:39

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v7 0/8] kbuild: yet another series of cleanups


A couple errors have been reported since I applied v6.

I am sending v7.


Masahiro Yamada (8):
kbuild: replace $(linked-object) with CONFIG options
kbuild: do not create *.prelink.o for Clang LTO or IBT
parisc: fix the exit status of arch/parisc/nm
kbuild: check static EXPORT_SYMBOL* by script instead of modpost
kbuild: make built-in.a rule robust against too long argument error
kbuild: make *.mod rule robust against too long argument error
kbuild: add cmd_and_savecmd macro
kbuild: rebuild multi-object modules when objtool is updated

arch/parisc/Makefile | 2 +-
arch/parisc/nm | 12 +++++-
scripts/Kbuild.include | 10 ++++-
scripts/Makefile.build | 87 +++++++++++++++++---------------------
scripts/Makefile.lib | 7 ---
scripts/Makefile.modfinal | 5 +--
scripts/Makefile.modpost | 9 +---
scripts/check-local-export | 65 ++++++++++++++++++++++++++++
scripts/mod/modpost.c | 35 +--------------
9 files changed, 128 insertions(+), 104 deletions(-)
mode change 100644 => 100755 arch/parisc/nm
create mode 100755 scripts/check-local-export

--
2.32.0



2022-05-28 15:54:23

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v7 5/8] kbuild: make built-in.a rule robust against too long argument error

Kbuild runs at the top of objtree instead of changing the working
directory to subdirectories. I think this design is nice overall but
some commands have a scalability issue.

The build command of built-in.a is one of them whose length scales with:

O(D * N)

Here, D is the length of the directory path (i.e. $(obj)/ prefix),
N is the number of objects in the Makefile, O() is the big O notation.

The deeper directory the Makefile directory is located, the more easily
it will hit the too long argument error.

We can make it better. Trim the $(obj)/ by Make's builtin function, and
restore it by a shell command (sed).

With this, the command length scales with:

O(D + N)

In-tree modules still have some room to the limit (ARG_MAX=2097152),
but this is more future-proof for big modules in a deep directory.

For example, you can build i915 as builtin (CONFIG_DRM_I915=y) and
compare drivers/gpu/drm/i915/.built-in.a.cmd with/without this commit.

Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-by: Nicolas Schier <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>
Tested-by: Sedat Dilek <[email protected]> # LLVM-14 (x86-64)
---

Changes in v7:
- Add comment to the code

Changes in v2:
- New patch

scripts/Makefile.build | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 0135b4e7f78b..014ebee70fd6 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -376,9 +376,14 @@ $(subdir-modorder): $(obj)/%/modules.order: $(obj)/% ;
#
# Rule to compile a set of .o files into one .a file (without symbol table)
#
+# To make this rule robust against "Argument list too long" error,
+# remove $(obj)/ prefix, and restore it by a shell command.

quiet_cmd_ar_builtin = AR $@
- cmd_ar_builtin = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
+ cmd_ar_builtin = rm -f $@; \
+ echo $(patsubst $(obj)/%,%,$(real-prereqs)) | \
+ sed -E 's:([^ ]+):$(obj)/\1:g' | \
+ xargs $(AR) cDPrST $@

$(obj)/built-in.a: $(real-obj-y) FORCE
$(call if_changed,ar_builtin)
--
2.32.0


2022-05-28 19:24:45

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v7 8/8] kbuild: rebuild multi-object modules when objtool is updated

When CONFIG_LTO_CLANG or CONFIG_X86_KERNEL_IBT is enabled, objtool for
multi-object modules is postponed until the objects are linked together.

Make sure to re-run objtool and re-link multi-object modules when
objtool is updated.

Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Acked-by: Josh Poimboeuf <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>
Reviewed-by: Nicolas Schier <[email protected]>
Tested-by: Sedat Dilek <[email protected]> # LLVM-14 (x86-64)
---

(no changes since v4)

Changes in v4:
- New
Resent of my previous submission
https://lore.kernel.org/linux-kbuild/[email protected]/

scripts/Makefile.build | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 31feb798e16e..bd5fc2b37387 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -412,13 +412,18 @@ $(obj)/modules.order: $(obj-m) FORCE
$(obj)/lib.a: $(lib-y) FORCE
$(call if_changed,ar)

-quiet_cmd_link_multi-m = LD [M] $@
- cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ @$(patsubst %.o,%.mod,$@) $(cmd_objtool)
+quiet_cmd_ld_multi_m = LD [M] $@
+ cmd_ld_multi_m = $(LD) $(ld_flags) -r -o $@ @$(patsubst %.o,%.mod,$@) $(cmd_objtool)
+
+define rule_ld_multi_m
+ $(call cmd_and_savecmd,ld_multi_m)
+ $(call cmd,gen_objtooldep)
+endef

$(multi-obj-m): objtool-enabled := $(delay-objtool)
$(multi-obj-m): part-of-module := y
$(multi-obj-m): %.o: %.mod FORCE
- $(call if_changed,link_multi-m)
+ $(call if_changed_rule,ld_multi_m)
$(call multi_depend, $(multi-obj-m), .o, -objs -y -m)

targets := $(filter-out $(PHONY), $(targets))
--
2.32.0


2022-05-28 19:29:21

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v7 3/8] parisc: fix the exit status of arch/parisc/nm

parisc overrides 'nm' with a shell script. I do not know the reason,
but anyway it is how it has worked since 2003. [1]

A problem is that this script returns the exit code of grep instead of
${CROSS_COMPILE}nm.

grep(1) says:
Normally the exit status is 0 if a line is selected, 1 if no lines
were selected, and 2 if an error occurred. However, if the -q or
--quiet or --silent is used and a line is selected, the exit status
is 0 even if an error occurred.

When the given object has no symbol, grep returns 1, while the true nm
returns 0. Hence, build rules using ${NM} fail on ARCH=parisc even if
the given object is valid.

This commit corrects the exit status of the script.

- A pipeline returns the exit status of the last command (here, grep).
The exit status of ${CROSS_COMPILE}nm is just ignored. Use bash's
pipefail flag to catch errors of ${CROSS_COMPILE}nm.

- If grep returns 1, this script should return 0 in order to mimic
true nm. If grep returns 2, it is a real and fatal error. Return
it as is.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=36eaa6e4c0e0b6950136b956b72fd08155b92ca3

Signed-off-by: Masahiro Yamada <[email protected]>
---

Changes in v7:
- New patch

arch/parisc/Makefile | 2 +-
arch/parisc/nm | 12 ++++++++++--
2 files changed, 11 insertions(+), 3 deletions(-)
mode change 100644 => 100755 arch/parisc/nm

diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile
index aca1710fd658..e7139955367d 100644
--- a/arch/parisc/Makefile
+++ b/arch/parisc/Makefile
@@ -18,7 +18,7 @@
boot := arch/parisc/boot
KBUILD_IMAGE := $(boot)/bzImage

-NM = sh $(srctree)/arch/parisc/nm
+NM = $(srctree)/arch/parisc/nm
CHECKFLAGS += -D__hppa__=1

ifdef CONFIG_64BIT
diff --git a/arch/parisc/nm b/arch/parisc/nm
old mode 100644
new mode 100755
index c788308de33f..3e72238a91f3
--- a/arch/parisc/nm
+++ b/arch/parisc/nm
@@ -1,6 +1,14 @@
-#!/bin/sh
+#!/bin/bash
##
# Hack to have an nm which removes the local symbols. We also rely
# on this nm being hidden out of the ordinarily executable path
##
-${CROSS_COMPILE}nm $* | grep -v '.LC*[0-9]*$'
+
+# use pipefail to catch error of ${CROSS_COMPILE}nm
+set -o pipefail
+
+# grep exits with 1 if no lines were selected.
+# If the given object has no symbol, grep returns 1, but it is not an error.
+
+${CROSS_COMPILE}nm "$@" |
+{ grep -v '.LC*[0-9]*$' || { exit_code=$?; test $exit_code -eq 1 || exit $exit_code; } }
--
2.32.0


2022-05-28 19:36:50

by Helge Deller

[permalink] [raw]
Subject: Re: [PATCH v7 3/8] parisc: fix the exit status of arch/parisc/nm

Hello Masahiro,

On 5/27/22 12:01, Masahiro Yamada wrote:
> parisc overrides 'nm' with a shell script. I do not know the reason,
> but anyway it is how it has worked since 2003. [1]

I don't know the reason either...
I assume it was that the older toolchains had bugs and kept lots of local
symbols like .LC? in the object files.

I did a small build without the nm script (and removed it's reference
in the Makefile), and it did not seem to break anything.

> A problem is that this script returns the exit code of grep instead of
> ${CROSS_COMPILE}nm.

Instead of fixing this, I'd suggest that you simply remove the nm script
alltogether. If you like I can apply such a patch in the parisc git tree,
and you just drop this specific patch. Or you change your patch to remove it.
Just let me know what you prefer.

Helge

> grep(1) says:
> Normally the exit status is 0 if a line is selected, 1 if no lines
> were selected, and 2 if an error occurred. However, if the -q or
> --quiet or --silent is used and a line is selected, the exit status
> is 0 even if an error occurred.
>
> When the given object has no symbol, grep returns 1, while the true nm
> returns 0. Hence, build rules using ${NM} fail on ARCH=parisc even if
> the given object is valid.
>
> This commit corrects the exit status of the script.
>
> - A pipeline returns the exit status of the last command (here, grep).
> The exit status of ${CROSS_COMPILE}nm is just ignored. Use bash's
> pipefail flag to catch errors of ${CROSS_COMPILE}nm.
>
> - If grep returns 1, this script should return 0 in order to mimic
> true nm. If grep returns 2, it is a real and fatal error. Return
> it as is.
>
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=36eaa6e4c0e0b6950136b956b72fd08155b92ca3
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> Changes in v7:
> - New patch
>
> arch/parisc/Makefile | 2 +-
> arch/parisc/nm | 12 ++++++++++--
> 2 files changed, 11 insertions(+), 3 deletions(-)
> mode change 100644 => 100755 arch/parisc/nm
>
> diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile
> index aca1710fd658..e7139955367d 100644
> --- a/arch/parisc/Makefile
> +++ b/arch/parisc/Makefile
> @@ -18,7 +18,7 @@
> boot := arch/parisc/boot
> KBUILD_IMAGE := $(boot)/bzImage
>
> -NM = sh $(srctree)/arch/parisc/nm
> +NM = $(srctree)/arch/parisc/nm
> CHECKFLAGS += -D__hppa__=1
>
> ifdef CONFIG_64BIT
> diff --git a/arch/parisc/nm b/arch/parisc/nm
> old mode 100644
> new mode 100755
> index c788308de33f..3e72238a91f3
> --- a/arch/parisc/nm
> +++ b/arch/parisc/nm
> @@ -1,6 +1,14 @@
> -#!/bin/sh
> +#!/bin/bash
> ##
> # Hack to have an nm which removes the local symbols. We also rely
> # on this nm being hidden out of the ordinarily executable path
> ##
> -${CROSS_COMPILE}nm $* | grep -v '.LC*[0-9]*$'
> +
> +# use pipefail to catch error of ${CROSS_COMPILE}nm
> +set -o pipefail
> +
> +# grep exits with 1 if no lines were selected.
> +# If the given object has no symbol, grep returns 1, but it is not an error.
> +
> +${CROSS_COMPILE}nm "$@" |
> +{ grep -v '.LC*[0-9]*$' || { exit_code=$?; test $exit_code -eq 1 || exit $exit_code; } }


2022-05-28 19:38:28

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v7 2/8] kbuild: do not create *.prelink.o for Clang LTO or IBT

When CONFIG_LTO_CLANG=y, additional intermediate *.prelink.o is created
for each module. Also, objtool is postponed until LLVM IR is converted
to ELF.

CONFIG_X86_KERNEL_IBT works in a similar way to postpone objtool until
objects are merged together.

This commit stops generating *.prelink.o, so the build flow will look
similar with/without LTO.

The following figures show how the LTO build currently works, and
how this commit is changing it.

Current build flow
==================

[1] single-object module

$(LD)
$(CC) +objtool $(LD)
foo.c --------------------> foo.o -----> foo.prelink.o -----> foo.ko
(LLVM IR) (ELF) |
|
foo.mod.o --/

[2] multi-object module
$(LD)
$(CC) $(AR) +objtool $(LD)
foo1.c -----> foo1.o -----> foo.o -----> foo.prelink.o -----> foo.ko
| (archive) (ELF) |
foo2.c -----> foo2.o --/ |
(LLVM IR) foo.mod.o --/

One confusion is foo.o in multi-object module is an archive despite of
its suffix.

New build flow
==============

[1] single-object module

Since there is only one object, there is no need to keep the LLVM IR.
Use $(CC)+$(LD) to generate an ELF object in one build rule. When LTO
is disabled, $(LD) is unneeded because $(CC) produces an ELF object.

$(CC)+$(LD)+objtool $(LD)
foo.c ----------------------------> foo.o ---------> foo.ko
(ELF) |
|
foo.mod.o --/

[2] multi-object module

Previously, $(AR) was used to combine LLVM bitcode into an archive,
but there was no technical reason to do so. Use $(LD) to merge them
into a single ELF object.

$(LD)
$(CC) +objtool $(LD)
foo1.c ---------> foo1.o ---------> foo.o ---------> foo.ko
| (ELF) |
foo2.c ---------> foo2.o ----/ |
(LLVM IR) foo.mod.o --/

Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-by: Nicolas Schier <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>
Reviewed-by: Sami Tolvanen <[email protected]>
Tested-by: Sedat Dilek <[email protected]> # LLVM-14 (x86-64)
---

(no changes since v2)

Changes in v2:
- replace the chain of $(if ...) with $(and )

scripts/Kbuild.include | 4 +++
scripts/Makefile.build | 60 ++++++++++++---------------------------
scripts/Makefile.lib | 7 -----
scripts/Makefile.modfinal | 5 ++--
scripts/Makefile.modpost | 9 ++----
scripts/mod/modpost.c | 7 -----
6 files changed, 26 insertions(+), 66 deletions(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 3514c2149e9d..455a0a6ce12d 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -15,6 +15,10 @@ pound := \#
# Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o
dot-target = $(dir $@).$(notdir $@)

+###
+# Name of target with a '.tmp_' as filename prefix. foo/bar.o => foo/.tmp_bar.o
+tmp-target = $(dir $@).tmp_$(notdir $@)
+
###
# The temporary file to save gcc -MMD generated dependencies must not
# contain a comma
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index f80196eef03a..23e1fb19f9ad 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -88,10 +88,6 @@ endif
targets-for-modules := $(foreach x, o mod $(if $(CONFIG_TRIM_UNUSED_KSYMS), usyms), \
$(patsubst %.o, %.$x, $(filter %.o, $(obj-m))))

-ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
-targets-for-modules += $(patsubst %.o, %.prelink.o, $(filter %.o, $(obj-m)))
-endif
-
ifdef need-modorder
targets-for-modules += $(obj)/modules.order
endif
@@ -152,8 +148,16 @@ $(obj)/%.ll: $(src)/%.c FORCE
# The C file is compiled and updated dependency information is generated.
# (See cmd_cc_o_c + relevant part of rule_cc_o_c)

+is-single-obj-m = $(and $(part-of-module),$(filter $@, $(obj-m)),y)
+
+ifdef CONFIG_LTO_CLANG
+cmd_ld_single_m = $(if $(is-single-obj-m), ; $(LD) $(ld_flags) -r -o $(tmp-target) $@; mv $(tmp-target) $@)
+endif
+
quiet_cmd_cc_o_c = CC $(quiet_modtag) $@
- cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $< $(cmd_objtool)
+ cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $< \
+ $(cmd_ld_single_m) \
+ $(cmd_objtool)

ifdef CONFIG_MODVERSIONS
# When module versioning is enabled the following steps are executed:
@@ -219,7 +223,7 @@ objtool_args = \
$(if $(CONFIG_STACK_VALIDATION), --stackval) \
$(if $(CONFIG_HAVE_STATIC_CALL_INLINE), --static-call) \
--uaccess \
- $(if $($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT)), --link) \
+ $(if $(delay-objtool), --link) \
$(if $(part-of-module), --module) \
$(if $(CONFIG_GCOV_KERNEL), --no-unreachable)

@@ -228,21 +232,16 @@ cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(o

endif # CONFIG_OBJTOOL

-ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
-
-# Skip objtool for LLVM bitcode
-$(obj)/%.o: objtool-enabled :=
-
-else

# 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
# 'OBJECT_FILES_NON_STANDARD_foo.o := 'y': skip objtool checking for a file
# 'OBJECT_FILES_NON_STANDARD_foo.o := 'n': override directory skip for a file

-$(obj)/%.o: objtool-enabled = $(if $(filter-out y%, \
- $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y)
+is-standard-object = $(if $(filter-out y%, $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n),y)

-endif
+delay-objtool := $(or $(CONFIG_LTO_CLANG),$(CONFIG_X86_KERNEL_IBT))
+
+$(obj)/%.o: objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y))

ifdef CONFIG_TRIM_UNUSED_KSYMS
cmd_gen_ksymdeps = \
@@ -271,24 +270,6 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
$(call if_changed_rule,cc_o_c)
$(call cmd,force_checksrc)

-ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
-# Module .o files may contain LLVM bitcode, compile them into native code
-# before ELF processing
-quiet_cmd_cc_prelink_modules = LD [M] $@
- cmd_cc_prelink_modules = \
- $(LD) $(ld_flags) -r -o $@ \
- --whole-archive $(filter-out FORCE,$^) \
- $(cmd_objtool)
-
-# objtool was skipped for LLVM bitcode, run it now that we have compiled
-# modules into native code
-$(obj)/%.prelink.o: objtool-enabled = y
-$(obj)/%.prelink.o: part-of-module := y
-
-$(obj)/%.prelink.o: $(obj)/%.o FORCE
- $(call if_changed,cc_prelink_modules)
-endif
-
cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) | \
$(AWK) -v RS='( |\n)' '!x[$$0]++' > $@

@@ -298,7 +279,7 @@ $(obj)/%.mod: FORCE
# List module undefined symbols
cmd_undefined_syms = $(NM) $< | sed -n 's/^ *U //p' > $@

-$(obj)/%.usyms: $(obj)/%$(mod-prelink-ext).o FORCE
+$(obj)/%.usyms: $(obj)/%.o FORCE
$(call if_changed,undefined_syms)

quiet_cmd_cc_lst_c = MKLST $@
@@ -420,16 +401,11 @@ $(obj)/modules.order: $(obj-m) FORCE
$(obj)/lib.a: $(lib-y) FORCE
$(call if_changed,ar)

-ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
-quiet_cmd_link_multi-m = AR [M] $@
-cmd_link_multi-m = \
- rm -f $@; \
- $(AR) cDPrsT $@ @$(patsubst %.o,%.mod,$@)
-else
quiet_cmd_link_multi-m = LD [M] $@
- cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ @$(patsubst %.o,%.mod,$@)
-endif
+ cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ @$(patsubst %.o,%.mod,$@) $(cmd_objtool)

+$(multi-obj-m): objtool-enabled := $(delay-objtool)
+$(multi-obj-m): part-of-module := y
$(multi-obj-m): %.o: %.mod FORCE
$(call if_changed,link_multi-m)
$(call multi_depend, $(multi-obj-m), .o, -objs -y -m)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 0453a1904646..f75138385449 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -225,13 +225,6 @@ dtc_cpp_flags = -Wp,-MMD,$(depfile).pre.tmp -nostdinc \
$(addprefix -I,$(DTC_INCLUDE)) \
-undef -D__DTS__

-ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
-# With CONFIG_LTO_CLANG, .o files in modules might be LLVM bitcode, so we
-# need to run LTO to compile them into native code (.lto.o) before further
-# processing.
-mod-prelink-ext := .prelink
-endif
-
# Useful for describing the dependency of composite objects
# Usage:
# $(call multi_depend, multi_used_targets, suffix_to_remove, suffix_to_add)
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 7f39599e9fae..35100e981f4a 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -9,7 +9,7 @@ __modfinal:
include include/config/auto.conf
include $(srctree)/scripts/Kbuild.include

-# for c_flags and mod-prelink-ext
+# for c_flags
include $(srctree)/scripts/Makefile.lib

# find all modules listed in modules.order
@@ -54,9 +54,8 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \
$(cmd); \
printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)

-
# Re-generate module BTFs if either module's .ko or vmlinux changed
-$(modules): %.ko: %$(mod-prelink-ext).o %.mod.o scripts/module.lds $(if $(KBUILD_BUILTIN),vmlinux) FORCE
+$(modules): %.ko: %.o %.mod.o scripts/module.lds $(if $(KBUILD_BUILTIN),vmlinux) FORCE
+$(call if_changed_except,ld_ko_o,vmlinux)
ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+$(if $(newer-prereqs),$(call cmd,btf_ko))
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 8b343480813d..911606496341 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -41,9 +41,6 @@ __modpost:
include include/config/auto.conf
include $(srctree)/scripts/Kbuild.include

-# for mod-prelink-ext
-include $(srctree)/scripts/Makefile.lib
-
MODPOST = scripts/mod/modpost \
$(if $(CONFIG_MODVERSIONS),-m) \
$(if $(CONFIG_MODULE_SRCVERSION_ALL),-a) \
@@ -117,8 +114,6 @@ $(input-symdump):
@echo >&2 ' Modules may not have dependencies or modversions.'
@echo >&2 ' You may get many unresolved symbol warnings.'

-modules := $(sort $(shell cat $(MODORDER)))
-
# KBUILD_MODPOST_WARN can be set to avoid error out in case of undefined symbols
ifneq ($(KBUILD_MODPOST_WARN)$(filter-out $(existing-input-symdump), $(input-symdump)),)
MODPOST += -w
@@ -127,9 +122,9 @@ endif
# Read out modules.order to pass in modpost.
# Otherwise, allmodconfig would fail with "Argument list too long".
quiet_cmd_modpost = MODPOST $@
- cmd_modpost = sed 's/\.ko$$/$(mod-prelink-ext)\.o/' $< | $(MODPOST) -T -
+ cmd_modpost = sed 's/ko$$/o/' $< | $(MODPOST) -T -

-$(output-symdump): $(MODORDER) $(input-symdump) $(modules:.ko=$(mod-prelink-ext).o) FORCE
+$(output-symdump): $(MODORDER) $(input-symdump) FORCE
$(call if_changed,modpost)

targets += $(output-symdump)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index b70a31b4245a..e74224a6a8e8 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1903,10 +1903,6 @@ static char *remove_dot(char *s)
size_t m = strspn(s + n + 1, "0123456789");
if (m && (s[n + m + 1] == '.' || s[n + m + 1] == 0))
s[n] = 0;
-
- /* strip trailing .prelink */
- if (strends(s, ".prelink"))
- s[strlen(s) - 8] = '\0';
}
return s;
}
@@ -2028,9 +2024,6 @@ static void read_symbols(const char *modname)
/* strip trailing .o */
tmp = NOFAIL(strdup(modname));
tmp[strlen(tmp) - 2] = '\0';
- /* strip trailing .prelink */
- if (strends(tmp, ".prelink"))
- tmp[strlen(tmp) - 8] = '\0';
mod = new_module(tmp);
free(tmp);
}
--
2.32.0


2022-05-28 19:51:09

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v7 1/8] kbuild: replace $(linked-object) with CONFIG options

*.prelink.o is created when CONFIG_LTO_CLANG or CONFIG_X86_KERNEL_IBT
is enabled.

Replace $(linked-object) with $(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT)
so you will get better idea when the --link option is passed.

No functional change is intended.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Changes in v7:
- New patch

scripts/Makefile.build | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 06400504150b..f80196eef03a 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -219,7 +219,7 @@ objtool_args = \
$(if $(CONFIG_STACK_VALIDATION), --stackval) \
$(if $(CONFIG_HAVE_STATIC_CALL_INLINE), --static-call) \
--uaccess \
- $(if $(linked-object), --link) \
+ $(if $($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT)), --link) \
$(if $(part-of-module), --module) \
$(if $(CONFIG_GCOV_KERNEL), --no-unreachable)

@@ -284,7 +284,6 @@ quiet_cmd_cc_prelink_modules = LD [M] $@
# modules into native code
$(obj)/%.prelink.o: objtool-enabled = y
$(obj)/%.prelink.o: part-of-module := y
-$(obj)/%.prelink.o: linked-object := y

$(obj)/%.prelink.o: $(obj)/%.o FORCE
$(call if_changed,cc_prelink_modules)
--
2.32.0


2022-05-28 19:57:38

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v7 3/8] parisc: fix the exit status of arch/parisc/nm

Hi Helge,


On Fri, May 27, 2022 at 10:58 PM Helge Deller <[email protected]> wrote:
>
> Hello Masahiro,
>
> On 5/27/22 12:01, Masahiro Yamada wrote:
> > parisc overrides 'nm' with a shell script. I do not know the reason,
> > but anyway it is how it has worked since 2003. [1]
>
> I don't know the reason either...
> I assume it was that the older toolchains had bugs and kept lots of local
> symbols like .LC? in the object files.
>
> I did a small build without the nm script (and removed it's reference
> in the Makefile), and it did not seem to break anything.
>
> > A problem is that this script returns the exit code of grep instead of
> > ${CROSS_COMPILE}nm.
>
> Instead of fixing this, I'd suggest that you simply remove the nm script
> alltogether. If you like I can apply such a patch in the parisc git tree,
> and you just drop this specific patch. Or you change your patch to remove it.
> Just let me know what you prefer.



This is a prerequisite of my kbuild work:
https://lore.kernel.org/linux-kbuild/[email protected]/T/#u

Without this, ARCH=parisc builds will be broken due to ${NM} returning 1.


I will send a patch to remove arch/parisc/nm.
Please give me your ack.

Thank you.




--
Best Regards
Masahiro Yamada

2022-05-28 20:02:34

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v7 7/8] kbuild: add cmd_and_savecmd macro

Separate out the command execution part of if_changed, as we did
for if_changed_dep.

This allows us to reuse it in if_changed_rule.

define rule_foo
$(call cmd_and_savecmd,foo)
$(call cmd,bar)
endef

Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>
Reviewed-by: Nicolas Schier <[email protected]>
Tested-by: Sedat Dilek <[email protected]> # LLVM-14 (x86-64)
---

(no changes since v4)

Changes in v4:
- New.
Resent of my previous submission.
https://lore.kernel.org/all/[email protected]/

scripts/Kbuild.include | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 455a0a6ce12d..ece44b735061 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -142,9 +142,11 @@ check-FORCE = $(if $(filter FORCE, $^),,$(warning FORCE prerequisite is missing)
if-changed-cond = $(newer-prereqs)$(cmd-check)$(check-FORCE)

# Execute command if command has changed or prerequisite(s) are updated.
-if_changed = $(if $(if-changed-cond), \
+if_changed = $(if $(if-changed-cond),$(cmd_and_savecmd),@:)
+
+cmd_and_savecmd = \
$(cmd); \
- printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
+ printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd

# Execute the command and also postprocess generated .d dependencies file.
if_changed_dep = $(if $(if-changed-cond),$(cmd_and_fixdep),@:)
--
2.32.0


2022-05-28 20:22:17

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH v7 2/8] kbuild: do not create *.prelink.o for Clang LTO or IBT

On Fri, May 27, 2022 at 07:01:49PM +0900, Masahiro Yamada wrote:
> New build flow
> ==============
>
> [1] single-object module
>
> Since there is only one object, there is no need to keep the LLVM IR.
> Use $(CC)+$(LD) to generate an ELF object in one build rule. When LTO
> is disabled, $(LD) is unneeded because $(CC) produces an ELF object.
>
> $(CC)+$(LD)+objtool $(LD)
> foo.c ----------------------------> foo.o ---------> foo.ko
> (ELF) |
> |
> foo.mod.o --/
>
> [2] multi-object module
>
> Previously, $(AR) was used to combine LLVM bitcode into an archive,
> but there was no technical reason to do so. Use $(LD) to merge them
> into a single ELF object.
>
> $(LD)
> $(CC) +objtool $(LD)
> foo1.c ---------> foo1.o ---------> foo.o ---------> foo.ko
> | (ELF) |
> foo2.c ---------> foo2.o ----/ |
> (LLVM IR) foo.mod.o --/
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> Reviewed-by: Nicolas Schier <[email protected]>
> Tested-by: Nathan Chancellor <[email protected]>
> Reviewed-by: Sami Tolvanen <[email protected]>
> Tested-by: Sedat Dilek <[email protected]> # LLVM-14 (x86-64)

Acked-by: Josh Poimboeuf <[email protected]>

--
Josh

2022-05-28 20:26:25

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v7 1/8] kbuild: replace $(linked-object) with CONFIG options

On Sat, May 28, 2022 at 11:32 AM Sedat Dilek <[email protected]> wrote:
>
> On Fri, May 27, 2022 at 1:56 PM Masahiro Yamada <[email protected]> wrote:
> >
> > *.prelink.o is created when CONFIG_LTO_CLANG or CONFIG_X86_KERNEL_IBT
> > is enabled.
> >
> > Replace $(linked-object) with $(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT)
> > so you will get better idea when the --link option is passed.
> >
> > No functional change is intended.
> >
> > Signed-off-by: Masahiro Yamada <[email protected]>
>
> Hi Masahiroy,
>
> I was not CCed on the cover-letter and only on some patches in this series.
>
> So, I have re-tested this series by pulling from kbuild.git#kbuild
> (saw later you renewed the parisc/nm patch).
>
> While testing my selfmade LLVM version 14.0.4 (PGO + ThinLTO
> optimized) toolchain I included this series.
>
> Just built fine and I was able to boot on bare metal.
>
> -Sedat -
>

Thanks for your testing!

I added your Tested-by tag.





> > ---
> >
> > Changes in v7:
> > - New patch
> >
> > scripts/Makefile.build | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index 06400504150b..f80196eef03a 100644
> > --- a/scripts/Makefile.build
> > +++ b/scripts/Makefile.build
> > @@ -219,7 +219,7 @@ objtool_args = \
> > $(if $(CONFIG_STACK_VALIDATION), --stackval) \
> > $(if $(CONFIG_HAVE_STATIC_CALL_INLINE), --static-call) \
> > --uaccess \
> > - $(if $(linked-object), --link) \
> > + $(if $($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT)), --link) \
> > $(if $(part-of-module), --module) \
> > $(if $(CONFIG_GCOV_KERNEL), --no-unreachable)
> >
> > @@ -284,7 +284,6 @@ quiet_cmd_cc_prelink_modules = LD [M] $@
> > # modules into native code
> > $(obj)/%.prelink.o: objtool-enabled = y
> > $(obj)/%.prelink.o: part-of-module := y
> > -$(obj)/%.prelink.o: linked-object := y
> >
> > $(obj)/%.prelink.o: $(obj)/%.o FORCE
> > $(call if_changed,cc_prelink_modules)
> > --
> > 2.32.0
> >



--
Best Regards
Masahiro Yamada

2022-05-28 20:26:30

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v7 6/8] kbuild: make *.mod rule robust against too long argument error

Like built-in.a, the command length of the *.mod rule scales with
the depth of the directory times the number of objects in the Makefile.

Add $(obj)/ by the shell command (awk) instead of by Make's builtin
function.

In-tree modules still have some room to the limit (ARG_MAX=2097152),
but this is more future-proof for big modules in a deep directory.

For example, you can build i915 as a module (CONFIG_DRM_I915=m) and
compare drivers/gpu/drm/i915/.i915.mod.cmd with/without this commit.

The issue is more critical for external modules because the M= path
can be very long as Jeff Johnson reported before [1].

[1] https://lore.kernel.org/linux-kbuild/[email protected]/

Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-by: Nicolas Schier <[email protected]>
Tested-by: Nathan Chancellor <[email protected]>
Tested-by: Sedat Dilek <[email protected]> # LLVM-14 (x86-64)
---

Changes in v7:
- Add comment to the code

scripts/Makefile.build | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 014ebee70fd6..31feb798e16e 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -274,8 +274,10 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
$(call if_changed_rule,cc_o_c)
$(call cmd,force_checksrc)

-cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) | \
- $(AWK) -v RS='( |\n)' '!x[$$0]++' > $@
+# To make this rule robust against "Argument list too long" error,
+# ensure to add $(obj)/ prefix by a shell command.
+cmd_mod = echo $(call real-search, $*.o, .o, -objs -y -m) | \
+ $(AWK) -v RS='( |\n)' '!x[$$0]++ { print("$(obj)/"$$0) }' > $@

$(obj)/%.mod: FORCE
$(call if_changed,mod)
--
2.32.0


2022-05-28 20:34:36

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH v7 1/8] kbuild: replace $(linked-object) with CONFIG options

On Fri, May 27, 2022 at 1:56 PM Masahiro Yamada <[email protected]> wrote:
>
> *.prelink.o is created when CONFIG_LTO_CLANG or CONFIG_X86_KERNEL_IBT
> is enabled.
>
> Replace $(linked-object) with $(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT)
> so you will get better idea when the --link option is passed.
>
> No functional change is intended.
>
> Signed-off-by: Masahiro Yamada <[email protected]>

Hi Masahiroy,

I was not CCed on the cover-letter and only on some patches in this series.

So, I have re-tested this series by pulling from kbuild.git#kbuild
(saw later you renewed the parisc/nm patch).

While testing my selfmade LLVM version 14.0.4 (PGO + ThinLTO
optimized) toolchain I included this series.

Just built fine and I was able to boot on bare metal.

-Sedat -

> ---
>
> Changes in v7:
> - New patch
>
> scripts/Makefile.build | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 06400504150b..f80196eef03a 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -219,7 +219,7 @@ objtool_args = \
> $(if $(CONFIG_STACK_VALIDATION), --stackval) \
> $(if $(CONFIG_HAVE_STATIC_CALL_INLINE), --static-call) \
> --uaccess \
> - $(if $(linked-object), --link) \
> + $(if $($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT)), --link) \
> $(if $(part-of-module), --module) \
> $(if $(CONFIG_GCOV_KERNEL), --no-unreachable)
>
> @@ -284,7 +284,6 @@ quiet_cmd_cc_prelink_modules = LD [M] $@
> # modules into native code
> $(obj)/%.prelink.o: objtool-enabled = y
> $(obj)/%.prelink.o: part-of-module := y
> -$(obj)/%.prelink.o: linked-object := y
>
> $(obj)/%.prelink.o: $(obj)/%.o FORCE
> $(call if_changed,cc_prelink_modules)
> --
> 2.32.0
>