diff mbox series

[v5,18/19] kbuild: Add gendwarfksyms as an alternative to genksyms

Message ID 20241030170106.1501763-39-samitolvanen@google.com (mailing list archive)
State Handled Elsewhere
Headers show
Series Implement DWARF modversions | expand

Commit Message

Sami Tolvanen Oct. 30, 2024, 5:01 p.m. UTC 
When MODVERSIONS is enabled, allow selecting gendwarfksyms as the
implementation, but default to genksyms.

Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Acked-by: Neal Gompa <neal@gompa.dev>
---
 kernel/module/Kconfig  | 25 ++++++++++++++++++++++++-
 scripts/Makefile       |  2 +-
 scripts/Makefile.build | 41 +++++++++++++++++++++++++++++++++--------
 3 files changed, 58 insertions(+), 10 deletions(-)

Comments

Masahiro Yamada Nov. 12, 2024, 4:08 a.m. UTC | #1 
On Thu, Oct 31, 2024 at 2:01 AM Sami Tolvanen <samitolvanen@google.com> wrote:
>
> When MODVERSIONS is enabled, allow selecting gendwarfksyms as the
> implementation, but default to genksyms.
>
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> Acked-by: Neal Gompa <neal@gompa.dev>
> ---
>  kernel/module/Kconfig  | 25 ++++++++++++++++++++++++-
>  scripts/Makefile       |  2 +-
>  scripts/Makefile.build | 41 +++++++++++++++++++++++++++++++++--------
>  3 files changed, 58 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
> index f9e5f82fa88b..e6b2427e5c19 100644
> --- a/kernel/module/Kconfig
> +++ b/kernel/module/Kconfig
> @@ -169,13 +169,36 @@ config MODVERSIONS
>           make them incompatible with the kernel you are running.  If
>           unsure, say N.
>
> +choice
> +       prompt "Module versioning implementation"
> +       depends on MODVERSIONS
> +       default GENKSYMS
> +       help
> +         Select the tool used to calculate symbol versions for modules.
> +
> +         If unsure, select GENKSYMS.
> +
> +config GENKSYMS
> +       bool "genksyms (from source code)"
> +       help
> +         Calculate symbol versions from pre-processed source code using
> +         genksyms.
> +
> +         If unsure, say Y.
> +
>  config GENDWARFKSYMS
> -       bool
> +       bool "gendwarfksyms (from debugging information)"
>         depends on DEBUG_INFO
>         # Requires full debugging information, split DWARF not supported.
>         depends on !DEBUG_INFO_REDUCED && !DEBUG_INFO_SPLIT
>         # Requires ELF object files.
>         depends on !LTO
> +       help
> +         Calculate symbol versions from DWARF debugging information using
> +         gendwarfksyms. Requires DEBUG_INFO to be enabled.
> +
> +         If unsure, say N.
> +endchoice
>
>  config ASM_MODVERSIONS
>         bool
> diff --git a/scripts/Makefile b/scripts/Makefile
> index d7fec46d38c0..8533f4498885 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -53,7 +53,7 @@ hostprogs += unifdef
>  targets += module.lds
>
>  subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
> -subdir-$(CONFIG_MODVERSIONS) += genksyms
> +subdir-$(CONFIG_GENKSYMS) += genksyms
>  subdir-$(CONFIG_GENDWARFKSYMS) += gendwarfksyms
>  subdir-$(CONFIG_SECURITY_SELINUX) += selinux
>  subdir-$(CONFIG_SECURITY_IPE) += ipe
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 8f423a1faf50..d2a0440cdb79 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -107,18 +107,31 @@ cmd_cpp_i_c       = $(CPP) $(c_flags) -o $@ $<
>  $(obj)/%.i: $(obj)/%.c FORCE
>         $(call if_changed_dep,cpp_i_c)
>
> +getexportsymbols = $(NM) $(1) | sed -n 's/.* __export_symbol_\(.*\)/$(2)/p'
> +
> +gendwarfksyms = scripts/gendwarfksyms/gendwarfksyms    \
> +       $(if $(1), --symtypes $(2))                     \
> +       $(if $(KBUILD_GENDWARFKSYMS_STABLE), --stable)
> +
>  genksyms = scripts/genksyms/genksyms           \
>         $(if $(1), -T $(2))                     \
>         $(if $(KBUILD_PRESERVE), -p)            \
>         -r $(or $(wildcard $(2:.symtypes=.symref)), /dev/null)
>
>  # These mirror gensymtypes_S and co below, keep them in synch.
> +ifdef CONFIG_GENDWARFKSYMS
> +symtypes_dep_c = $(obj)/%.o
> +cmd_gensymtypes_c = $(if $(skip_gendwarfksyms),,       \
> +       $(call getexportsymbols,$(2:.symtypes=.o),\1) | \
> +       $(gendwarfksyms) $(2:.symtypes=.o))
> +else
>  cmd_gensymtypes_c = $(CPP) -D__GENKSYMS__ $(c_flags) $< | $(genksyms)
> +endif # CONFIG_GENDWARFKSYMS
>
>  quiet_cmd_cc_symtypes_c = SYM $(quiet_modtag) $@
>        cmd_cc_symtypes_c = $(call cmd_gensymtypes_c,true,$@) >/dev/null
>
> -$(obj)/%.symtypes : $(obj)/%.c FORCE
> +$(obj)/%.symtypes : $(obj)/%.c $(symtypes_dep_c) FORCE
>         $(call cmd,cc_symtypes_c)
>
>  # LLVM assembly
> @@ -314,19 +327,31 @@ $(obj)/%.ll: $(obj)/%.rs FORCE
>  # This is convoluted. The .S file must first be preprocessed to run guards and
>  # expand names, then the resulting exports must be constructed into plain
>  # EXPORT_SYMBOL(symbol); to build our dummy C file, and that gets preprocessed
> -# to make the genksyms input.
> +# to make the genksyms input or compiled into an object for gendwarfksyms.
>  #
>  # These mirror gensymtypes_c and co above, keep them in synch.
> -cmd_gensymtypes_S =                                                         \
> -   { echo "\#include <linux/kernel.h>" ;                                    \
> -     echo "\#include <asm/asm-prototypes.h>" ;                              \
> -     $(NM) $@ | sed -n 's/.* __export_symbol_\(.*\)/EXPORT_SYMBOL(\1);/p' ; } | \
> -    $(CPP) -D__GENKSYMS__ $(c_flags) -xc - | $(genksyms)
> +getasmexports =                                                                \
> +   { echo "\#include <linux/kernel.h>" ;                               \
> +     echo "\#include <linux/string.h>" ;                               \
> +     echo "\#include <asm/asm-prototypes.h>" ;                         \
> +     $(call getexportsymbols,$(2:.symtypes=.o),EXPORT_SYMBOL(\1);) ; }
> +
> +ifdef CONFIG_GENDWARFKSYMS
> +cmd_gensymtypes_S =                                                    \
> +       $(getasmexports) |                                              \
> +       $(CC) $(c_flags) -c -o $(2:.symtypes=.gendwarfksyms.o) -xc -;   \
> +       $(call getexportsymbols,$(2:.symtypes=.o),\1) |                 \
> +       $(gendwarfksyms) $(2:.symtypes=.gendwarfksyms.o)


