linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/5] modpost: get the *.mod file path more simply
@ 2021-08-28  9:50 Masahiro Yamada
  2021-08-28  9:51 ` [PATCH 2/5] kbuild: detect objtool changes correctly without .SECONDEXPANSION Masahiro Yamada
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Masahiro Yamada @ 2021-08-28  9:50 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Masahiro Yamada, Michal Marek, Nick Desaulniers, linux-kernel

get_src_version() strips 'o' or 'lto.o' from the end of the object file
path (so, postfixlen is 1 or 5), then adds 'mod'.

If you look at the code closely, mod->name already holds the base path
with the extension stripped.

Most of the code changes made by commit 7ac204b545f2 ("modpost: lto:
strip .lto from module names") was actually unneeded.

sumversion.c does not need strends(), so it can get back local in
modpost.c again.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c    | 11 ++++++++++-
 scripts/mod/modpost.h    |  9 ---------
 scripts/mod/sumversion.c |  7 +------
 3 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 270a7df898e2..a26139aa57fd 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -17,6 +17,7 @@
 #include <ctype.h>
 #include <string.h>
 #include <limits.h>
+#include <stdbool.h>
 #include <errno.h>
 #include "modpost.h"
 #include "../../include/linux/license.h"
@@ -89,6 +90,14 @@ modpost_log(enum loglevel loglevel, const char *fmt, ...)
 		error_occurred = true;
 }
 
+static inline bool strends(const char *str, const char *postfix)
+{
+	if (strlen(str) < strlen(postfix))
+		return false;
+
+	return strcmp(str + strlen(str) - strlen(postfix), postfix) == 0;
+}
+
 void *do_nofail(void *ptr, const char *expr)
 {
 	if (!ptr)
@@ -2060,7 +2069,7 @@ static void read_symbols(const char *modname)
 	if (!mod->is_vmlinux) {
 		version = get_modinfo(&info, "version");
 		if (version || all_versions)
-			get_src_version(modname, mod->srcversion,
+			get_src_version(mod->name, mod->srcversion,
 					sizeof(mod->srcversion) - 1);
 	}
 
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index c1a895c0d682..0c47ff95c0e2 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -2,7 +2,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdarg.h>
-#include <stdbool.h>
 #include <string.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -178,14 +177,6 @@ static inline unsigned int get_secindex(const struct elf_info *info,
 	return info->symtab_shndx_start[sym - info->symtab_start];
 }
 
-static inline bool strends(const char *str, const char *postfix)
-{
-	if (strlen(str) < strlen(postfix))
-		return false;
-
-	return strcmp(str + strlen(str) - strlen(postfix), postfix) == 0;
-}
-
 /* file2alias.c */
 extern unsigned int cross_build;
 void handle_moddevtable(struct module *mod, struct elf_info *info,
diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
index 760e6baa7eda..905c0ec291e1 100644
--- a/scripts/mod/sumversion.c
+++ b/scripts/mod/sumversion.c
@@ -391,14 +391,9 @@ void get_src_version(const char *modname, char sum[], unsigned sumlen)
 	struct md4_ctx md;
 	char *fname;
 	char filelist[PATH_MAX + 1];
-	int postfix_len = 1;
-
-	if (strends(modname, ".lto.o"))
-		postfix_len = 5;
 
 	/* objects for a module are listed in the first line of *.mod file. */
-	snprintf(filelist, sizeof(filelist), "%.*smod",
-		 (int)strlen(modname) - postfix_len, modname);
+	snprintf(filelist, sizeof(filelist), "%s.mod", modname);
 
 	buf = read_text_file(filelist);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/5] kbuild: detect objtool changes correctly without .SECONDEXPANSION
  2021-08-28  9:50 [PATCH 1/5] modpost: get the *.mod file path more simply Masahiro Yamada
@ 2021-08-28  9:51 ` Masahiro Yamada
  2021-08-31  1:43   ` Masahiro Yamada
  2021-08-28  9:51 ` [PATCH 3/5] kbuild: clean up objtool_args slightly Masahiro Yamada
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2021-08-28  9:51 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Masahiro Yamada, Michal Marek, Nick Desaulniers, linux-kernel

