All of lore.kernel.org
 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: 35+ 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 10:01     ` Adam Borowski
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 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.