All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.