This reverts commit 8852c5524029 ("kbuild: Fix objtool dependency for
'OBJECT_FILES_NON_STANDARD_<obj> := n'"), and fixes the dependency in
a cleaner, more precise way.

Using .SECONDEXPANSION is expensive since Makefile.build is parsed
twice every time, and the escaping dollars makes the code unreadable.

Adding include/config/* as dependency is not maintainable either because
objtool_args is dependent on more CONFIG options.

A better fix is to include the objtool command in *.cmd files so any
command change is naturally detected by if_change.

Also, include the md5sum of objtool into *.cmd files so any change in
the objtool executable will result in rebuilding all objects that depend
on objtool.

This allows us to drop $(objtool_deps) entirely.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 Makefile                |  3 ++-
 scripts/Makefile.build  | 26 +++++++++-----------------
 scripts/link-vmlinux.sh |  3 ++-
 3 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/Makefile b/Makefile
index 829bd339ffdc..3ef3685b7e4a 100644
--- a/Makefile
+++ b/Makefile
@@ -1858,7 +1858,8 @@ descend: $(build-dirs)
 $(build-dirs): prepare
 	$(Q)$(MAKE) $(build)=$@ \
 	single-build=$(if $(filter-out $@/, $(filter $@/%, $(KBUILD_SINGLE_TARGETS))),1) \
-	need-builtin=1 need-modorder=1
+	need-builtin=1 need-modorder=1 \
+	$(if $(CONFIG_STACK_VALIDATION),objtool-md5sum=$(firstword $(shell md5sum tools/objtool/objtool)))
 
 clean-dirs := $(addprefix _clean_, $(clean-dirs))
 PHONY += $(clean-dirs) clean
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 3efc984d4c69..8aa6eaa4bf21 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -155,7 +155,7 @@ $(obj)/%.ll: $(src)/%.c FORCE
 # (See cmd_cc_o_c + relevant part of rule_cc_o_c)
 
 quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
-      cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $<
+      cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $< $(cmd_objtool)
 
 ifdef CONFIG_MODVERSIONS
 # When module versioning is enabled the following steps are executed:
@@ -226,26 +226,21 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
 ifdef CONFIG_STACK_VALIDATION
 ifndef CONFIG_LTO_CLANG
 
-__objtool_obj := $(objtree)/tools/objtool/objtool
+objtool := $(objtree)/tools/objtool/objtool
 
 # '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
+#
+# Record the md5sum of the objtool executable so any change in it results in
+# rebuilding objects.
 cmd_objtool = $(if $(patsubst y%,, \
 	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
-	$(__objtool_obj) $(objtool_args) $@)
-objtool_obj = $(if $(patsubst y%,, \
-	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
-	$(__objtool_obj))
+	; : $(objtool-md5sum) ; $(objtool) $(objtool_args) $@)
 
 endif # CONFIG_LTO_CLANG
 endif # CONFIG_STACK_VALIDATION
 
-# Rebuild all objects when objtool changes, or is enabled/disabled.
-objtool_dep = $(objtool_obj)					\
-	      $(wildcard include/config/ORC_UNWINDER		\
-			 include/config/STACK_VALIDATION)
-
 ifdef CONFIG_TRIM_UNUSED_KSYMS
 cmd_gen_ksymdeps = \
 	$(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
@@ -259,7 +254,6 @@ define rule_cc_o_c
 	$(call cmd,gen_ksymdeps)
 	$(call cmd,checksrc)
 	$(call cmd,checkdoc)
-	$(call cmd,objtool)
 	$(call cmd,modversions_c)
 	$(call cmd,record_mcount)
 endef
@@ -267,13 +261,11 @@ endef
 define rule_as_o_S
 	$(call cmd_and_fixdep,as_o_S)
 	$(call cmd,gen_ksymdeps)
-	$(call cmd,objtool)
 	$(call cmd,modversions_S)
 endef
 
 # Built-in and composite module parts
-.SECONDEXPANSION:
-$(obj)/%.o: $(src)/%.c $(recordmcount_source) $$(objtool_dep) FORCE
+$(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
 	$(call if_changed_rule,cc_o_c)
 	$(call cmd,force_checksrc)
 
@@ -356,7 +348,7 @@ $(obj)/%.s: $(src)/%.S FORCE
 	$(call if_changed_dep,cpp_s_S)
 
 quiet_cmd_as_o_S = AS $(quiet_modtag)  $@
-      cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $<
+      cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $< $(cmd_objtool)
 
 ifdef CONFIG_ASM_MODVERSIONS
 
@@ -375,7 +367,7 @@ cmd_modversions_S =								\
 	fi
 endif
 
-$(obj)/%.o: $(src)/%.S $$(objtool_dep) FORCE
+$(obj)/%.o: $(src)/%.S FORCE
 	$(call if_changed_rule,as_o_S)
 
 targets += $(filter-out $(subdir-builtin), $(real-obj-y))
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index d74cee5c4326..58b3a94c934b 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -335,7 +335,8 @@ else
 fi;
 
 # final build of init/
-${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init need-builtin=1
+${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init need-builtin=1 \
+	${CONFIG_STACK_VALIDATION:+objtool-md5sum=$(md5sum tools/objtool/objtool | cut -d ' ' -f1)}
 
 #link vmlinux.o
 modpost_link vmlinux.o
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/5] kbuild: clean up objtool_args slightly
  2021-08-28  9:50 [PATCH 1/5] modpost: get the *.mod file path more simply Masahiro Yamada
  2021-08-28  9:51 ` [PATCH 2/5] kbuild: detect objtool changes correctly without .SECONDEXPANSION Masahiro Yamada
@ 2021-08-28  9:51 ` Masahiro Yamada
  2021-08-31  1:41   ` Masahiro Yamada
  2021-08-28  9:51 ` [PATCH 4/5] kbuild: merge objtool_args into objtool in scripts/Makefile.build Masahiro Yamada
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2021-08-28  9:51 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Masahiro Yamada, Michal Marek, Nick Desaulniers, linux-kernel

The code:

  $(if $(or $(CONFIG_GCOV_KERNEL),$(CONFIG_LTO_CLANG)), ...)

... can be simpled to:

  $(if $(CONFIG_GCOV_KERNEL)$(CONFIG_LTO_CLANG), ...)

Also, remove meaningless commas at the end of $(if ...).

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.lib | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index af1c920a585c..cd011f3f6f78 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -236,13 +236,12 @@ endif
 # then here to avoid duplication.
 objtool_args =								\
 	$(if $(CONFIG_UNWINDER_ORC),orc generate,check)			\
-	$(if $(part-of-module), --module,)				\
+	$(if $(part-of-module), --module)				\
 	$(if $(CONFIG_FRAME_POINTER),, --no-fp)				\
-	$(if $(or $(CONFIG_GCOV_KERNEL),$(CONFIG_LTO_CLANG)), 		\
-		--no-unreachable,)					\
-	$(if $(CONFIG_RETPOLINE), --retpoline,)				\
-	$(if $(CONFIG_X86_SMAP), --uaccess,)				\
-	$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount,)
+	$(if $(CONFIG_GCOV_KERNEL)$(CONFIG_LTO_CLANG), --no-unreachable)\
+	$(if $(CONFIG_RETPOLINE), --retpoline)				\
+	$(if $(CONFIG_X86_SMAP), --uaccess)				\
+	$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount)
 
 # Useful for describing the dependency of composite objects
 # Usage:
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/5] kbuild: merge objtool_args into objtool in scripts/Makefile.build
  2021-08-28  9:50 [PATCH 1/5] modpost: get the *.mod file path more simply Masahiro Yamada
  2021-08-28  9:51 ` [PATCH 2/5] kbuild: detect objtool changes correctly without .SECONDEXPANSION Masahiro Yamada
  2021-08-28  9:51 ` [PATCH 3/5] kbuild: clean up objtool_args slightly Masahiro Yamada
@ 2021-08-28  9:51 ` Masahiro Yamada
  2021-08-28  9:51 ` [PATCH 5/5] kbuild: rebuild *.lto.o when objtool is changed Masahiro Yamada
  2021-08-31  1:41 ` [PATCH 1/5] modpost: get the *.mod file path more simply Masahiro Yamada
  4 siblings, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2021-08-28  9:51 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Masahiro Yamada, Michal Marek, Nick Desaulniers, linux-kernel

Commit b1a1a1a09b46 ("kbuild: lto: postpone objtool") moved objtool_args
to Makefile.lib, so the arguments can be used in Makefile.modfinal as
well as Makefile.build.

With commit 2b1d7fc05467 ("kbuild: Fix TRIM_UNUSED_KSYMS with
LTO_CLANG"), module LTO linking came back to scripts/Makefile.build
again.

So, there is no more reason to keep objtool_args in a separate file.

Move it to scripts/Makefile.build and merge into the 'objtool' variable.

You might wonder why cmd_cc_lto_link_modules adds --module again. Add
a small comment to explain it.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.build | 19 ++++++++++++++-----
 scripts/Makefile.lib   | 11 -----------
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 8aa6eaa4bf21..cc0c494a48d3 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -224,9 +224,17 @@ cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)),
 endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
 
 ifdef CONFIG_STACK_VALIDATION
-ifndef CONFIG_LTO_CLANG
 
-objtool := $(objtree)/tools/objtool/objtool
+objtool = $(objtree)/tools/objtool/objtool				\
+	$(if $(CONFIG_UNWINDER_ORC),orc generate,check)			\
+	$(if $(part-of-module), --module)				\
+	$(if $(CONFIG_FRAME_POINTER),, --no-fp)				\
+	$(if $(CONFIG_GCOV_KERNEL)$(CONFIG_LTO_CLANG), --no-unreachable)\
+	$(if $(CONFIG_RETPOLINE), --retpoline)				\
+	$(if $(CONFIG_X86_SMAP), --uaccess)				\
+	$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount)
+
+ifndef CONFIG_LTO_CLANG
 
 # 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
 # 'OBJECT_FILES_NON_STANDARD_foo.o := 'y': skip objtool checking for a file
@@ -236,7 +244,7 @@ objtool := $(objtree)/tools/objtool/objtool
 # rebuilding objects.
 cmd_objtool = $(if $(patsubst y%,, \
 	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
-	; : $(objtool-md5sum) ; $(objtool) $(objtool_args) $@)
+	; : $(objtool-md5sum) ; $(objtool) $@)
 
 endif # CONFIG_LTO_CLANG
 endif # CONFIG_STACK_VALIDATION
@@ -282,8 +290,9 @@ cmd_cc_lto_link_modules =						\
 ifdef CONFIG_STACK_VALIDATION
 # objtool was skipped for LLVM bitcode, run it now that we have compiled
 # modules into native code
-cmd_cc_lto_link_modules += ;						\
-	$(objtree)/tools/objtool/objtool $(objtool_args) --module $@
+#
+# Repeat --module because $(part-of-module) does not work here.
+cmd_cc_lto_link_modules += ; $(objtool) --module $@
 endif
 
 $(obj)/%.lto.o: $(obj)/%.o FORCE
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index cd011f3f6f78..34c4c11c4bc1 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -232,17 +232,6 @@ ifeq ($(CONFIG_LTO_CLANG),y)
 mod-prelink-ext := .lto
 endif
 
-# Objtool arguments are also needed for modfinal with LTO, so we define
-# then here to avoid duplication.
-objtool_args =								\
-	$(if $(CONFIG_UNWINDER_ORC),orc generate,check)			\
-	$(if $(part-of-module), --module)				\
-	$(if $(CONFIG_FRAME_POINTER),, --no-fp)				\
-	$(if $(CONFIG_GCOV_KERNEL)$(CONFIG_LTO_CLANG), --no-unreachable)\
-	$(if $(CONFIG_RETPOLINE), --retpoline)				\
-	$(if $(CONFIG_X86_SMAP), --uaccess)				\
-	$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount)
-
 # Useful for describing the dependency of composite objects
 # Usage:
 #   $(call multi_depend, multi_used_targets, suffix_to_remove, suffix_to_add)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/5] kbuild: rebuild *.lto.o when objtool is changed
  2021-08-28  9:50 [PATCH 1/5] modpost: get the *.mod file path more simply Masahiro Yamada
                   ` (2 preceding siblings ...)
  2021-08-28  9:51 ` [PATCH 4/5] kbuild: merge objtool_args into objtool in scripts/Makefile.build Masahiro Yamada
@ 2021-08-28  9:51 ` Masahiro Yamada
  2021-08-31  1:41 ` [PATCH 1/5] modpost: get the *.mod file path more simply Masahiro Yamada
  4 siblings, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2021-08-28  9:51 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Masahiro Yamada, Michal Marek, Nick Desaulniers, linux-kernel

When objtool is changed, all objects are rebuilt, but this check
is missing in the %.lto.o rule.

Move the objtool-md5sum into the objtool variable so the updated
objtool is detected when CONFIG_LTO_CLANG=y as well.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.build | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index cc0c494a48d3..790b72208668 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -225,7 +225,10 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
 
 ifdef CONFIG_STACK_VALIDATION
 
-objtool = $(objtree)/tools/objtool/objtool				\
+# Record the md5sum of the objtool executable so any change in it results in
+# rebuilding objects.
+objtool = : $(objtool-md5sum) ;						\
+	$(objtree)/tools/objtool/objtool				\
 	$(if $(CONFIG_UNWINDER_ORC),orc generate,check)			\
 	$(if $(part-of-module), --module)				\
 	$(if $(CONFIG_FRAME_POINTER),, --no-fp)				\
@@ -239,12 +242,9 @@ ifndef CONFIG_LTO_CLANG
 # '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
-#
-# Record the md5sum of the objtool executable so any change in it results in
-# rebuilding objects.
 cmd_objtool = $(if $(patsubst y%,, \
 	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
-	; : $(objtool-md5sum) ; $(objtool) $@)
+	; $(objtool) $@)
 
 endif # CONFIG_LTO_CLANG
 endif # CONFIG_STACK_VALIDATION
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/5] modpost: get the *.mod file path more simply
  2021-08-28  9:50 [PATCH 1/5] modpost: get the *.mod file path more simply Masahiro Yamada
                   ` (3 preceding siblings ...)
  2021-08-28  9:51 ` [PATCH 5/5] kbuild: rebuild *.lto.o when objtool is changed Masahiro Yamada
@ 2021-08-31  1:41 ` Masahiro Yamada
  4 siblings, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2021-08-31  1:41 UTC (permalink / raw)
  To: Linux Kbuild mailing list
  Cc: Michal Marek, Nick Desaulniers, Linux Kernel Mailing List

On Sat, Aug 28, 2021 at 6:51 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> get_src_version() strips 'o' or 'lto.o' from the end of the object file
> path (so, postfixlen is 1 or 5), then adds 'mod'.
>
> If you look at the code closely, mod->name already holds the base path
> with the extension stripped.
>
> Most of the code changes made by commit 7ac204b545f2 ("modpost: lto:
> strip .lto from module names") was actually unneeded.
>
> sumversion.c does not need strends(), so it can get back local in
> modpost.c again.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---


Applied this to linux-kbuild.
(only 1/5 and 3/5)






>  scripts/mod/modpost.c    | 11 ++++++++++-
>  scripts/mod/modpost.h    |  9 ---------
>  scripts/mod/sumversion.c |  7 +------
>  3 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 270a7df898e2..a26139aa57fd 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -17,6 +17,7 @@
>  #include <ctype.h>
>  #include <string.h>
>  #include <limits.h>
> +#include <stdbool.h>
>  #include <errno.h>
>  #include "modpost.h"
>  #include "../../include/linux/license.h"
> @@ -89,6 +90,14 @@ modpost_log(enum loglevel loglevel, const char *fmt, ...)
>                 error_occurred = true;
>  }
>
> +static inline bool strends(const char *str, const char *postfix)
> +{
> +       if (strlen(str) < strlen(postfix))
> +               return false;
> +
> +       return strcmp(str + strlen(str) - strlen(postfix), postfix) == 0;
> +}
> +
>  void *do_nofail(void *ptr, const char *expr)
>  {
>         if (!ptr)
> @@ -2060,7 +2069,7 @@ static void read_symbols(const char *modname)
>         if (!mod->is_vmlinux) {
>                 version = get_modinfo(&info, "version");
>                 if (version || all_versions)
> -                       get_src_version(modname, mod->srcversion,
> +                       get_src_version(mod->name, mod->srcversion,
>                                         sizeof(mod->srcversion) - 1);
>         }
>
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index c1a895c0d682..0c47ff95c0e2 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -2,7 +2,6 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <stdarg.h>
> -#include <stdbool.h>
>  #include <string.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
> @@ -178,14 +177,6 @@ static inline unsigned int get_secindex(const struct elf_info *info,
>         return info->symtab_shndx_start[sym - info->symtab_start];
>  }
>
> -static inline bool strends(const char *str, const char *postfix)
> -{
> -       if (strlen(str) < strlen(postfix))
> -               return false;
> -
> -       return strcmp(str + strlen(str) - strlen(postfix), postfix) == 0;
> -}
> -
>  /* file2alias.c */
>  extern unsigned int cross_build;
>  void handle_moddevtable(struct module *mod, struct elf_info *info,
> diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
> index 760e6baa7eda..905c0ec291e1 100644
> --- a/scripts/mod/sumversion.c
> +++ b/scripts/mod/sumversion.c
> @@ -391,14 +391,9 @@ void get_src_version(const char *modname, char sum[], unsigned sumlen)
>         struct md4_ctx md;
>         char *fname;
>         char filelist[PATH_MAX + 1];
> -       int postfix_len = 1;
> -
> -       if (strends(modname, ".lto.o"))
> -               postfix_len = 5;
>
>         /* objects for a module are listed in the first line of *.mod file. */
> -       snprintf(filelist, sizeof(filelist), "%.*smod",
> -                (int)strlen(modname) - postfix_len, modname);
> +       snprintf(filelist, sizeof(filelist), "%s.mod", modname);
>
>         buf = read_text_file(filelist);
>
> --
> 2.30.2
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/5] kbuild: clean up objtool_args slightly
  2021-08-28  9:51 ` [PATCH 3/5] kbuild: clean up objtool_args slightly Masahiro Yamada
@ 2021-08-31  1:41   ` Masahiro Yamada
  0 siblings, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2021-08-31  1:41 UTC (permalink / raw)
  To: Linux Kbuild mailing list
  Cc: Michal Marek, Nick Desaulniers, Linux Kernel Mailing List

On Sat, Aug 28, 2021 at 6:51 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The code:
>
>   $(if $(or $(CONFIG_GCOV_KERNEL),$(CONFIG_LTO_CLANG)), ...)
>
> ... can be simpled to:
>
>   $(if $(CONFIG_GCOV_KERNEL)$(CONFIG_LTO_CLANG), ...)
>
> Also, remove meaningless commas at the end of $(if ...).
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---


Applied this to linux-kbuild.
(only 1/5 and 3/5)



>  scripts/Makefile.lib | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index af1c920a585c..cd011f3f6f78 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -236,13 +236,12 @@ endif
>  # then here to avoid duplication.
>  objtool_args =                                                         \
>         $(if $(CONFIG_UNWINDER_ORC),orc generate,check)                 \
> -       $(if $(part-of-module), --module,)                              \
> +       $(if $(part-of-module), --module)                               \
>         $(if $(CONFIG_FRAME_POINTER),, --no-fp)                         \
> -       $(if $(or $(CONFIG_GCOV_KERNEL),$(CONFIG_LTO_CLANG)),           \
> -               --no-unreachable,)                                      \
> -       $(if $(CONFIG_RETPOLINE), --retpoline,)                         \
> -       $(if $(CONFIG_X86_SMAP), --uaccess,)                            \
> -       $(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount,)
> +       $(if $(CONFIG_GCOV_KERNEL)$(CONFIG_LTO_CLANG), --no-unreachable)\
> +       $(if $(CONFIG_RETPOLINE), --retpoline)                          \
> +       $(if $(CONFIG_X86_SMAP), --uaccess)                             \
> +       $(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount)
>
>  # Useful for describing the dependency of composite objects
>  # Usage:
> --
> 2.30.2
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/5] kbuild: detect objtool changes correctly without .SECONDEXPANSION
  2021-08-28  9:51 ` [PATCH 2/5] kbuild: detect objtool changes correctly without .SECONDEXPANSION Masahiro Yamada
@ 2021-08-31  1:43   ` Masahiro Yamada
  0 siblings, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2021-08-31  1:43 UTC (permalink / raw)
  To: Linux Kbuild mailing list
  Cc: Michal Marek, Nick Desaulniers, Linux Kernel Mailing List