I do not want to see crazy suffix replacements like this.

I decided to delete this.
https://lore.kernel.org/linux-kbuild/20241111171753.2917697-2-masahiroy@kernel.org/T/#u
Sami Tolvanen Nov. 13, 2024, 5:48 p.m. UTC | #2 
Hi Masahiro,

On Mon, Nov 11, 2024 at 8:09 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Thu, Oct 31, 2024 at 2:01 AM Sami Tolvanen <samitolvanen@google.com> wrote:
> >
> >  # These mirror gensymtypes_c and co above, keep them in synch.
> > -cmd_gensymtypes_S =                                                         \
> > -   { echo "\#include <linux/kernel.h>" ;                                    \
> > -     echo "\#include <asm/asm-prototypes.h>" ;                              \
> > -     $(NM) $@ | sed -n 's/.* __export_symbol_\(.*\)/EXPORT_SYMBOL(\1);/p' ; } | \
> > -    $(CPP) -D__GENKSYMS__ $(c_flags) -xc - | $(genksyms)
> > +getasmexports =                                                                \
> > +   { echo "\#include <linux/kernel.h>" ;                               \
> > +     echo "\#include <linux/string.h>" ;                               \
> > +     echo "\#include <asm/asm-prototypes.h>" ;                         \
> > +     $(call getexportsymbols,$(2:.symtypes=.o),EXPORT_SYMBOL(\1);) ; }
> > +
> > +ifdef CONFIG_GENDWARFKSYMS
> > +cmd_gensymtypes_S =                                                    \
> > +       $(getasmexports) |                                              \
> > +       $(CC) $(c_flags) -c -o $(2:.symtypes=.gendwarfksyms.o) -xc -;   \
> > +       $(call getexportsymbols,$(2:.symtypes=.o),\1) |                 \
> > +       $(gendwarfksyms) $(2:.symtypes=.gendwarfksyms.o)
>
>
> I do not want to see crazy suffix replacements like this.

