linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: Omar Sandoval <osandov@osandov.com>
Cc: Michal Marek <mmarek@suse.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	adobriyan@gmail.com, sfr@canb.auug.org.au,
	viro@zeniv.linux.org.uk, linux-kbuild@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [GIT PULL] kbuild changes for v4.9-rc1
Date: Mon, 17 Oct 2016 14:57:09 +1100	[thread overview]
Message-ID: <20161017145709.2882f33f@roar.ozlabs.ibm.com> (raw)
In-Reply-To: <20161016002205.GA9686@vader>

On Sat, 15 Oct 2016 17:22:05 -0700
Omar Sandoval <osandov@osandov.com> wrote:

> On Fri, Oct 14, 2016 at 10:12:46PM +0200, Michal Marek wrote:
> > Hi Linus,
> > 
> > please pull these kbuild changes for v4.9-rc1:
> > 
> > - EXPORT_SYMBOL for asm source by Al Viro. This does bring a regression,
> >   because genksyms no longer generates checksums for these symbols
> >   (CONFIG_MODVERSIONS). Nick Piggin is working on a patch to fix this.
> >   Plus, we are talking about functions like strcpy(), which rarely
> >   change prototypes.  
> 
> So this has broken all module loading for me. I get the following dmesg
> spew:
> 
> ...
> [    4.586914] scsi_mod: no symbol version for memset
> [    4.587920] scsi_mod: Unknown symbol memset (err -22)
> [    4.588443] scsi_mod: no symbol version for ___preempt_schedule
> [    4.589026] scsi_mod: Unknown symbol ___preempt_schedule (err -22)
> ...
> 
> Reverting 784d5699eddc ("x86: move exports to actual definitions") fixes
> it for me. This is with GCC 6.2.1, binutils 2.27, attached config.
> 

Thanks for the report. Could you try this patch and see if it helps?


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(-)

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)
-- 
2.9.3

  reply	other threads:[~2016-10-17  3:57 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-14 20:12 [GIT PULL] kbuild changes for v4.9-rc1 Michal Marek
2016-10-16  0:22 ` Omar Sandoval
2016-10-17  3:57   ` Nicholas Piggin [this message]
2016-10-17  6:57     ` Mathieu OTHACEHE
  -- strict thread matches above, loose matches on Subject: below --
2016-10-17  6:51 Adam Borowski
2016-10-17  6:59 ` Nicholas Piggin
2016-10-17 10:01   ` Adam Borowski
2016-10-17 11:12     ` Alexey Dobriyan
2016-10-17 11:17       ` Geert Uytterhoeven
2016-10-17 11:32         ` Alexey Dobriyan
2016-10-17 12:22     ` Mathieu OTHACEHE
2016-10-18  0:16       ` Adam Borowski
2016-10-18  1:34         ` Nicholas Piggin
2016-10-19 14:38           ` Michal Marek
2016-10-20  3:52             ` Nicholas Piggin
2016-10-27  8:10               ` Kalle Valo
2016-10-27 11:15                 ` Nicholas Piggin
2016-10-27 13:14                   ` Kalle Valo
2016-10-27 13:25                     ` Nicholas Piggin
2016-10-30 10:51                 ` Thorsten Leemhuis
2016-11-01 15:48           ` Michal Marek
2016-11-02 12:11             ` Adam Borowski
2016-12-16 19:55     ` Jiri Slaby
2016-12-16 19:57       ` Linus Torvalds
2016-12-17  8:57         ` Jiri Slaby
2016-12-17  9:33           ` Adam Borowski
2016-12-17 23:59           ` Linus Torvalds
2016-12-18 10:49             ` Jiri Slaby
2016-12-18 11:03               ` Arend Van Spriel
2016-12-18 13:27                 ` Nikolay Borisov
2016-12-18 14:45                   ` Jiri Slaby
2016-12-18 14:54                     ` Nikolay Borisov
2016-12-18 15:08                       ` Jiri Slaby

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161017145709.2882f33f@roar.ozlabs.ibm.com \
    --to=npiggin@gmail.com \
    --cc=adobriyan@gmail.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mmarek@suse.com \
    --cc=osandov@osandov.com \
    --cc=sfr@canb.auug.org.au \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).