| Message ID | 20161015124352.10795-3-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Nick, sorry for the late feedback. Dne 15.10.2016 v 14:43 Nicholas Piggin napsal(a): > +# .S file exports must have their C prototypes defined in asm/asm-prototypes.h > +# or a file that it includes, in order to get versioned symbols. We build a > +# dummy C file that includes asm-prototypes and the EXPORT_SYMBOL lines from > +# the .S file (with trailing ';'), and run genksyms on that, to extract vers. > +# > +# 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>" ; \ > + grep EXPORT_SYMBOL $< | sed 's/$$/;/' ) | \ > + $(CPP) -D__GENKSYMS__ $(c_flags) -xc - | \ > + $(GENKSYMS) $(if $(1), -T $(2)) \ > + $(patsubst y,-s _,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX)) \ > + $(if $(KBUILD_PRESERVE),-p) \ > + -r $(firstword $(wildcard $(2:.symtypes=.symref) /dev/null)) I think it would be cleaner to add the #include to the .S files themselves and grep for both EXPORT_SYMBOL and #include here. The reason is that some files might need additional #includes to allow genksyms to properly expand some function prototypes. Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday, October 19, 2016 4:50:26 PM CEST Michal Marek wrote: > Dne 15.10.2016 v 14:43 Nicholas Piggin napsal(a): > > +# .S file exports must have their C prototypes defined in asm/asm-prototypes.h > > +# or a file that it includes, in order to get versioned symbols. We build a > > +# dummy C file that includes asm-prototypes and the EXPORT_SYMBOL lines from > > +# the .S file (with trailing ';'), and run genksyms on that, to extract vers. > > +# > > +# 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>" ; \ > > + grep EXPORT_SYMBOL $< | sed 's/$$/;/' ) | \ > > + $(CPP) -D__GENKSYMS__ $(c_flags) -xc - | \ > > + $(GENKSYMS) $(if $(1), -T $(2)) \ > > + $(patsubst y,-s _,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX)) \ > > + $(if $(KBUILD_PRESERVE),-p) \ > > + -r $(firstword $(wildcard $(2:.symtypes=.symref) /dev/null)) > > I think it would be cleaner to add the #include to the .S files > themselves and grep for both EXPORT_SYMBOL and #include here. The reason > is that some files might need additional #includes to allow genksyms to > properly expand some function prototypes. > This is something I tried earlier, and it wasn't pretty: Some of the assembler files rely on -D__ASSEMBLER__ to be set in order to read the right headers, but setting that macro means that all of the declarations get skipped. I ended up testing for -D__GENKSYMS__ in each .S file, which was also rather ugly. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 19 Oct 2016 16:59:42 +0200 Arnd Bergmann <arnd@arndb.de> wrote: > On Wednesday, October 19, 2016 4:50:26 PM CEST Michal Marek wrote: > > Dne 15.10.2016 v 14:43 Nicholas Piggin napsal(a): > > > +# .S file exports must have their C prototypes defined in asm/asm-prototypes.h > > > +# or a file that it includes, in order to get versioned symbols. We build a > > > +# dummy C file that includes asm-prototypes and the EXPORT_SYMBOL lines from > > > +# the .S file (with trailing ';'), and run genksyms on that, to extract vers. > > > +# > > > +# 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>" ; \ > > > + grep EXPORT_SYMBOL $< | sed 's/$$/;/' ) | \ > > > + $(CPP) -D__GENKSYMS__ $(c_flags) -xc - | \ > > > + $(GENKSYMS) $(if $(1), -T $(2)) \ > > > + $(patsubst y,-s _,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX)) \ > > > + $(if $(KBUILD_PRESERVE),-p) \ > > > + -r $(firstword $(wildcard $(2:.symtypes=.symref) /dev/null)) > > > > I think it would be cleaner to add the #include to the .S files > > themselves and grep for both EXPORT_SYMBOL and #include here. The reason > > is that some files might need additional #includes to allow genksyms to > > properly expand some function prototypes. > > > > This is something I tried earlier, and it wasn't pretty: Some of the assembler > files rely on -D__ASSEMBLER__ to be set in order to read the right headers, > but setting that macro means that all of the declarations get skipped. > > I ended up testing for -D__GENKSYMS__ in each .S file, which was also > rather ugly. Yeah, I had the same idea as you and Michal too. It's conceptually nicer, but in practice it turned into a mess. If some architectures wanted to start protecting their .h files and including them into .S for the prototypes, we could start parsing those too. Should we do the quick and dirty way for 4.9? Thanks, Nick -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday, October 20, 2016 2:58:27 PM CEST Nicholas Piggin wrote: > > Yeah, I had the same idea as you and Michal too. It's conceptually nicer, > but in practice it turned into a mess. If some architectures wanted to start > protecting their .h files and including them into .S for the prototypes, we > could start parsing those too. Should we do the quick and dirty way for 4.9? Let's stay with your approach for now. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne 20.10.2016 v 10:03 Arnd Bergmann napsal(a): > On Thursday, October 20, 2016 2:58:27 PM CEST Nicholas Piggin wrote: >> >> Yeah, I had the same idea as you and Michal too. It's conceptually nicer, >> but in practice it turned into a mess. If some architectures wanted to start >> protecting their .h files and including them into .S for the prototypes, we >> could start parsing those too. Should we do the quick and dirty way for 4.9? > > Let's stay with your approach for now. Agreed. Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/scripts/Makefile.build b/scripts/Makefile.build index de46ab0..edcf876 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -159,7 +159,8 @@ cmd_cpp_i_c = $(CPP) $(c_flags) -o $@ $< $(obj)/%.i: $(src)/%.c FORCE $(call if_changed_dep,cpp_i_c) -cmd_gensymtypes = \ +# These mirror gensymtypes_S and co below, keep them in synch. +cmd_gensymtypes_c = \ $(CPP) -D__GENKSYMS__ $(c_flags) $< | \ $(GENKSYMS) $(if $(1), -T $(2)) \ $(patsubst y,-s _,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX)) \ @@ -169,7 +170,7 @@ cmd_gensymtypes = \ quiet_cmd_cc_symtypes_c = SYM $(quiet_modtag) $@ cmd_cc_symtypes_c = \ set -e; \ - $(call cmd_gensymtypes,true,$@) >/dev/null; \ + $(call cmd_gensymtypes_c,true,$@) >/dev/null; \ test -s $@ || rm -f $@ $(obj)/%.symtypes : $(src)/%.c FORCE @@ -198,9 +199,10 @@ else # the actual value of the checksum generated by genksyms cmd_cc_o_c = $(CC) $(c_flags) -c -o $(@D)/.tmp_$(@F) $< -cmd_modversions = \ + +cmd_modversions_c = \ if $(OBJDUMP) -h $(@D)/.tmp_$(@F) | grep -q __ksymtab; then \ - $(call cmd_gensymtypes,$(KBUILD_SYMTYPES),$(@:.o=.symtypes)) \ + $(call cmd_gensymtypes_c,$(KBUILD_SYMTYPES),$(@:.o=.symtypes)) \ > $(@D)/.tmp_$(@F:.o=.ver); \ \ $(LD) $(LDFLAGS) -r -o $@ $(@D)/.tmp_$(@F) \ @@ -268,13 +270,14 @@ endif # CONFIG_STACK_VALIDATION define rule_cc_o_c $(call echo-cmd,checksrc) $(cmd_checksrc) \ $(call cmd_and_fixdep,cc_o_c) \ - $(cmd_modversions) \ + $(cmd_modversions_c) \ $(cmd_objtool) \ $(call echo-cmd,record_mcount) $(cmd_record_mcount) endef define rule_as_o_S $(call cmd_and_fixdep,as_o_S) \ + $(cmd_modversions_S) \ $(cmd_objtool) endef @@ -314,6 +317,32 @@ modkern_aflags := $(KBUILD_AFLAGS_KERNEL) $(AFLAGS_KERNEL) $(real-objs-m) : modkern_aflags := $(KBUILD_AFLAGS_MODULE) $(AFLAGS_MODULE) $(real-objs-m:.o=.s): modkern_aflags := $(KBUILD_AFLAGS_MODULE) $(AFLAGS_MODULE) +# .S file exports must have their C prototypes defined in asm/asm-prototypes.h +# or a file that it includes, in order to get versioned symbols. We build a +# dummy C file that includes asm-prototypes and the EXPORT_SYMBOL lines from +# the .S file (with trailing ';'), and run genksyms on that, to extract vers. +# +# 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>" ; \ + grep EXPORT_SYMBOL $< | sed 's/$$/;/' ) | \ + $(CPP) -D__GENKSYMS__ $(c_flags) -xc - | \ + $(GENKSYMS) $(if $(1), -T $(2)) \ + $(patsubst y,-s _,$(CONFIG_HAVE_UNDERSCORE_SYMBOL_PREFIX)) \ + $(if $(KBUILD_PRESERVE),-p) \ + -r $(firstword $(wildcard $(2:.symtypes=.symref) /dev/null)) + +quiet_cmd_cc_symtypes_S = SYM $(quiet_modtag) $@ +cmd_cc_symtypes_S = \ + set -e; \ + $(call cmd_gensymtypes_S,true,$@) >/dev/null; \ + test -s $@ || rm -f $@ + +$(obj)/%.symtypes : $(src)/%.S FORCE + $(call cmd,cc_symtypes_S) + + quiet_cmd_cpp_s_S = CPP $(quiet_modtag) $@ cmd_cpp_s_S = $(CPP) $(a_flags) -o $@ $< @@ -321,7 +350,37 @@ $(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 $@ $< + +ifndef CONFIG_MODVERSIONS +cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $< + +else + +ASM_PROTOTYPES := $(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/asm-prototypes.h) + +ifeq ($(ASM_PROTOTYPES),) +cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $< + +else + +# versioning matches the C process described above, with difference that +# we parse asm-prototypes.h C header to get function definitions. + +cmd_as_o_S = $(CC) $(a_flags) -c -o $(@D)/.tmp_$(@F) $< + +cmd_modversions_S = \ + if $(OBJDUMP) -h $(@D)/.tmp_$(@F) | grep -q __ksymtab; then \ + $(call cmd_gensymtypes_S,$(KBUILD_SYMTYPES),$(@:.o=.symtypes)) \ + > $(@D)/.tmp_$(@F:.o=.ver); \ + \ + $(LD) $(LDFLAGS) -r -o $@ $(@D)/.tmp_$(@F) \ + -T $(@D)/.tmp_$(@F:.o=.ver); \ + rm -f $(@D)/.tmp_$(@F) $(@D)/.tmp_$(@F:.o=.ver); \ + else \ + mv -f $(@D)/.tmp_$(@F) $@; \ + fi; +endif +endif $(obj)/%.o: $(src)/%.S $(objtool_obj) FORCE $(call if_changed_rule,as_o_S)
Allow architectures to create asm/asm-prototypes.h file that provides C prototypes for exported asm functions, which enables proper CRC versions to be generated for them. It's done by creating a trivial C program that includes that file and the EXPORT_SYMBOL()s from the .S file, and preprocesses that then sends the result to genksyms. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- scripts/Makefile.build | 71 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 65 insertions(+), 6 deletions(-)