Yeah, I agree. It does get a bit ugly.

> I decided to delete this.
> https://lore.kernel.org/linux-kbuild/20241111171753.2917697-2-masahiroy@kernel.org/T/#u

Cool, thanks! I'll rebase v6 on top of your patch.

Sami
diff mbox series

Patch

diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
index f9e5f82fa88b..e6b2427e5c19 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -169,13 +169,36 @@  config MODVERSIONS
 	  make them incompatible with the kernel you are running.  If
 	  unsure, say N.
 
+choice
+	prompt "Module versioning implementation"
+	depends on MODVERSIONS
+	default GENKSYMS
+	help
+	  Select the tool used to calculate symbol versions for modules.
+
+	  If unsure, select GENKSYMS.
+
+config GENKSYMS
+	bool "genksyms (from source code)"
+	help
+	  Calculate symbol versions from pre-processed source code using
+	  genksyms.
+
+	  If unsure, say Y.
+
 config GENDWARFKSYMS
-	bool
+	bool "gendwarfksyms (from debugging information)"
 	depends on DEBUG_INFO
 	# Requires full debugging information, split DWARF not supported.
 	depends on !DEBUG_INFO_REDUCED && !DEBUG_INFO_SPLIT
 	# Requires ELF object files.
 	depends on !LTO
+	help
+	  Calculate symbol versions from DWARF debugging information using
+	  gendwarfksyms. Requires DEBUG_INFO to be enabled.
+
+	  If unsure, say N.
+endchoice
 
 config ASM_MODVERSIONS
 	bool
diff --git a/scripts/Makefile b/scripts/Makefile
index d7fec46d38c0..8533f4498885 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -53,7 +53,7 @@  hostprogs += unifdef
 targets += module.lds
 
 subdir-$(CONFIG_GCC_PLUGINS) += gcc-plugins
-subdir-$(CONFIG_MODVERSIONS) += genksyms
+subdir-$(CONFIG_GENKSYMS) += genksyms
 subdir-$(CONFIG_GENDWARFKSYMS) += gendwarfksyms
 subdir-$(CONFIG_SECURITY_SELINUX) += selinux
 subdir-$(CONFIG_SECURITY_IPE) += ipe
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 8f423a1faf50..d2a0440cdb79 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -107,18 +107,31 @@  cmd_cpp_i_c       = $(CPP) $(c_flags) -o $@ $<
 $(obj)/%.i: $(obj)/%.c FORCE
 	$(call if_changed_dep,cpp_i_c)
 