On Sat, Aug 28, 2021 at 6:51 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> This reverts commit 8852c5524029 ("kbuild: Fix objtool dependency for
> 'OBJECT_FILES_NON_STANDARD_<obj> := n'"), and fixes the dependency in
> a cleaner, more precise way.
>
> Using .SECONDEXPANSION is expensive since Makefile.build is parsed
> twice every time, and the escaping dollars makes the code unreadable.
>
> Adding include/config/* as dependency is not maintainable either because
> objtool_args is dependent on more CONFIG options.
>
> A better fix is to include the objtool command in *.cmd files so any
> command change is naturally detected by if_change.
>
> Also, include the md5sum of objtool into *.cmd files so any change in
> the objtool executable will result in rebuilding all objects that depend
> on objtool.


After more consideration, I decided not to do this.
(I retract 2/5, 4/5, 5/5 from this series).

I came up with a cleaner patch set.

I will send it later.








> This allows us to drop $(objtool_deps) entirely.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  Makefile                |  3 ++-
>  scripts/Makefile.build  | 26 +++++++++-----------------
>  scripts/link-vmlinux.sh |  3 ++-
>  3 files changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 829bd339ffdc..3ef3685b7e4a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1858,7 +1858,8 @@ descend: $(build-dirs)
>  $(build-dirs): prepare
>         $(Q)$(MAKE) $(build)=$@ \
>         single-build=$(if $(filter-out $@/, $(filter $@/%, $(KBUILD_SINGLE_TARGETS))),1) \
> -       need-builtin=1 need-modorder=1
> +       need-builtin=1 need-modorder=1 \
> +       $(if $(CONFIG_STACK_VALIDATION),objtool-md5sum=$(firstword $(shell md5sum tools/objtool/objtool)))
>
>  clean-dirs := $(addprefix _clean_, $(clean-dirs))
>  PHONY += $(clean-dirs) clean
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 3efc984d4c69..8aa6eaa4bf21 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -155,7 +155,7 @@ $(obj)/%.ll: $(src)/%.c FORCE
>  # (See cmd_cc_o_c + relevant part of rule_cc_o_c)
>
>  quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
> -      cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $<
> +      cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $< $(cmd_objtool)
>
>  ifdef CONFIG_MODVERSIONS
>  # When module versioning is enabled the following steps are executed:
> @@ -226,26 +226,21 @@ endif # CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
>  ifdef CONFIG_STACK_VALIDATION
>  ifndef CONFIG_LTO_CLANG
>
> -__objtool_obj := $(objtree)/tools/objtool/objtool
> +objtool := $(objtree)/tools/objtool/objtool
>
>  # '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
> +#
> +# Record the md5sum of the objtool executable so any change in it results in
> +# rebuilding objects.
>  cmd_objtool = $(if $(patsubst y%,, \
>         $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
> -       $(__objtool_obj) $(objtool_args) $@)
> -objtool_obj = $(if $(patsubst y%,, \
> -       $(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
> -       $(__objtool_obj))
> +       ; : $(objtool-md5sum) ; $(objtool) $(objtool_args) $@)
>
>  endif # CONFIG_LTO_CLANG
>  endif # CONFIG_STACK_VALIDATION
>
> -# Rebuild all objects when objtool changes, or is enabled/disabled.
> -objtool_dep = $(objtool_obj)                                   \
> -             $(wildcard include/config/ORC_UNWINDER            \
> -                        include/config/STACK_VALIDATION)
> -
>  ifdef CONFIG_TRIM_UNUSED_KSYMS
>  cmd_gen_ksymdeps = \
>         $(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
> @@ -259,7 +254,6 @@ define rule_cc_o_c
>         $(call cmd,gen_ksymdeps)
>         $(call cmd,checksrc)
>         $(call cmd,checkdoc)
> -       $(call cmd,objtool)
>         $(call cmd,modversions_c)
>         $(call cmd,record_mcount)
>  endef
> @@ -267,13 +261,11 @@ endef
>  define rule_as_o_S
>         $(call cmd_and_fixdep,as_o_S)
>         $(call cmd,gen_ksymdeps)
> -       $(call cmd,objtool)
>         $(call cmd,modversions_S)
>  endef
>
>  # Built-in and composite module parts
> -.SECONDEXPANSION:
> -$(obj)/%.o: $(src)/%.c $(recordmcount_source) $$(objtool_dep) FORCE
> +$(obj)/%.o: $(src)/%.c $(recordmcount_source) FORCE
>         $(call if_changed_rule,cc_o_c)
>         $(call cmd,force_checksrc)
>
> @@ -356,7 +348,7 @@ $(obj)/%.s: $(src)/%.S FORCE
>         $(call if_changed_dep,cpp_s_S)
>
>  quiet_cmd_as_o_S = AS $(quiet_modtag)  $@
> -      cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $<
> +      cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $< $(cmd_objtool)
>
>  ifdef CONFIG_ASM_MODVERSIONS
>
> @@ -375,7 +367,7 @@ cmd_modversions_S =                                                         \
>         fi
>  endif
>
> -$(obj)/%.o: $(src)/%.S $$(objtool_dep) FORCE
> +$(obj)/%.o: $(src)/%.S FORCE
>         $(call if_changed_rule,as_o_S)
>
>  targets += $(filter-out $(subdir-builtin), $(real-obj-y))
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index d74cee5c4326..58b3a94c934b 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -335,7 +335,8 @@ else
>  fi;
>
>  # final build of init/
> -${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init need-builtin=1
> +${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init need-builtin=1 \
> +       ${CONFIG_STACK_VALIDATION:+objtool-md5sum=$(md5sum tools/objtool/objtool | cut -d ' ' -f1)}
>
>  #link vmlinux.o
>  modpost_link vmlinux.o
> --
> 2.30.2
>


--
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-08-31  1:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-28  9:50 [PATCH 1/5] modpost: get the *.mod file path more simply Masahiro Yamada
2021-08-28  9:51 ` [PATCH 2/5] kbuild: detect objtool changes correctly without .SECONDEXPANSION Masahiro Yamada
2021-08-31  1:43   ` Masahiro Yamada
2021-08-28  9:51 ` [PATCH 3/5] kbuild: clean up objtool_args slightly Masahiro Yamada
2021-08-31  1:41   ` Masahiro Yamada
2021-08-28  9:51 ` [PATCH 4/5] kbuild: merge objtool_args into objtool in scripts/Makefile.build Masahiro Yamada
2021-08-28  9:51 ` [PATCH 5/5] kbuild: rebuild *.lto.o when objtool is changed Masahiro Yamada
2021-08-31  1:41 ` [PATCH 1/5] modpost: get the *.mod file path more simply Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).