+getexportsymbols = $(NM) $(1) | sed -n 's/.* __export_symbol_\(.*\)/$(2)/p'
+
+gendwarfksyms = scripts/gendwarfksyms/gendwarfksyms	\
+	$(if $(1), --symtypes $(2))			\
+	$(if $(KBUILD_GENDWARFKSYMS_STABLE), --stable)
+
 genksyms = scripts/genksyms/genksyms		\
 	$(if $(1), -T $(2))			\
 	$(if $(KBUILD_PRESERVE), -p)		\
 	-r $(or $(wildcard $(2:.symtypes=.symref)), /dev/null)
 
 # These mirror gensymtypes_S and co below, keep them in synch.
+ifdef CONFIG_GENDWARFKSYMS
+symtypes_dep_c = $(obj)/%.o
+cmd_gensymtypes_c = $(if $(skip_gendwarfksyms),,	\
+	$(call getexportsymbols,$(2:.symtypes=.o),\1) |	\
+	$(gendwarfksyms) $(2:.symtypes=.o))
+else
 cmd_gensymtypes_c = $(CPP) -D__GENKSYMS__ $(c_flags) $< | $(genksyms)
+endif # CONFIG_GENDWARFKSYMS
 
 quiet_cmd_cc_symtypes_c = SYM $(quiet_modtag) $@
       cmd_cc_symtypes_c = $(call cmd_gensymtypes_c,true,$@) >/dev/null
 
-$(obj)/%.symtypes : $(obj)/%.c FORCE
+$(obj)/%.symtypes : $(obj)/%.c $(symtypes_dep_c) FORCE
 	$(call cmd,cc_symtypes_c)
 
 # LLVM assembly
@@ -314,19 +327,31 @@  $(obj)/%.ll: $(obj)/%.rs FORCE
 # This is convoluted. The .S file must first be preprocessed to run guards and
 # expand names, then the resulting exports must be constructed into plain
 # EXPORT_SYMBOL(symbol); to build our dummy C file, and that gets preprocessed
-# to make the genksyms input.
+# to make the genksyms input or compiled into an object for gendwarfksyms.
 #
 # These mirror gensymtypes_c and co above, keep them in synch.
-cmd_gensymtypes_S =                                                         \
-   { echo "\#include <linux/kernel.h>" ;                                    \
-     echo "\#include <asm/asm-prototypes.h>" ;                              \
-     $(NM) $@ | sed -n 's/.* __export_symbol_\(.*\)/EXPORT_SYMBOL(\1);/p' ; } | \
-    $(CPP) -D__GENKSYMS__ $(c_flags) -xc - | $(genksyms)
+getasmexports =								\
+   { echo "\#include <linux/kernel.h>" ;				\
+     echo "\#include <linux/string.h>" ;				\
+     echo "\#include <asm/asm-prototypes.h>" ;				\
+     $(call getexportsymbols,$(2:.symtypes=.o),EXPORT_SYMBOL(\1);) ; }
+
+ifdef CONFIG_GENDWARFKSYMS
+cmd_gensymtypes_S =							\
+	$(getasmexports) |						\
+	$(CC) $(c_flags) -c -o $(2:.symtypes=.gendwarfksyms.o) -xc -;	\
+	$(call getexportsymbols,$(2:.symtypes=.o),\1) |			\
+	$(gendwarfksyms) $(2:.symtypes=.gendwarfksyms.o)
+else
+cmd_gensymtypes_S =							\
+	$(getasmexports) |						\
+	$(CPP) -D__GENKSYMS__ $(c_flags) -xc - | $(genksyms)
+endif # CONFIG_GENDWARFKSYMS
 
 quiet_cmd_cc_symtypes_S = SYM $(quiet_modtag) $@
       cmd_cc_symtypes_S = $(call cmd_gensymtypes_S,true,$@) >/dev/null
 
-$(obj)/%.symtypes : $(obj)/%.S FORCE
+$(obj)/%.symtypes : $(obj)/%.S $(obj)/%.o FORCE
 	$(call cmd,cc_symtypes_S)