linux-kbuild.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS
@ 2023-05-13 20:44 Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 01/21] modpost: remove broken calculation of exception_table_entry size Masahiro Yamada
                   ` (21 more replies)
  0 siblings, 22 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada


This patch set refactors modpost first to make it easier to
add new code.

Main goals:

 - Refactors EXPORT_SYMBOL, <linux/export.h> and <asm/export.h>.
   You can still put EXPORT_SYMBOL() in *.S file, very close to the definition,
   but you do not need to care about whether it is a function or a data.
   This removes EXPORT_DATA_SYMBOL().

 - Re-implement TRIM_UNUSED_KSYMS in one-pass.
   This makes the building faster.

 - Move the static EXPORT_SYMBOL check to modpost.
   This also makes the building faster.

Previous version
v3: https://lore.kernel.org/all/20220928063947.299333-1-masahiroy@kernel.org/



Masahiro Yamada (21):
  modpost: remove broken calculation of exception_table_entry size
  modpost: remove fromsym info in __ex_table section mismatch warning
  modpost: remove get_prettyname()
  modpost: squash report_extable_warnings() into
    extable_mismatch_handler()
  modpost: squash report_sec_mismatch() into default_mismatch_handler()
  modpost: clean up is_executable_section()
  modpost: squash extable_mismatch_handler() into
    default_mismatch_handler()
  modpost: pass 'tosec' down to default_mismatch_handler()
  modpost: pass section index to find_elf_symbol2()
  modpost: simplify find_elf_symbol()
  modpost: rename find_elf_symbol() and find_elf_symbol2()
  modpost: unify 'sym' and 'to' in default_mismatch_handler()
  modpost: replace r->r_offset, r->r_addend with faddr, taddr
  modpost: remove is_shndx_special() check from section_rel(a)
  modpost: pass struct module pointer to check_section_mismatch()
  kbuild: generate KSYMTAB entries by modpost
  ia64,export.h: replace EXPORT_DATA_SYMBOL* with EXPORT_SYMBOL*
  modpost: check static EXPORT_SYMBOL* by modpost again
  modpost: squash sym_update_namespace() into sym_add_exported()
  modpost: use null string instead of NULL pointer for default namespace
  kbuild: implement CONFIG_TRIM_UNUSED_KSYMS without recursion

 .gitignore                        |   1 -
 Makefile                          |  19 +-
 arch/ia64/include/asm/Kbuild      |   1 +
 arch/ia64/include/asm/export.h    |   3 -
 arch/ia64/kernel/head.S           |   2 +-
 arch/ia64/kernel/ivt.S            |   2 +-
 include/asm-generic/export.h      |  83 +----
 include/asm-generic/vmlinux.lds.h |   1 +
 include/linux/export-internal.h   |  49 +++
 include/linux/export.h            | 116 ++-----
 include/linux/pm.h                |   8 +-
 kernel/module/internal.h          |  12 +
 scripts/Makefile.build            |  19 +-
 scripts/Makefile.modpost          |   7 +
 scripts/adjust_autoksyms.sh       |  73 ----
 scripts/basic/fixdep.c            |   3 +-
 scripts/check-local-export        |  70 ----
 scripts/gen_ksymdeps.sh           |  30 --
 scripts/mod/modpost.c             | 534 ++++++++++++------------------
 scripts/mod/modpost.h             |   1 +
 scripts/remove-stale-files        |   2 +
 21 files changed, 317 insertions(+), 719 deletions(-)
 delete mode 100644 arch/ia64/include/asm/export.h
 delete mode 100755 scripts/adjust_autoksyms.sh
 delete mode 100755 scripts/check-local-export
 delete mode 100755 scripts/gen_ksymdeps.sh

-- 
2.39.2


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v4 01/21] modpost: remove broken calculation of exception_table_entry size
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 02/21] modpost: remove fromsym info in __ex_table section mismatch warning Masahiro Yamada
                   ` (20 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

find_extable_entry_size() is completely broken. It has awesome comments
about how to calculate sizeof(struct exception_table_entry).

It was based on these assumptions:

  - struct exception_table_entry has two fields
  - both of the fields have the same size

Then, we came up with this equation:

  (offset of the second field) * 2 == (size of struct)

It was true for all architectures when commit 52dc0595d540 ("modpost:
handle relocations mismatch in __ex_table.") was applied.

Our mathematics broke when commit 548acf19234d ("x86/mm: Expand the
exception table logic to allow new handling options") introduced the
third field.

Now, the definition of exception_table_entry is highly arch-dependent.

For x86, sizeof(struct exception_table_entry) is apparently 12, but
find_extable_entry_size() sets extable_entry_size to 8.

I could fix it, but I do not see much value in this code.

extable_entry_size is used just for selecting a slightly different
error message.

If the first field ("insn") references to a non-executable section,

    The relocation at %s+0x%lx references
    section "%s" which is not executable, IOW
    it is not possible for the kernel to fault
    at that address.  Something is seriously wrong
    and should be fixed.

If the second field ("fixup") references to a non-executable section,

    The relocation at %s+0x%lx references
    section "%s" which is not executable, IOW
    the kernel will fault if it ever tries to
    jump to it.  Something is seriously wrong
    and should be fixed.

Merge the two error messages rather than adding even more complexity.

Change fatal() to error() to make it continue running and catch more
possible errors.

Fixes: 548acf19234d ("x86/mm: Expand the exception table logic to allow new handling options")
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 60 +++----------------------------------------
 1 file changed, 3 insertions(+), 57 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index c1c523adb139..ba4577aa4f1d 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1292,43 +1292,6 @@ static int is_executable_section(struct elf_info* elf, unsigned int section_inde
 	return ((elf->sechdrs[section_index].sh_flags & SHF_EXECINSTR) == SHF_EXECINSTR);
 }
 
-/*
- * We rely on a gross hack in section_rel[a]() calling find_extable_entry_size()
- * to know the sizeof(struct exception_table_entry) for the target architecture.
- */
-static unsigned int extable_entry_size = 0;
-static void find_extable_entry_size(const char* const sec, const Elf_Rela* r)
-{
-	/*
-	 * If we're currently checking the second relocation within __ex_table,
-	 * that relocation offset tells us the offsetof(struct
-	 * exception_table_entry, fixup) which is equal to sizeof(struct
-	 * exception_table_entry) divided by two.  We use that to our advantage
-	 * since there's no portable way to get that size as every architecture
-	 * seems to go with different sized types.  Not pretty but better than
-	 * hard-coding the size for every architecture..
-	 */
-	if (!extable_entry_size)
-		extable_entry_size = r->r_offset * 2;
-}
-
-static inline bool is_extable_fault_address(Elf_Rela *r)
-{
-	/*
-	 * extable_entry_size is only discovered after we've handled the
-	 * _second_ relocation in __ex_table, so only abort when we're not
-	 * handling the first reloc and extable_entry_size is zero.
-	 */
-	if (r->r_offset && extable_entry_size == 0)
-		fatal("extable_entry size hasn't been discovered!\n");
-
-	return ((r->r_offset == 0) ||
-		(r->r_offset % extable_entry_size == 0));
-}
-
-#define is_second_extable_reloc(Start, Cur, Sec)			\
-	(((Cur) == (Start) + 1) && (strcmp("__ex_table", (Sec)) == 0))
-
 static void report_extable_warnings(const char* modname, struct elf_info* elf,
 				    const struct sectioncheck* const mismatch,
 				    Elf_Rela* r, Elf_Sym* sym,
@@ -1384,22 +1347,9 @@ static void extable_mismatch_handler(const char* modname, struct elf_info *elf,
 		      "You might get more information about where this is\n"
 		      "coming from by using scripts/check_extable.sh %s\n",
 		      fromsec, (long)r->r_offset, tosec, modname);
-	else if (!is_executable_section(elf, get_secindex(elf, sym))) {
-		if (is_extable_fault_address(r))
-			fatal("The relocation at %s+0x%lx references\n"
-			      "section \"%s\" which is not executable, IOW\n"
-			      "it is not possible for the kernel to fault\n"
-			      "at that address.  Something is seriously wrong\n"
-			      "and should be fixed.\n",
-			      fromsec, (long)r->r_offset, tosec);
-		else
-			fatal("The relocation at %s+0x%lx references\n"
-			      "section \"%s\" which is not executable, IOW\n"
-			      "the kernel will fault if it ever tries to\n"
-			      "jump to it.  Something is seriously wrong\n"
-			      "and should be fixed.\n",
-			      fromsec, (long)r->r_offset, tosec);
-	}
+	else if (!is_executable_section(elf, get_secindex(elf, sym)))
+		error("%s+0x%lx references non-executable section '%s'\n",
+		      fromsec, (long)r->r_offset, tosec);
 }
 
 static void check_section_mismatch(const char *modname, struct elf_info *elf,
@@ -1574,8 +1524,6 @@ static void section_rela(const char *modname, struct elf_info *elf,
 		/* Skip special sections */
 		if (is_shndx_special(sym->st_shndx))
 			continue;
-		if (is_second_extable_reloc(start, rela, fromsec))
-			find_extable_entry_size(fromsec, &r);
 		check_section_mismatch(modname, elf, &r, sym, fromsec);
 	}
 }
@@ -1635,8 +1583,6 @@ static void section_rel(const char *modname, struct elf_info *elf,
 		/* Skip special sections */
 		if (is_shndx_special(sym->st_shndx))
 			continue;
-		if (is_second_extable_reloc(start, rel, fromsec))
-			find_extable_entry_size(fromsec, &r);
 		check_section_mismatch(modname, elf, &r, sym, fromsec);
 	}
 }
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 02/21] modpost: remove fromsym info in __ex_table section mismatch warning
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 01/21] modpost: remove broken calculation of exception_table_entry size Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 03/21] modpost: remove get_prettyname() Masahiro Yamada
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

report_extable_warnings() prints "from" in a pretty form, but we know
it is always located in the __ex_table section, i.e. a collection of
struct exception_table_entry.

It is very likely to fail to get the symbol name and ends up with
meaningless message:

  ... in reference from the (unknown reference) (unknown) to ...

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index ba4577aa4f1d..bbe066f7adbc 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1297,23 +1297,16 @@ static void report_extable_warnings(const char* modname, struct elf_info* elf,
 				    Elf_Rela* r, Elf_Sym* sym,
 				    const char* fromsec, const char* tosec)
 {
-	Elf_Sym* fromsym = find_elf_symbol2(elf, r->r_offset, fromsec);
-	const char* fromsym_name = sym_name(elf, fromsym);
 	Elf_Sym* tosym = find_elf_symbol(elf, r->r_addend, sym);
 	const char* tosym_name = sym_name(elf, tosym);
-	const char* from_pretty_name;
-	const char* from_pretty_name_p;
 	const char* to_pretty_name;
 	const char* to_pretty_name_p;
 
-	get_pretty_name(is_function(fromsym),
-			&from_pretty_name, &from_pretty_name_p);
 	get_pretty_name(is_function(tosym),
 			&to_pretty_name, &to_pretty_name_p);
 
-	warn("%s(%s+0x%lx): Section mismatch in reference from the %s %s%s to the %s %s:%s%s\n",
-	     modname, fromsec, (long)r->r_offset, from_pretty_name,
-	     fromsym_name, from_pretty_name_p,
+	warn("%s(%s+0x%lx): Section mismatch in reference to the %s %s:%s%s\n",
+	     modname, fromsec, (long)r->r_offset,
 	     to_pretty_name, tosec, tosym_name, to_pretty_name_p);
 
 	if (!match(tosec, mismatch->bad_tosec) &&
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 03/21] modpost: remove get_prettyname()
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 01/21] modpost: remove broken calculation of exception_table_entry size Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 02/21] modpost: remove fromsym info in __ex_table section mismatch warning Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 04/21] modpost: squash report_extable_warnings() into extable_mismatch_handler() Masahiro Yamada
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

This is the last user of get_pretty_name() - it is just used to
distinguish whether the symbol is a function or not. It is not
valuable information.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index bbe066f7adbc..371891d67175 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1207,23 +1207,6 @@ static Elf_Sym *find_elf_symbol2(struct elf_info *elf, Elf_Addr addr,
 	return near;
 }
 
-static int is_function(Elf_Sym *sym)
-{
-	if (sym)
-		return ELF_ST_TYPE(sym->st_info) == STT_FUNC;
-	else
-		return -1;
-}
-
-static inline void get_pretty_name(int is_func, const char** name, const char** name_p)
-{
-	switch (is_func) {
-	case 0:	*name = "variable"; *name_p = ""; break;
-	case 1:	*name = "function"; *name_p = "()"; break;
-	default: *name = "(unknown reference)"; *name_p = ""; break;
-	}
-}
-
 /*
  * Print a warning about a section mismatch.
  * Try to find symbols near it so user can find it.
@@ -1299,15 +1282,9 @@ static void report_extable_warnings(const char* modname, struct elf_info* elf,
 {
 	Elf_Sym* tosym = find_elf_symbol(elf, r->r_addend, sym);
 	const char* tosym_name = sym_name(elf, tosym);
-	const char* to_pretty_name;
-	const char* to_pretty_name_p;
 
-	get_pretty_name(is_function(tosym),
-			&to_pretty_name, &to_pretty_name_p);
-
-	warn("%s(%s+0x%lx): Section mismatch in reference to the %s %s:%s%s\n",
-	     modname, fromsec, (long)r->r_offset,
-	     to_pretty_name, tosec, tosym_name, to_pretty_name_p);
+	warn("%s(%s+0x%lx): Section mismatch in reference to the %s:%s\n",
+	     modname, fromsec, (long)r->r_offset, tosec, tosym_name);
 
 	if (!match(tosec, mismatch->bad_tosec) &&
 	    is_executable_section(elf, get_secindex(elf, sym)))
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 04/21] modpost: squash report_extable_warnings() into extable_mismatch_handler()
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (2 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 03/21] modpost: remove get_prettyname() Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 05/21] modpost: squash report_sec_mismatch() into default_mismatch_handler() Masahiro Yamada
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

Collect relevant code into one place to clarify all the cases are
covered by 'if () ... else if ... else ...'.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 40 ++++++++++++++--------------------------
 1 file changed, 14 insertions(+), 26 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 371891d67175..7a9a3ef8ca0d 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1275,40 +1275,19 @@ static int is_executable_section(struct elf_info* elf, unsigned int section_inde
 	return ((elf->sechdrs[section_index].sh_flags & SHF_EXECINSTR) == SHF_EXECINSTR);
 }
 
-static void report_extable_warnings(const char* modname, struct elf_info* elf,
-				    const struct sectioncheck* const mismatch,
-				    Elf_Rela* r, Elf_Sym* sym,
-				    const char* fromsec, const char* tosec)
-{
-	Elf_Sym* tosym = find_elf_symbol(elf, r->r_addend, sym);
-	const char* tosym_name = sym_name(elf, tosym);
-
-	warn("%s(%s+0x%lx): Section mismatch in reference to the %s:%s\n",
-	     modname, fromsec, (long)r->r_offset, tosec, tosym_name);
-
-	if (!match(tosec, mismatch->bad_tosec) &&
-	    is_executable_section(elf, get_secindex(elf, sym)))
-		fprintf(stderr,
-			"The relocation at %s+0x%lx references\n"
-			"section \"%s\" which is not in the list of\n"
-			"authorized sections.  If you're adding a new section\n"
-			"and/or if this reference is valid, add \"%s\" to the\n"
-			"list of authorized sections to jump to on fault.\n"
-			"This can be achieved by adding \"%s\" to \n"
-			"OTHER_TEXT_SECTIONS in scripts/mod/modpost.c.\n",
-			fromsec, (long)r->r_offset, tosec, tosec, tosec);
-}
-
 static void extable_mismatch_handler(const char* modname, struct elf_info *elf,
 				     const struct sectioncheck* const mismatch,
 				     Elf_Rela* r, Elf_Sym* sym,
 				     const char *fromsec)
 {
 	const char* tosec = sec_name(elf, get_secindex(elf, sym));
+	Elf_Sym *tosym = find_elf_symbol(elf, r->r_addend, sym);
+	const char *tosym_name = sym_name(elf, tosym);
 
 	sec_mismatch_count++;
 
-	report_extable_warnings(modname, elf, mismatch, r, sym, fromsec, tosec);
+	warn("%s(%s+0x%lx): Section mismatch in reference to the %s:%s\n",
+	     modname, fromsec, (long)r->r_offset, tosec, tosym_name);
 
 	if (match(tosec, mismatch->bad_tosec))
 		fatal("The relocation at %s+0x%lx references\n"
@@ -1317,7 +1296,16 @@ static void extable_mismatch_handler(const char* modname, struct elf_info *elf,
 		      "You might get more information about where this is\n"
 		      "coming from by using scripts/check_extable.sh %s\n",
 		      fromsec, (long)r->r_offset, tosec, modname);
-	else if (!is_executable_section(elf, get_secindex(elf, sym)))
+	else if (is_executable_section(elf, get_secindex(elf, sym)))
+		warn("The relocation at %s+0x%lx references\n"
+		     "section \"%s\" which is not in the list of\n"
+		     "authorized sections.  If you're adding a new section\n"
+		     "and/or if this reference is valid, add \"%s\" to the\n"
+		     "list of authorized sections to jump to on fault.\n"
+		     "This can be achieved by adding \"%s\" to\n"
+		     "OTHER_TEXT_SECTIONS in scripts/mod/modpost.c.\n",
+		     fromsec, (long)r->r_offset, tosec, tosec, tosec);
+	else
 		error("%s+0x%lx references non-executable section '%s'\n",
 		      fromsec, (long)r->r_offset, tosec);
 }
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 05/21] modpost: squash report_sec_mismatch() into default_mismatch_handler()
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (3 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 04/21] modpost: squash report_extable_warnings() into extable_mismatch_handler() Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 06/21] modpost: clean up is_executable_section() Masahiro Yamada
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

report_sec_mismatch() and default_mismatch_handler() are small enough
to be merged together.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 55 ++++++++++++++++---------------------------
 1 file changed, 20 insertions(+), 35 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 7a9a3ef8ca0d..bb7d1d87bae7 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1207,17 +1207,27 @@ static Elf_Sym *find_elf_symbol2(struct elf_info *elf, Elf_Addr addr,
 	return near;
 }
 
-/*
- * Print a warning about a section mismatch.
- * Try to find symbols near it so user can find it.
- * Check whitelist before warning - it may be a false positive.
- */
-static void report_sec_mismatch(const char *modname,
-				const struct sectioncheck *mismatch,
-				const char *fromsec,
-				const char *fromsym,
-				const char *tosec, const char *tosym)
+static void default_mismatch_handler(const char *modname, struct elf_info *elf,
+				     const struct sectioncheck* const mismatch,
+				     Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
 {
+	const char *tosec;
+	Elf_Sym *to;
+	Elf_Sym *from;
+	const char *tosym;
+	const char *fromsym;
+
+	from = find_elf_symbol2(elf, r->r_offset, fromsec);
+	fromsym = sym_name(elf, from);
+
+	tosec = sec_name(elf, get_secindex(elf, sym));
+	to = find_elf_symbol(elf, r->r_addend, sym);
+	tosym = sym_name(elf, to);
+
+	/* check whitelist - we may ignore it */
+	if (!secref_whitelist(mismatch, fromsec, fromsym, tosec, tosym))
+		return;
+
 	sec_mismatch_count++;
 
 	switch (mismatch->mismatch) {
@@ -1242,31 +1252,6 @@ static void report_sec_mismatch(const char *modname,
 	}
 }
 
-static void default_mismatch_handler(const char *modname, struct elf_info *elf,
-				     const struct sectioncheck* const mismatch,
-				     Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
-{
-	const char *tosec;
-	Elf_Sym *to;
-	Elf_Sym *from;
-	const char *tosym;
-	const char *fromsym;
-
-	from = find_elf_symbol2(elf, r->r_offset, fromsec);
-	fromsym = sym_name(elf, from);
-
-	tosec = sec_name(elf, get_secindex(elf, sym));
-	to = find_elf_symbol(elf, r->r_addend, sym);
-	tosym = sym_name(elf, to);
-
-	/* check whitelist - we may ignore it */
-	if (secref_whitelist(mismatch,
-			     fromsec, fromsym, tosec, tosym)) {
-		report_sec_mismatch(modname, mismatch,
-				    fromsec, fromsym, tosec, tosym);
-	}
-}
-
 static int is_executable_section(struct elf_info* elf, unsigned int section_index)
 {
 	if (section_index > elf->num_sections)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 06/21] modpost: clean up is_executable_section()
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (4 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 05/21] modpost: squash report_sec_mismatch() into default_mismatch_handler() Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 07/21] modpost: squash extable_mismatch_handler() into default_mismatch_handler() Masahiro Yamada
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

SHF_EXECINSTR is a bit flag (#define SHF_EXECINSTR 0x4).
Compare the masked flag to '!= 0'.

There is no good reason to stop modpost immediately even if a special
section index is given. You will get a section mismatch error anyway.

Also, change the return type to bool.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index bb7d1d87bae7..0bda2f22c985 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1207,6 +1207,14 @@ static Elf_Sym *find_elf_symbol2(struct elf_info *elf, Elf_Addr addr,
 	return near;
 }
 
+static bool is_executable_section(struct elf_info *elf, unsigned int secndx)
+{
+	if (secndx > elf->num_sections)
+		return false;
+
+	return (elf->sechdrs[secndx].sh_flags & SHF_EXECINSTR) != 0;
+}
+
 static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 				     const struct sectioncheck* const mismatch,
 				     Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
@@ -1252,14 +1260,6 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 	}
 }
 
-static int is_executable_section(struct elf_info* elf, unsigned int section_index)
-{
-	if (section_index > elf->num_sections)
-		fatal("section_index is outside elf->num_sections!\n");
-
-	return ((elf->sechdrs[section_index].sh_flags & SHF_EXECINSTR) == SHF_EXECINSTR);
-}
-
 static void extable_mismatch_handler(const char* modname, struct elf_info *elf,
 				     const struct sectioncheck* const mismatch,
 				     Elf_Rela* r, Elf_Sym* sym,
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 07/21] modpost: squash extable_mismatch_handler() into default_mismatch_handler()
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (5 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 06/21] modpost: clean up is_executable_section() Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 08/21] modpost: pass 'tosec' down to default_mismatch_handler() Masahiro Yamada
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

Merging these two reduces several lines of code. The extable section
mismatch is already distinguished by EXTABLE_TO_NON_TEXT.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 84 ++++++++++++++-----------------------------
 1 file changed, 26 insertions(+), 58 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 0bda2f22c985..49357a716519 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -881,27 +881,14 @@ enum mismatch {
  * targeting sections in this array (white-list).  Can be empty.
  *
  * @mismatch: Type of mismatch.
- *
- * @handler: Specific handler to call when a match is found.  If NULL,
- * default_mismatch_handler() will be called.
- *
  */
 struct sectioncheck {
 	const char *fromsec[20];
 	const char *bad_tosec[20];
 	const char *good_tosec[20];
 	enum mismatch mismatch;
-	void (*handler)(const char *modname, struct elf_info *elf,
-			const struct sectioncheck* const mismatch,
-			Elf_Rela *r, Elf_Sym *sym, const char *fromsec);
-
 };
 
-static void extable_mismatch_handler(const char *modname, struct elf_info *elf,
-				     const struct sectioncheck* const mismatch,
-				     Elf_Rela *r, Elf_Sym *sym,
-				     const char *fromsec);
-
 static const struct sectioncheck sectioncheck[] = {
 /* Do not reference init/exit code/data from
  * normal code and data
@@ -974,7 +961,6 @@ static const struct sectioncheck sectioncheck[] = {
 	.bad_tosec = { ".altinstr_replacement", NULL },
 	.good_tosec = {ALL_TEXT_SECTIONS , NULL},
 	.mismatch = EXTABLE_TO_NON_TEXT,
-	.handler = extable_mismatch_handler,
 }
 };
 
@@ -1255,60 +1241,42 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 		     modname, tosym, tosec);
 		break;
 	case EXTABLE_TO_NON_TEXT:
-		fatal("There's a special handler for this mismatch type, we should never get here.\n");
+		warn("%s(%s+0x%lx): Section mismatch in reference to the %s:%s\n",
+		     modname, fromsec, (long)r->r_offset, tosec, tosym);
+
+		if (match(tosec, mismatch->bad_tosec))
+			fatal("The relocation at %s+0x%lx references\n"
+			      "section \"%s\" which is black-listed.\n"
+			      "Something is seriously wrong and should be fixed.\n"
+			      "You might get more information about where this is\n"
+			      "coming from by using scripts/check_extable.sh %s\n",
+			      fromsec, (long)r->r_offset, tosec, modname);
+		else if (is_executable_section(elf, get_secindex(elf, sym)))
+			warn("The relocation at %s+0x%lx references\n"
+			     "section \"%s\" which is not in the list of\n"
+			     "authorized sections.  If you're adding a new section\n"
+			     "and/or if this reference is valid, add \"%s\" to the\n"
+			     "list of authorized sections to jump to on fault.\n"
+			     "This can be achieved by adding \"%s\" to\n"
+			     "OTHER_TEXT_SECTIONS in scripts/mod/modpost.c.\n",
+			     fromsec, (long)r->r_offset, tosec, tosec, tosec);
+		else
+			error("%s+0x%lx references non-executable section '%s'\n",
+			      fromsec, (long)r->r_offset, tosec);
 		break;
 	}
 }
 
-static void extable_mismatch_handler(const char* modname, struct elf_info *elf,
-				     const struct sectioncheck* const mismatch,
-				     Elf_Rela* r, Elf_Sym* sym,
-				     const char *fromsec)
-{
-	const char* tosec = sec_name(elf, get_secindex(elf, sym));
-	Elf_Sym *tosym = find_elf_symbol(elf, r->r_addend, sym);
-	const char *tosym_name = sym_name(elf, tosym);
-
-	sec_mismatch_count++;
-
-	warn("%s(%s+0x%lx): Section mismatch in reference to the %s:%s\n",
-	     modname, fromsec, (long)r->r_offset, tosec, tosym_name);
-
-	if (match(tosec, mismatch->bad_tosec))
-		fatal("The relocation at %s+0x%lx references\n"
-		      "section \"%s\" which is black-listed.\n"
-		      "Something is seriously wrong and should be fixed.\n"
-		      "You might get more information about where this is\n"
-		      "coming from by using scripts/check_extable.sh %s\n",
-		      fromsec, (long)r->r_offset, tosec, modname);
-	else if (is_executable_section(elf, get_secindex(elf, sym)))
-		warn("The relocation at %s+0x%lx references\n"
-		     "section \"%s\" which is not in the list of\n"
-		     "authorized sections.  If you're adding a new section\n"
-		     "and/or if this reference is valid, add \"%s\" to the\n"
-		     "list of authorized sections to jump to on fault.\n"
-		     "This can be achieved by adding \"%s\" to\n"
-		     "OTHER_TEXT_SECTIONS in scripts/mod/modpost.c.\n",
-		     fromsec, (long)r->r_offset, tosec, tosec, tosec);
-	else
-		error("%s+0x%lx references non-executable section '%s'\n",
-		      fromsec, (long)r->r_offset, tosec);
-}
-
 static void check_section_mismatch(const char *modname, struct elf_info *elf,
 				   Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
 {
 	const char *tosec = sec_name(elf, get_secindex(elf, sym));
 	const struct sectioncheck *mismatch = section_mismatch(fromsec, tosec);
 
-	if (mismatch) {
-		if (mismatch->handler)
-			mismatch->handler(modname, elf,  mismatch,
-					  r, sym, fromsec);
-		else
-			default_mismatch_handler(modname, elf, mismatch,
-						 r, sym, fromsec);
-	}
+	if (!mismatch)
+		return;
+
+	default_mismatch_handler(modname, elf, mismatch, r, sym, fromsec);
 }
 
 static unsigned int *reloc_location(struct elf_info *elf,
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 08/21] modpost: pass 'tosec' down to default_mismatch_handler()
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (6 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 07/21] modpost: squash extable_mismatch_handler() into default_mismatch_handler() Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 09/21] modpost: pass section index to find_elf_symbol2() Masahiro Yamada
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

default_mismatch_handler() does not need to compute 'tosec' because
it is calculated by the caller.

Pass it down to default_mismatch_handler() instead of calling
sec_name() twice.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 49357a716519..2cc9c2a4aadc 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1203,9 +1203,9 @@ static bool is_executable_section(struct elf_info *elf, unsigned int secndx)
 
 static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 				     const struct sectioncheck* const mismatch,
-				     Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
+				     Elf_Rela *r, Elf_Sym *sym, const char *fromsec,
+				     const char *tosec)
 {
-	const char *tosec;
 	Elf_Sym *to;
 	Elf_Sym *from;
 	const char *tosym;
@@ -1214,7 +1214,6 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 	from = find_elf_symbol2(elf, r->r_offset, fromsec);
 	fromsym = sym_name(elf, from);
 
-	tosec = sec_name(elf, get_secindex(elf, sym));
 	to = find_elf_symbol(elf, r->r_addend, sym);
 	tosym = sym_name(elf, to);
 
@@ -1276,7 +1275,7 @@ static void check_section_mismatch(const char *modname, struct elf_info *elf,
 	if (!mismatch)
 		return;
 
-	default_mismatch_handler(modname, elf, mismatch, r, sym, fromsec);
+	default_mismatch_handler(modname, elf, mismatch, r, sym, fromsec, tosec);
 }
 
 static unsigned int *reloc_location(struct elf_info *elf,
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 09/21] modpost: pass section index to find_elf_symbol2()
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (7 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 08/21] modpost: pass 'tosec' down to default_mismatch_handler() Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 10/21] modpost: simplify find_elf_symbol() Masahiro Yamada
                   ` (12 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

find_elf_symbol2() converts the section index to the section name,
then compares the two section names in each iteration. This is slow.

It is faster to compare the section indices (i.e. integers) directly.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 2cc9c2a4aadc..3b7b78e69137 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1169,19 +1169,14 @@ static Elf_Sym *find_elf_symbol(struct elf_info *elf, Elf64_Sword addr,
  * it is, but this works for now.
  **/
 static Elf_Sym *find_elf_symbol2(struct elf_info *elf, Elf_Addr addr,
-				 const char *sec)
+				 unsigned int secndx)
 {
 	Elf_Sym *sym;
 	Elf_Sym *near = NULL;
 	Elf_Addr distance = ~0;
 
 	for (sym = elf->symtab_start; sym < elf->symtab_stop; sym++) {
-		const char *symsec;
-
-		if (is_shndx_special(sym->st_shndx))
-			continue;
-		symsec = sec_name(elf, get_secindex(elf, sym));
-		if (strcmp(symsec, sec) != 0)
+		if (get_secindex(elf, sym) != secndx)
 			continue;
 		if (!is_valid_name(elf, sym))
 			continue;
@@ -1203,7 +1198,8 @@ static bool is_executable_section(struct elf_info *elf, unsigned int secndx)
 
 static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 				     const struct sectioncheck* const mismatch,
-				     Elf_Rela *r, Elf_Sym *sym, const char *fromsec,
+				     Elf_Rela *r, Elf_Sym *sym,
+				     unsigned int fsecndx, const char *fromsec,
 				     const char *tosec)
 {
 	Elf_Sym *to;
@@ -1211,7 +1207,7 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 	const char *tosym;
 	const char *fromsym;
 
-	from = find_elf_symbol2(elf, r->r_offset, fromsec);
+	from = find_elf_symbol2(elf, r->r_offset, fsecndx);
 	fromsym = sym_name(elf, from);
 
 	to = find_elf_symbol(elf, r->r_addend, sym);
@@ -1267,7 +1263,8 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 }
 
 static void check_section_mismatch(const char *modname, struct elf_info *elf,
-				   Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
+				   Elf_Rela *r, Elf_Sym *sym,
+				   unsigned int fsecndx, const char *fromsec)
 {
 	const char *tosec = sec_name(elf, get_secindex(elf, sym));
 	const struct sectioncheck *mismatch = section_mismatch(fromsec, tosec);
@@ -1275,7 +1272,8 @@ static void check_section_mismatch(const char *modname, struct elf_info *elf,
 	if (!mismatch)
 		return;
 
-	default_mismatch_handler(modname, elf, mismatch, r, sym, fromsec, tosec);
+	default_mismatch_handler(modname, elf, mismatch, r, sym, fsecndx, fromsec,
+				 tosec);
 }
 
 static unsigned int *reloc_location(struct elf_info *elf,
@@ -1390,12 +1388,11 @@ static void section_rela(const char *modname, struct elf_info *elf,
 	Elf_Rela *rela;
 	Elf_Rela r;
 	unsigned int r_sym;
-	const char *fromsec;
-
+	unsigned int fsecndx = sechdr->sh_info;
+	const char *fromsec = sec_name(elf, fsecndx);
 	Elf_Rela *start = (void *)elf->hdr + sechdr->sh_offset;
 	Elf_Rela *stop  = (void *)start + sechdr->sh_size;
 
-	fromsec = sec_name(elf, sechdr->sh_info);
 	/* if from section (name) is know good then skip it */
 	if (match(fromsec, section_white_list))
 		return;
@@ -1434,7 +1431,7 @@ static void section_rela(const char *modname, struct elf_info *elf,
 		/* Skip special sections */
 		if (is_shndx_special(sym->st_shndx))
 			continue;
-		check_section_mismatch(modname, elf, &r, sym, fromsec);
+		check_section_mismatch(modname, elf, &r, sym, fsecndx, fromsec);
 	}
 }
 
@@ -1445,12 +1442,11 @@ static void section_rel(const char *modname, struct elf_info *elf,
 	Elf_Rel *rel;
 	Elf_Rela r;
 	unsigned int r_sym;
-	const char *fromsec;
-
+	unsigned int fsecndx = sechdr->sh_info;
+	const char *fromsec = sec_name(elf, fsecndx);
 	Elf_Rel *start = (void *)elf->hdr + sechdr->sh_offset;
 	Elf_Rel *stop  = (void *)start + sechdr->sh_size;
 
-	fromsec = sec_name(elf, sechdr->sh_info);
 	/* if from section (name) is know good then skip it */
 	if (match(fromsec, section_white_list))
 		return;
@@ -1493,7 +1489,7 @@ static void section_rel(const char *modname, struct elf_info *elf,
 		/* Skip special sections */
 		if (is_shndx_special(sym->st_shndx))
 			continue;
-		check_section_mismatch(modname, elf, &r, sym, fromsec);
+		check_section_mismatch(modname, elf, &r, sym, fsecndx, fromsec);
 	}
 }
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 10/21] modpost: simplify find_elf_symbol()
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (8 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 09/21] modpost: pass section index to find_elf_symbol2() Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 11/21] modpost: rename find_elf_symbol() and find_elf_symbol2() Masahiro Yamada
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

I do not know why commit 157c23c80eed ("kbuild: use simpler section
mismatch warnings in modpost") is only applicable to find_elf_symbol2().

Simplify find_elf_symbol() based on find_elf_symbol2().

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 64 +++++++++----------------------------------
 1 file changed, 13 insertions(+), 51 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 3b7b78e69137..b4fa9e0be4d1 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1117,57 +1117,7 @@ static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym)
 	return !is_mapping_symbol(name);
 }
 
-/**
- * Find symbol based on relocation record info.
- * In some cases the symbol supplied is a valid symbol so
- * return refsym. If st_name != 0 we assume this is a valid symbol.
- * In other cases the symbol needs to be looked up in the symbol table
- * based on section and address.
- *  **/
-static Elf_Sym *find_elf_symbol(struct elf_info *elf, Elf64_Sword addr,
-				Elf_Sym *relsym)
-{
-	Elf_Sym *sym;
-	Elf_Sym *near = NULL;
-	Elf64_Sword distance = 20;
-	Elf64_Sword d;
-	unsigned int relsym_secindex;
-
-	if (relsym->st_name != 0)
-		return relsym;
-
-	relsym_secindex = get_secindex(elf, relsym);
-	for (sym = elf->symtab_start; sym < elf->symtab_stop; sym++) {
-		if (get_secindex(elf, sym) != relsym_secindex)
-			continue;
-		if (ELF_ST_TYPE(sym->st_info) == STT_SECTION)
-			continue;
-		if (!is_valid_name(elf, sym))
-			continue;
-		if (sym->st_value == addr)
-			return sym;
-		/* Find a symbol nearby - addr are maybe negative */
-		d = sym->st_value - addr;
-		if (d < 0)
-			d = addr - sym->st_value;
-		if (d < distance) {
-			distance = d;
-			near = sym;
-		}
-	}
-	/* We need a close match */
-	if (distance < 20)
-		return near;
-	else
-		return NULL;
-}
-
-/*
- * Find symbols before or equal addr and after addr - in the section sec.
- * If we find two symbols with equal offset prefer one with a valid name.
- * The ELF format may have a better way to detect what type of symbol
- * it is, but this works for now.
- **/
+/* Look up the nearest symbol based on the section and the address */
 static Elf_Sym *find_elf_symbol2(struct elf_info *elf, Elf_Addr addr,
 				 unsigned int secndx)
 {
@@ -1178,6 +1128,8 @@ static Elf_Sym *find_elf_symbol2(struct elf_info *elf, Elf_Addr addr,
 	for (sym = elf->symtab_start; sym < elf->symtab_stop; sym++) {
 		if (get_secindex(elf, sym) != secndx)
 			continue;
+		if (ELF_ST_TYPE(sym->st_info) == STT_SECTION)
+			continue;
 		if (!is_valid_name(elf, sym))
 			continue;
 		if (sym->st_value <= addr && addr - sym->st_value <= distance) {
@@ -1188,6 +1140,16 @@ static Elf_Sym *find_elf_symbol2(struct elf_info *elf, Elf_Addr addr,
 	return near;
 }
 
+static Elf_Sym *find_elf_symbol(struct elf_info *elf, Elf_Addr addr, Elf_Sym *sym)
+{
+	/* If the supplied symbol is valid, return it */
+	if (sym->st_name != 0)
+		return sym;
+
+	/* Otherwise, look up a better symbol */
+	return find_elf_symbol2(elf, addr, get_secindex(elf, sym));
+}
+
 static bool is_executable_section(struct elf_info *elf, unsigned int secndx)
 {
 	if (secndx > elf->num_sections)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 11/21] modpost: rename find_elf_symbol() and find_elf_symbol2()
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (9 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 10/21] modpost: simplify find_elf_symbol() Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 12/21] modpost: unify 'sym' and 'to' in default_mismatch_handler() Masahiro Yamada
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

find_elf_symbol() and find_elf_symbol2() are not good names.

Rename them to find_better_symbol(), find_nearest_symbol(),
respectively.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index b4fa9e0be4d1..d2a9c655f6ea 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1118,8 +1118,8 @@ static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym)
 }
 
 /* Look up the nearest symbol based on the section and the address */
-static Elf_Sym *find_elf_symbol2(struct elf_info *elf, Elf_Addr addr,
-				 unsigned int secndx)
+static Elf_Sym *find_nearest_symbol(struct elf_info *elf, Elf_Addr addr,
+				    unsigned int secndx)
 {
 	Elf_Sym *sym;
 	Elf_Sym *near = NULL;
@@ -1140,14 +1140,14 @@ static Elf_Sym *find_elf_symbol2(struct elf_info *elf, Elf_Addr addr,
 	return near;
 }
 
-static Elf_Sym *find_elf_symbol(struct elf_info *elf, Elf_Addr addr, Elf_Sym *sym)
+static Elf_Sym *find_better_symbol(struct elf_info *elf, Elf_Addr addr, Elf_Sym *sym)
 {
 	/* If the supplied symbol is valid, return it */
 	if (sym->st_name != 0)
 		return sym;
 
 	/* Otherwise, look up a better symbol */
-	return find_elf_symbol2(elf, addr, get_secindex(elf, sym));
+	return find_nearest_symbol(elf, addr, get_secindex(elf, sym));
 }
 
 static bool is_executable_section(struct elf_info *elf, unsigned int secndx)
@@ -1169,10 +1169,10 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 	const char *tosym;
 	const char *fromsym;
 
-	from = find_elf_symbol2(elf, r->r_offset, fsecndx);
+	from = find_nearest_symbol(elf, r->r_offset, fsecndx);
 	fromsym = sym_name(elf, from);
 
-	to = find_elf_symbol(elf, r->r_addend, sym);
+	to = find_better_symbol(elf, r->r_addend, sym);
 	tosym = sym_name(elf, to);
 
 	/* check whitelist - we may ignore it */
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 12/21] modpost: unify 'sym' and 'to' in default_mismatch_handler()
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (10 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 11/21] modpost: rename find_elf_symbol() and find_elf_symbol2() Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 13/21] modpost: replace r->r_offset, r->r_addend with faddr, taddr Masahiro Yamada
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

find_better_symbol() takes 'sym' and stores the return value to
another variable 'to'.

We can use the same variable because we want to replace the original
one if possible.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index d2a9c655f6ea..9e81cd6803a7 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1160,11 +1160,10 @@ static bool is_executable_section(struct elf_info *elf, unsigned int secndx)
 
 static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 				     const struct sectioncheck* const mismatch,
-				     Elf_Rela *r, Elf_Sym *sym,
+				     Elf_Rela *r, Elf_Sym *tsym,
 				     unsigned int fsecndx, const char *fromsec,
 				     const char *tosec)
 {
-	Elf_Sym *to;
 	Elf_Sym *from;
 	const char *tosym;
 	const char *fromsym;
@@ -1172,8 +1171,8 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 	from = find_nearest_symbol(elf, r->r_offset, fsecndx);
 	fromsym = sym_name(elf, from);
 
-	to = find_better_symbol(elf, r->r_addend, sym);
-	tosym = sym_name(elf, to);
+	tsym = find_better_symbol(elf, r->r_addend, tsym);
+	tosym = sym_name(elf, tsym);
 
 	/* check whitelist - we may ignore it */
 	if (!secref_whitelist(mismatch, fromsec, fromsym, tosec, tosym))
@@ -1208,7 +1207,7 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 			      "You might get more information about where this is\n"
 			      "coming from by using scripts/check_extable.sh %s\n",
 			      fromsec, (long)r->r_offset, tosec, modname);
-		else if (is_executable_section(elf, get_secindex(elf, sym)))
+		else if (is_executable_section(elf, get_secindex(elf, tsym)))
 			warn("The relocation at %s+0x%lx references\n"
 			     "section \"%s\" which is not in the list of\n"
 			     "authorized sections.  If you're adding a new section\n"
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 13/21] modpost: replace r->r_offset, r->r_addend with faddr, taddr
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (11 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 12/21] modpost: unify 'sym' and 'to' in default_mismatch_handler() Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 14/21] modpost: remove is_shndx_special() check from section_rel(a) Masahiro Yamada
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

r_offset/r_addend holds the offset address from/to which a symbol is
referenced. It is unclear unless you are familiar with ELF.

Rename them to faddr, taddr, respectively. The prefix 'f' means 'from',
't' means 'to'.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 9e81cd6803a7..43ce35377f6a 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1160,18 +1160,18 @@ static bool is_executable_section(struct elf_info *elf, unsigned int secndx)
 
 static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 				     const struct sectioncheck* const mismatch,
-				     Elf_Rela *r, Elf_Sym *tsym,
-				     unsigned int fsecndx, const char *fromsec,
-				     const char *tosec)
+				     Elf_Sym *tsym,
+				     unsigned int fsecndx, const char *fromsec, Elf_Addr faddr,
+				     const char *tosec, Elf_Addr taddr)
 {
 	Elf_Sym *from;
 	const char *tosym;
 	const char *fromsym;
 
-	from = find_nearest_symbol(elf, r->r_offset, fsecndx);
+	from = find_nearest_symbol(elf, faddr, fsecndx);
 	fromsym = sym_name(elf, from);
 
-	tsym = find_better_symbol(elf, r->r_addend, tsym);
+	tsym = find_better_symbol(elf, taddr, tsym);
 	tosym = sym_name(elf, tsym);
 
 	/* check whitelist - we may ignore it */
@@ -1198,7 +1198,7 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 		break;
 	case EXTABLE_TO_NON_TEXT:
 		warn("%s(%s+0x%lx): Section mismatch in reference to the %s:%s\n",
-		     modname, fromsec, (long)r->r_offset, tosec, tosym);
+		     modname, fromsec, (long)faddr, tosec, tosym);
 
 		if (match(tosec, mismatch->bad_tosec))
 			fatal("The relocation at %s+0x%lx references\n"
@@ -1206,7 +1206,7 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 			      "Something is seriously wrong and should be fixed.\n"
 			      "You might get more information about where this is\n"
 			      "coming from by using scripts/check_extable.sh %s\n",
-			      fromsec, (long)r->r_offset, tosec, modname);
+			      fromsec, (long)faddr, tosec, modname);
 		else if (is_executable_section(elf, get_secindex(elf, tsym)))
 			warn("The relocation at %s+0x%lx references\n"
 			     "section \"%s\" which is not in the list of\n"
@@ -1215,17 +1215,18 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 			     "list of authorized sections to jump to on fault.\n"
 			     "This can be achieved by adding \"%s\" to\n"
 			     "OTHER_TEXT_SECTIONS in scripts/mod/modpost.c.\n",
-			     fromsec, (long)r->r_offset, tosec, tosec, tosec);
+			     fromsec, (long)faddr, tosec, tosec, tosec);
 		else
 			error("%s+0x%lx references non-executable section '%s'\n",
-			      fromsec, (long)r->r_offset, tosec);
+			      fromsec, (long)faddr, tosec);
 		break;
 	}
 }
 
 static void check_section_mismatch(const char *modname, struct elf_info *elf,
-				   Elf_Rela *r, Elf_Sym *sym,
-				   unsigned int fsecndx, const char *fromsec)
+				   Elf_Sym *sym,
+				   unsigned int fsecndx, const char *fromsec,
+				   Elf_Addr faddr, Elf_Addr taddr)
 {
 	const char *tosec = sec_name(elf, get_secindex(elf, sym));
 	const struct sectioncheck *mismatch = section_mismatch(fromsec, tosec);
@@ -1233,8 +1234,9 @@ static void check_section_mismatch(const char *modname, struct elf_info *elf,
 	if (!mismatch)
 		return;
 
-	default_mismatch_handler(modname, elf, mismatch, r, sym, fsecndx, fromsec,
-				 tosec);
+	default_mismatch_handler(modname, elf, mismatch, sym,
+				 fsecndx, fromsec, faddr,
+				 tosec, taddr);
 }
 
 static unsigned int *reloc_location(struct elf_info *elf,
@@ -1392,7 +1394,8 @@ static void section_rela(const char *modname, struct elf_info *elf,
 		/* Skip special sections */
 		if (is_shndx_special(sym->st_shndx))
 			continue;
-		check_section_mismatch(modname, elf, &r, sym, fsecndx, fromsec);
+		check_section_mismatch(modname, elf, sym,
+				       fsecndx, fromsec, r.r_offset, r.r_addend);
 	}
 }
 
@@ -1450,7 +1453,8 @@ static void section_rel(const char *modname, struct elf_info *elf,
 		/* Skip special sections */
 		if (is_shndx_special(sym->st_shndx))
 			continue;
-		check_section_mismatch(modname, elf, &r, sym, fsecndx, fromsec);
+		check_section_mismatch(modname, elf, sym,
+				       fsecndx, fromsec, r.r_offset, r.r_addend);
 	}
 }
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 14/21] modpost: remove is_shndx_special() check from section_rel(a)
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (12 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 13/21] modpost: replace r->r_offset, r->r_addend with faddr, taddr Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 15/21] modpost: pass struct module pointer to check_section_mismatch() Masahiro Yamada
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

This check is unneeded. Without it, sec_name() will returns the null
string "", then section_mismatch() will return immediately.

Anyway, special section indices do not appear quite often in these
loops.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 43ce35377f6a..1c5dd81c0987 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1347,7 +1347,6 @@ static int addend_mips_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
 static void section_rela(const char *modname, struct elf_info *elf,
 			 Elf_Shdr *sechdr)
 {
-	Elf_Sym  *sym;
 	Elf_Rela *rela;
 	Elf_Rela r;
 	unsigned int r_sym;
@@ -1390,11 +1389,8 @@ static void section_rela(const char *modname, struct elf_info *elf,
 				continue;
 			break;
 		}
-		sym = elf->symtab_start + r_sym;
-		/* Skip special sections */
-		if (is_shndx_special(sym->st_shndx))
-			continue;
-		check_section_mismatch(modname, elf, sym,
+
+		check_section_mismatch(modname, elf, elf->symtab_start + r_sym,
 				       fsecndx, fromsec, r.r_offset, r.r_addend);
 	}
 }
@@ -1402,7 +1398,6 @@ static void section_rela(const char *modname, struct elf_info *elf,
 static void section_rel(const char *modname, struct elf_info *elf,
 			Elf_Shdr *sechdr)
 {
-	Elf_Sym *sym;
 	Elf_Rel *rel;
 	Elf_Rela r;
 	unsigned int r_sym;
@@ -1449,11 +1444,8 @@ static void section_rel(const char *modname, struct elf_info *elf,
 		default:
 			fatal("Please add code to calculate addend for this architecture\n");
 		}
-		sym = elf->symtab_start + r_sym;
-		/* Skip special sections */
-		if (is_shndx_special(sym->st_shndx))
-			continue;
-		check_section_mismatch(modname, elf, sym,
+
+		check_section_mismatch(modname, elf, elf->symtab_start + r_sym,
 				       fsecndx, fromsec, r.r_offset, r.r_addend);
 	}
 }
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 15/21] modpost: pass struct module pointer to check_section_mismatch()
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (13 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 14/21] modpost: remove is_shndx_special() check from section_rel(a) Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 16/21] kbuild: generate KSYMTAB entries by modpost Masahiro Yamada
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

The next commit will use it.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 1c5dd81c0987..e82299c90836 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1223,7 +1223,7 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 	}
 }
 
-static void check_section_mismatch(const char *modname, struct elf_info *elf,
+static void check_section_mismatch(struct module *mod, struct elf_info *elf,
 				   Elf_Sym *sym,
 				   unsigned int fsecndx, const char *fromsec,
 				   Elf_Addr faddr, Elf_Addr taddr)
@@ -1234,7 +1234,7 @@ static void check_section_mismatch(const char *modname, struct elf_info *elf,
 	if (!mismatch)
 		return;
 
-	default_mismatch_handler(modname, elf, mismatch, sym,
+	default_mismatch_handler(mod->name, elf, mismatch, sym,
 				 fsecndx, fromsec, faddr,
 				 tosec, taddr);
 }
@@ -1344,7 +1344,7 @@ static int addend_mips_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
 #define R_LARCH_SUB32		55
 #endif
 
-static void section_rela(const char *modname, struct elf_info *elf,
+static void section_rela(struct module *mod, struct elf_info *elf,
 			 Elf_Shdr *sechdr)
 {
 	Elf_Rela *rela;
@@ -1395,7 +1395,7 @@ static void section_rela(const char *modname, struct elf_info *elf,
 	}
 }
 
-static void section_rel(const char *modname, struct elf_info *elf,
+static void section_rel(struct module *mod, struct elf_info *elf,
 			Elf_Shdr *sechdr)
 {
 	Elf_Rel *rel;
@@ -1462,19 +1462,19 @@ static void section_rel(const char *modname, struct elf_info *elf,
  * to find all references to a section that reference a section that will
  * be discarded and warns about it.
  **/
-static void check_sec_ref(const char *modname, struct elf_info *elf)
+static void check_sec_ref(struct module *mod, struct elf_info *elf)
 {
 	int i;
 	Elf_Shdr *sechdrs = elf->sechdrs;
 
 	/* Walk through all sections */
 	for (i = 0; i < elf->num_sections; i++) {
-		check_section(modname, elf, &elf->sechdrs[i]);
+		check_section(mod->name, elf, &elf->sechdrs[i]);
 		/* We want to process only relocation sections and not .init */
 		if (sechdrs[i].sh_type == SHT_RELA)
-			section_rela(modname, elf, &elf->sechdrs[i]);
+			section_rela(mod, elf, &elf->sechdrs[i]);
 		else if (sechdrs[i].sh_type == SHT_REL)
-			section_rel(modname, elf, &elf->sechdrs[i]);
+			section_rel(mod, elf, &elf->sechdrs[i]);
 	}
 }
 
@@ -1645,7 +1645,7 @@ static void read_symbols(const char *modname)
 					     sym_get_data(&info, sym));
 	}
 
-	check_sec_ref(modname, &info);
+	check_sec_ref(mod, &info);
 
 	if (!mod->is_vmlinux) {
 		version = get_modinfo(&info, "version");
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 16/21] kbuild: generate KSYMTAB entries by modpost
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (14 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 15/21] modpost: pass struct module pointer to check_section_mismatch() Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 17/21] ia64,export.h: replace EXPORT_DATA_SYMBOL* with EXPORT_SYMBOL* Masahiro Yamada
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

Commit 7b4537199a4a ("kbuild: link symbol CRCs at final link, removing
CONFIG_MODULE_REL_CRCS") made modpost output CRCs in the same way
whether the EXPORT_SYMBOL() is placed in *.c or *.S.

This commit applies a similar approach to the entire data structure of
EXPORT_SYMBOL() for further cleanups. The EXPORT_SYMBOL() compilation
is split into two stages.

When a source file is compiled, EXPORT_SYMBOL() is converted into a
dummy symbol in the .export_symbol section.

For example,

    EXPORT_SYMBOL(foo);
    EXPORT_SYMBOL_NS_GPL(bar, BAR_NAMESPACE);

will be encoded into the following assembly code:

    .section ".export_symbol","a"
    __export_symbol.foo:
            .asciz ""
            .quad foo
    .previous

    .section ".export_symbol","a"
    __export_symbol_gpl.bar:
            .asciz "BAR_NAMESPACE"
            .quad bar
    .previous

They are just markers to tell modpost the name, license, and namespace
of the symbols. They will be dropped from the final vmlinux and modules
because the *(.export_symbol) will go into /DISCARD/ in the linker script.

Then, modpost extracts all the information about EXPORT_SYMBOL() from the
.export_symbol section, and generates C code:

    KSYMTAB_FUNC(foo, "", "");
    KSYMTAB_FUNC(bar, "_gpl", "BAR_NAMESPACE");

KSYMTAB_FUNC() (or KSYMTAB_DATA() if it is data) is expanded to struct
kernel_symbol that will be linked to the vmlinux or a module.

With this change, EXPORT_SYMBOL() works in the same way for *.c and *.S
files, providing the following benefits.

[1] Deprecate EXPORT_DATA_SYMBOL()

In the old days, EXPORT_SYMBOL() was only available in C files. To export
a symbol in *.S, EXPORT_SYMBOL() was placed in a separate *.c file.
arch/arm/kernel/armksyms.c is one example written in the classic manner.

Commit 22823ab419d8 ("EXPORT_SYMBOL() for asm") removed this limitation.
Since then, EXPORT_SYMBOL() can be placed close to the symbol definition
in *.S files. It was a nice improvement.

However, as that commit mentioned, you need to use EXPORT_DATA_SYMBOL()
for data objects on some architectures.

In the new approach, modpost checks symbol's type (STT_FUNC or not),
and outputs KSYMTAB_FUNC() or KSYMTAB_DATA() accordingly.

There are only two users of EXPORT_DATA_SYMBOL:

  EXPORT_DATA_SYMBOL_GPL(empty_zero_page)    (arch/ia64/kernel/head.S)
  EXPORT_DATA_SYMBOL(ia64_ivt)               (arch/ia64/kernel/ivt.S)

They are transformed as follows and output into .vmlinux.export.c

  KSYMTAB_DATA(empty_zero_page, "_gpl", "");
  KSYMTAB_DATA(ia64_ivt, "", "");

The other EXPORT_SYMBOL users in ia64 assembly are output as
KSYMTAB_FUNC().

EXPORT_DATA_SYMBOL() is now deprecated.

[2] merge <linux/export.h> and <asm-generic/export.h>

There are two similar header implementations:

  include/linux/export.h        for .c files
  include/asm-generic/export.h  for .S files

Ideally, the functionality should be consistent between them, but they
tend to diverge.

Commit 8651ec01daed ("module: add support for symbol namespaces.") did
not support the namespace for *.S files.

This commit shifts the essential implementation part to C, which supports
EXPORT_SYMBOL_NS() for *.S files.

<asm/export.h> and <asm-generic/export.h> will remain as a wrapper of
<linux/export.h> for a while.

They will be removed after #include <asm/export.h> directives are all
replaced with #include <linux/export.h>.

[3] Implement CONFIG_TRIM_UNUSED_KSYMS in one-pass algorithm (by a later commit)

When CONFIG_TRIM_UNUSED_KSYMS is enabled, Kbuild recursively traverses
the directory tree to determine which EXPORT_SYMBOL to trim. If an
EXPORT_SYMBOL turns out to be unused by anyone, Kbuild begins the
second traverse, where some source files are recompiled with their
EXPORT_SYMBOL() tuned into a no-op.

We can do this better now; modpost can selectively emit KSYMTAB entries
that are really used by modules.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---


 arch/ia64/include/asm/Kbuild      |   1 +
 arch/ia64/include/asm/export.h    |   3 -
 include/asm-generic/export.h      |  84 ++----------------------
 include/asm-generic/vmlinux.lds.h |   1 +
 include/linux/export-internal.h   |  49 ++++++++++++++
 include/linux/export.h            |  99 ++++++++++------------------
 include/linux/pm.h                |   8 +--
 kernel/module/internal.h          |  12 ++++
 scripts/Makefile.build            |   8 +--
 scripts/check-local-export        |   4 +-
 scripts/mod/modpost.c             | 105 ++++++++++++++++++++----------
 scripts/mod/modpost.h             |   1 +
 12 files changed, 182 insertions(+), 193 deletions(-)
 delete mode 100644 arch/ia64/include/asm/export.h

diff --git a/arch/ia64/include/asm/Kbuild b/arch/ia64/include/asm/Kbuild
index aefae2efde9f..33733245f42b 100644
--- a/arch/ia64/include/asm/Kbuild
+++ b/arch/ia64/include/asm/Kbuild
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 generated-y += syscall_table.h
 generic-y += agp.h
+generic-y += export.h
 generic-y += kvm_para.h
 generic-y += mcs_spinlock.h
 generic-y += vtime.h
diff --git a/arch/ia64/include/asm/export.h b/arch/ia64/include/asm/export.h
deleted file mode 100644
index ad18c6583252..000000000000
--- a/arch/ia64/include/asm/export.h
+++ /dev/null
@@ -1,3 +0,0 @@
-/* EXPORT_DATA_SYMBOL != EXPORT_SYMBOL here */
-#define KSYM_FUNC(name) @fptr(name)
-#include <asm-generic/export.h>
diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
index 5e4b1f2369d2..0ae9f38a904c 100644
--- a/include/asm-generic/export.h
+++ b/include/asm-generic/export.h
@@ -3,86 +3,12 @@
 #define __ASM_GENERIC_EXPORT_H
 
 /*
- * This comment block is used by fixdep. Please do not remove.
- *
- * When CONFIG_MODVERSIONS is changed from n to y, all source files having
- * EXPORT_SYMBOL variants must be re-compiled because genksyms is run as a
- * side effect of the *.o build rule.
+ * <asm/export.h> and <asm-generic/export.h> are deprecated.
+ * Please include <linux/export.h> directly.
  */
+#include <linux/export.h>
 
-#ifndef KSYM_FUNC
-#define KSYM_FUNC(x) x
-#endif
-#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
-#define KSYM_ALIGN 4
-#elif defined(CONFIG_64BIT)
-#define KSYM_ALIGN 8
-#else
-#define KSYM_ALIGN 4
-#endif
-
-.macro __put, val, name
-#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
-	.long	\val - ., \name - ., 0
-#elif defined(CONFIG_64BIT)
-	.quad	\val, \name, 0
-#else
-	.long	\val, \name, 0
-#endif
-.endm
-
-/*
- * note on .section use: we specify progbits since usage of the "M" (SHF_MERGE)
- * section flag requires it. Use '%progbits' instead of '@progbits' since the
- * former apparently works on all arches according to the binutils source.
- */
-
-.macro ___EXPORT_SYMBOL name,val,sec
-#if defined(CONFIG_MODULES) && !defined(__DISABLE_EXPORTS)
-	.section ___ksymtab\sec+\name,"a"
-	.balign KSYM_ALIGN
-__ksymtab_\name:
-	__put \val, __kstrtab_\name
-	.previous
-	.section __ksymtab_strings,"aMS",%progbits,1
-__kstrtab_\name:
-	.asciz "\name"
-	.previous
-#endif
-.endm
-
-#if defined(CONFIG_TRIM_UNUSED_KSYMS)
-
-#include <linux/kconfig.h>
-#include <generated/autoksyms.h>
-
-.macro __ksym_marker sym
-	.section ".discard.ksym","a"
-__ksym_marker_\sym:
-	 .previous
-.endm
-
-#define __EXPORT_SYMBOL(sym, val, sec)				\
-	__ksym_marker sym;					\
-	__cond_export_sym(sym, val, sec, __is_defined(__KSYM_##sym))
-#define __cond_export_sym(sym, val, sec, conf)			\
-	___cond_export_sym(sym, val, sec, conf)
-#define ___cond_export_sym(sym, val, sec, enabled)		\
-	__cond_export_sym_##enabled(sym, val, sec)
-#define __cond_export_sym_1(sym, val, sec) ___EXPORT_SYMBOL sym, val, sec
-#define __cond_export_sym_0(sym, val, sec) /* nothing */
-
-#else
-#define __EXPORT_SYMBOL(sym, val, sec) ___EXPORT_SYMBOL sym, val, sec
-#endif
-
-#define EXPORT_SYMBOL(name)					\
-	__EXPORT_SYMBOL(name, KSYM_FUNC(name),)
-#define EXPORT_SYMBOL_GPL(name) 				\
-	__EXPORT_SYMBOL(name, KSYM_FUNC(name), _gpl)
-#define EXPORT_DATA_SYMBOL(name)				\
-	__EXPORT_SYMBOL(name, name,)
-#define EXPORT_DATA_SYMBOL_GPL(name)				\
-	__EXPORT_SYMBOL(name, name,_gpl)
+#define EXPORT_DATA_SYMBOL(name)	EXPORT_SYMBOL(name)
+#define EXPORT_DATA_SYMBOL_GPL(name)	EXPORT_SYMBOL_GPL(name)
 
 #endif
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index d1f57e4868ed..e65d55e8819c 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -1006,6 +1006,7 @@
 	PATCHABLE_DISCARDS						\
 	*(.discard)							\
 	*(.discard.*)							\
+	*(.export_symbol)						\
 	*(.modinfo)							\
 	/* ld.bfd warns about .gnu.version* even when not emitted */	\
 	*(.gnu.version*)						\
diff --git a/include/linux/export-internal.h b/include/linux/export-internal.h
index fe7e6ba918f1..1c849db953a5 100644
--- a/include/linux/export-internal.h
+++ b/include/linux/export-internal.h
@@ -10,6 +10,55 @@
 #include <linux/compiler.h>
 #include <linux/types.h>
 
+#if defined(CONFIG_HAVE_ARCH_PREL32_RELOCATIONS)
+/*
+ * relative reference: this reduces the size by half on 64-bit architectures,
+ * and eliminates the need for absolute relocations that require runtime
+ * processing on relocatable kernels.
+ */
+#define __KSYM_REF(sym)		".long " #sym "- ."
+#elif defined(CONFIG_64BIT)
+#define __KSYM_REF(sym)		".quad " #sym
+#else
+#define __KSYM_REF(sym)		".long " #sym
+#endif
+
+/*
+ * For every exported symbol, do the following:
+ *
+ * - Put the name of the symbol and namespace (empty string "" for none) in
+ *   __ksymtab_strings.
+ * - Place a struct kernel_symbol entry in the __ksymtab section.
+ *
+ * Note on .section use: we specify progbits since usage of the "M" (SHF_MERGE)
+ * section flag requires it. Use '%progbits' instead of '@progbits' since the
+ * former apparently works on all arches according to the binutils source.
+ */
+#define __KSYMTAB(name, sym, sec, ns)						\
+	asm("	.section \"__ksymtab_strings\",\"aMS\",%progbits,1"	"\n"	\
+	    "__kstrtab_" #name ":"					"\n"	\
+	    "	.asciz \"" #name "\""					"\n"	\
+	    "__kstrtabns_" #name ":"					"\n"	\
+	    "	.asciz \"" ns "\""					"\n"	\
+	    "	.previous"						"\n"	\
+	    "	.section \"___ksymtab" sec "+" #name "\", \"a\""	"\n"	\
+	    "	.balign	4"						"\n"	\
+	    "__ksymtab_" #name ":"					"\n"	\
+		__KSYM_REF(sym)						"\n"	\
+		__KSYM_REF(__kstrtab_ ##name)				"\n"	\
+		__KSYM_REF(__kstrtabns_ ##name)				"\n"	\
+	    "	.previous"						"\n"	\
+	)
+
+#ifdef CONFIG_IA64
+#define KSYM_FUNC(name)		@fptr(name)
+#else
+#define KSYM_FUNC(name)		name
+#endif
+
+#define KSYMTAB_FUNC(name, sec, ns)	__KSYMTAB(name, KSYM_FUNC(name), sec, ns)
+#define KSYMTAB_DATA(name, sec, ns)	__KSYMTAB(name, name, sec, ns)
+
 #define SYMBOL_CRC(sym, crc, sec)   \
 	asm(".section \"___kcrctab" sec "+" #sym "\",\"a\""	"\n" \
 	    "__crc_" #sym ":"					"\n" \
diff --git a/include/linux/export.h b/include/linux/export.h
index 3f31ced0d977..4335a17a1ea7 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -2,6 +2,7 @@
 #ifndef _LINUX_EXPORT_H
 #define _LINUX_EXPORT_H
 
+#include <linux/compiler.h>
 #include <linux/stringify.h>
 
 /*
@@ -28,72 +29,30 @@ extern struct module __this_module;
 #else
 #define THIS_MODULE ((struct module *)0)
 #endif
-
-#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
-#include <linux/compiler.h>
-/*
- * Emit the ksymtab entry as a pair of relative references: this reduces
- * the size by half on 64-bit architectures, and eliminates the need for
- * absolute relocations that require runtime processing on relocatable
- * kernels.
- */
-#define __KSYMTAB_ENTRY(sym, sec)					\
-	__ADDRESSABLE(sym)						\
-	asm("	.section \"___ksymtab" sec "+" #sym "\", \"a\"	\n"	\
-	    "	.balign	4					\n"	\
-	    "__ksymtab_" #sym ":				\n"	\
-	    "	.long	" #sym "- .				\n"	\
-	    "	.long	__kstrtab_" #sym "- .			\n"	\
-	    "	.long	__kstrtabns_" #sym "- .			\n"	\
-	    "	.previous					\n")
-
-struct kernel_symbol {
-	int value_offset;
-	int name_offset;
-	int namespace_offset;
-};
-#else
-#define __KSYMTAB_ENTRY(sym, sec)					\
-	static const struct kernel_symbol __ksymtab_##sym		\
-	__attribute__((section("___ksymtab" sec "+" #sym), used))	\
-	__aligned(sizeof(void *))					\
-	= { (unsigned long)&sym, __kstrtab_##sym, __kstrtabns_##sym }
-
-struct kernel_symbol {
-	unsigned long value;
-	const char *name;
-	const char *namespace;
-};
-#endif
+#endif /* __ASSEMBLY__ */
 
 #ifdef __GENKSYMS__
 
 #define ___EXPORT_SYMBOL(sym, sec, ns)	__GENKSYMS_EXPORT_SYMBOL(sym)
 
+#elif defined(__ASSEMBLY__)
+
+#define ___EXPORT_SYMBOL(sym, sec, ns)				\
+	.section ".export_symbol","a" ;				\
+	__export_symbol##sec##.sym: ;				\
+	.asciz ns ;						\
+	.quad sym ;						\
+	.previous
+
 #else
 
-/*
- * For every exported symbol, do the following:
- *
- * - Put the name of the symbol and namespace (empty string "" for none) in
- *   __ksymtab_strings.
- * - Place a struct kernel_symbol entry in the __ksymtab section.
- *
- * note on .section use: we specify progbits since usage of the "M" (SHF_MERGE)
- * section flag requires it. Use '%progbits' instead of '@progbits' since the
- * former apparently works on all arches according to the binutils source.
- */
-#define ___EXPORT_SYMBOL(sym, sec, ns)						\
-	extern typeof(sym) sym;							\
-	extern const char __kstrtab_##sym[];					\
-	extern const char __kstrtabns_##sym[];					\
-	asm("	.section \"__ksymtab_strings\",\"aMS\",%progbits,1	\n"	\
-	    "__kstrtab_" #sym ":					\n"	\
-	    "	.asciz 	\"" #sym "\"					\n"	\
-	    "__kstrtabns_" #sym ":					\n"	\
-	    "	.asciz 	\"" ns "\"					\n"	\
-	    "	.previous						\n");	\
-	__KSYMTAB_ENTRY(sym, sec)
+#define ___EXPORT_SYMBOL(sym, sec, ns)				\
+	__ADDRESSABLE(sym)					\
+	asm(".section \".export_symbol\",\"a\"		\n"	\
+	    "__export_symbol" #sec "." #sym ":		\n"	\
+	    ".asciz " "\"" ns "\"" "			\n"	\
+	    ".quad " #sym "				\n"	\
+	    ".previous					\n")
 
 #endif
 
@@ -117,9 +76,21 @@ struct kernel_symbol {
  * from the $(NM) output (see scripts/gen_ksymdeps.sh). These symbols are
  * discarded in the final link stage.
  */
+
+#ifdef __ASSEMBLY__
+
+#define __ksym_marker(sym)					\
+	.section ".discard.ksym","a" ;				\
+__ksym_marker_##sym: ;						\
+	.previous
+
+#else
+
 #define __ksym_marker(sym)	\
 	static int __ksym_marker_##sym[0] __section(".discard.ksym") __used
 
+#endif
+
 #define __EXPORT_SYMBOL(sym, sec, ns)					\
 	__ksym_marker(sym);						\
 	__cond_export_sym(sym, sec, ns, __is_defined(__KSYM_##sym))
@@ -147,11 +118,9 @@ struct kernel_symbol {
 #define _EXPORT_SYMBOL(sym, sec)	__EXPORT_SYMBOL(sym, sec, "")
 #endif
 
-#define EXPORT_SYMBOL(sym)		_EXPORT_SYMBOL(sym, "")
-#define EXPORT_SYMBOL_GPL(sym)		_EXPORT_SYMBOL(sym, "_gpl")
-#define EXPORT_SYMBOL_NS(sym, ns)	__EXPORT_SYMBOL(sym, "", __stringify(ns))
-#define EXPORT_SYMBOL_NS_GPL(sym, ns)	__EXPORT_SYMBOL(sym, "_gpl", __stringify(ns))
-
-#endif /* !__ASSEMBLY__ */
+#define EXPORT_SYMBOL(sym)		_EXPORT_SYMBOL(sym,)
+#define EXPORT_SYMBOL_GPL(sym)		_EXPORT_SYMBOL(sym,_gpl)
+#define EXPORT_SYMBOL_NS(sym, ns)	__EXPORT_SYMBOL(sym,, __stringify(ns))
+#define EXPORT_SYMBOL_NS_GPL(sym, ns)	__EXPORT_SYMBOL(sym,_gpl, __stringify(ns))
 
 #endif /* _LINUX_EXPORT_H */
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 035d9649eba4..a3bb4996074d 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -388,10 +388,10 @@ const struct dev_pm_ops name = { \
 #define EXPORT_PM_FN_NS_GPL(name, ns)
 #endif
 
-#define EXPORT_DEV_PM_OPS(name) _EXPORT_DEV_PM_OPS(name, "", "")
-#define EXPORT_GPL_DEV_PM_OPS(name) _EXPORT_DEV_PM_OPS(name, "_gpl", "")
-#define EXPORT_NS_DEV_PM_OPS(name, ns) _EXPORT_DEV_PM_OPS(name, "", #ns)
-#define EXPORT_NS_GPL_DEV_PM_OPS(name, ns) _EXPORT_DEV_PM_OPS(name, "_gpl", #ns)
+#define EXPORT_DEV_PM_OPS(name) _EXPORT_DEV_PM_OPS(name,, "")
+#define EXPORT_GPL_DEV_PM_OPS(name) _EXPORT_DEV_PM_OPS(name,_gpl, "")
+#define EXPORT_NS_DEV_PM_OPS(name, ns) _EXPORT_DEV_PM_OPS(name,, #ns)
+#define EXPORT_NS_GPL_DEV_PM_OPS(name, ns) _EXPORT_DEV_PM_OPS(name,_gpl, #ns)
 
 /*
  * Use this if you want to use the same suspend and resume callbacks for suspend
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index dc7b0160c480..c8b7b4dcf782 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -32,6 +32,18 @@
 /* Maximum number of characters written by module_flags() */
 #define MODULE_FLAGS_BUF_SIZE (TAINT_FLAGS_COUNT + 4)
 
+struct kernel_symbol {
+#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
+	int value_offset;
+	int name_offset;
+	int namespace_offset;
+#else
+	unsigned long value;
+	const char *name;
+	const char *namespace;
+#endif
+};
+
 extern struct mutex module_mutex;
 extern struct list_head modules;
 
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 9f94fc83f086..c26c04aea652 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -161,7 +161,7 @@ quiet_cmd_cc_o_c = CC $(quiet_modtag)  $@
 ifdef CONFIG_MODVERSIONS
 # When module versioning is enabled the following steps are executed:
 # o compile a <file>.o from <file>.c
-# o if <file>.o doesn't contain a __ksymtab version, i.e. does
+# o if <file>.o doesn't contain a __export_symbol*, i.e. does
 #   not export symbols, it's done.
 # o otherwise, we calculate symbol versions using the good old
 #   genksyms on the preprocessed source and dump them into the .cmd file.
@@ -169,7 +169,7 @@ ifdef CONFIG_MODVERSIONS
 #   be compiled and linked to the kernel and/or modules.
 
 gen_symversions =								\
-	if $(NM) $@ 2>/dev/null | grep -q __ksymtab; then			\
+	if $(NM) $@ 2>/dev/null | grep -q '__export_symbol.*\.'; then		\
 		$(call cmd_gensymtypes_$(1),$(KBUILD_SYMTYPES),$(@:.o=.symtypes)) \
 			>> $(dot-target).cmd;					\
 	fi
@@ -340,9 +340,7 @@ $(obj)/%.ll: $(src)/%.rs FORCE
 cmd_gensymtypes_S =                                                         \
    { echo "\#include <linux/kernel.h>" ;                                    \
      echo "\#include <asm/asm-prototypes.h>" ;                              \
-    $(CPP) $(a_flags) $< |                                                  \
-     grep "\<___EXPORT_SYMBOL\>" |                                          \
-     sed 's/.*___EXPORT_SYMBOL[[:space:]]*\([a-zA-Z0-9_]*\)[[:space:]]*,.*/EXPORT_SYMBOL(\1);/' ; } | \
+     $(NM) $@ | sed -n 's/.*__export_symbol.*\.\(.*\)/EXPORT_SYMBOL(\1);/p' ; } | \
     $(CPP) -D__GENKSYMS__ $(c_flags) -xc - | $(genksyms)
 
 quiet_cmd_cc_symtypes_S = SYM $(quiet_modtag) $@
diff --git a/scripts/check-local-export b/scripts/check-local-export
index f90b5a9c67b3..e54a1642c970 100755
--- a/scripts/check-local-export
+++ b/scripts/check-local-export
@@ -46,9 +46,9 @@ BEGIN {
 { symbol_types[$3]=$2 }
 
 # append the exported symbol to the array
-($3 ~ /^__ksymtab_/) {
+($3 ~ /^__export_symbol.*\..*/) {
 	export_symbols[i] = $3
-	sub(/^__ksymtab_/, "", export_symbols[i])
+	sub(/^__export_symbol.*\./, "", export_symbols[i])
 	i++
 }
 
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index e82299c90836..8cf6bc0ac050 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -215,6 +215,7 @@ struct symbol {
 	unsigned int crc;
 	bool crc_valid;
 	bool weak;
+	bool is_func;
 	bool is_gpl_only;	/* exported by EXPORT_SYMBOL_GPL */
 	char name[];
 };
@@ -531,6 +532,8 @@ static int parse_elf(struct elf_info *info, const char *filename)
 				fatal("%s has NOBITS .modinfo\n", filename);
 			info->modinfo = (void *)hdr + sechdrs[i].sh_offset;
 			info->modinfo_len = sechdrs[i].sh_size;
+		} else if (!strcmp(secname, ".export_symbol")) {
+			info->export_symbol_secndx = i;
 		}
 
 		if (sechdrs[i].sh_type == SHT_SYMTAB) {
@@ -653,18 +656,6 @@ static void handle_symbol(struct module *mod, struct elf_info *info,
 				   ELF_ST_BIND(sym->st_info) == STB_WEAK);
 		break;
 	default:
-		/* All exported symbols */
-		if (strstarts(symname, "__ksymtab_")) {
-			const char *name, *secname;
-
-			name = symname + strlen("__ksymtab_");
-			secname = sec_name(info, get_secindex(info, sym));
-
-			if (strstarts(secname, "___ksymtab_gpl+"))
-				sym_add_exported(name, mod, true);
-			else if (strstarts(secname, "___ksymtab+"))
-				sym_add_exported(name, mod, false);
-		}
 		if (strcmp(symname, "init_module") == 0)
 			mod->has_init = true;
 		if (strcmp(symname, "cleanup_module") == 0)
@@ -865,7 +856,6 @@ enum mismatch {
 	XXXEXIT_TO_SOME_EXIT,
 	ANY_INIT_TO_ANY_EXIT,
 	ANY_EXIT_TO_ANY_INIT,
-	EXPORT_TO_INIT_EXIT,
 	EXTABLE_TO_NON_TEXT,
 };
 
@@ -947,12 +937,6 @@ static const struct sectioncheck sectioncheck[] = {
 	.bad_tosec = { INIT_SECTIONS, NULL },
 	.mismatch = ANY_INIT_TO_ANY_EXIT,
 },
-/* Do not export init/exit functions or data */
-{
-	.fromsec = { "___ksymtab*", NULL },
-	.bad_tosec = { INIT_SECTIONS, EXIT_SECTIONS, NULL },
-	.mismatch = EXPORT_TO_INIT_EXIT,
-},
 {
 	.fromsec = { "__ex_table", NULL },
 	/* If you're adding any new black-listed sections in here, consider
@@ -1192,10 +1176,6 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 		warn("%s: section mismatch in reference: %s (section: %s) -> %s (section: %s)\n",
 		     modname, fromsym, fromsec, tosym, tosec);
 		break;
-	case EXPORT_TO_INIT_EXIT:
-		warn("%s: EXPORT_SYMBOL used for init/exit symbol: %s (section: %s)\n",
-		     modname, tosym, tosec);
-		break;
 	case EXTABLE_TO_NON_TEXT:
 		warn("%s(%s+0x%lx): Section mismatch in reference to the %s:%s\n",
 		     modname, fromsec, (long)faddr, tosec, tosym);
@@ -1223,14 +1203,70 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
 	}
 }
 
+static void check_export_symbol(struct module *mod, struct elf_info *elf,
+				Elf_Addr faddr, const char *secname,
+				Elf_Addr addr, Elf_Sym *sym)
+{
+	const char *label_name, *name, *prefix;
+	Elf_Sym *label;
+	struct symbol *s;
+	bool is_gpl;
+
+	label = find_nearest_symbol(elf, faddr, elf->export_symbol_secndx);
+	label_name = sym_name(elf, label);
+
+	sym = find_better_symbol(elf, addr, sym);
+	name = sym_name(elf, sym);
+
+	if (strstarts(label_name, "__export_symbol_gpl.")) {
+		prefix = "__export_symbol_gpl.";
+		is_gpl = true;
+	} else if (strstarts(label_name, "__export_symbol.")) {
+		prefix = "__export_symbol.";
+		is_gpl = false;
+	} else {
+		error(".export_symbol section contains strange symbol '%s'\n",
+		      label_name);
+		return;
+	}
+
+	if (strcmp(label_name + strlen(prefix), name)) {
+		error(".export_symbol section references '%s', but it does not seem to be an export symbol\n",
+		      name);
+		return;
+	}
+
+	s = sym_add_exported(name, mod, is_gpl);
+	sym_update_namespace(name, sym_get_data(elf, label));
+
+	/*
+	 * We need to be aware whether we are exporting a function or
+	 * a data on some architectures.
+	 */
+	s->is_func = (ELF_ST_TYPE(sym->st_info) == STT_FUNC);
+
+	if (match(secname, PATTERNS(INIT_SECTIONS)))
+		error("%s: EXPORT_SYMBOL used for init symbol. Remove __init or EXPORT_SYMBOL.\n",
+		      name);
+	else if (match(secname, PATTERNS(EXIT_SECTIONS)))
+		error("%s: EXPORT_SYMBOL used for exit symbol. Remove __exit or EXPORT_SYMBOL.\n",
+		      name);
+}
+
 static void check_section_mismatch(struct module *mod, struct elf_info *elf,
 				   Elf_Sym *sym,
 				   unsigned int fsecndx, const char *fromsec,
 				   Elf_Addr faddr, Elf_Addr taddr)
 {
 	const char *tosec = sec_name(elf, get_secindex(elf, sym));
-	const struct sectioncheck *mismatch = section_mismatch(fromsec, tosec);
+	const struct sectioncheck *mismatch;
 
+	if (elf->export_symbol_secndx == fsecndx) {
+		check_export_symbol(mod, elf, faddr, tosec, taddr, sym);
+		return;
+	}
+
+	mismatch = section_mismatch(fromsec, tosec);
 	if (!mismatch)
 		return;
 
@@ -1390,7 +1426,7 @@ static void section_rela(struct module *mod, struct elf_info *elf,
 			break;
 		}
 
-		check_section_mismatch(modname, elf, elf->symtab_start + r_sym,
+		check_section_mismatch(mod, elf, elf->symtab_start + r_sym,
 				       fsecndx, fromsec, r.r_offset, r.r_addend);
 	}
 }
@@ -1445,7 +1481,7 @@ static void section_rel(struct module *mod, struct elf_info *elf,
 			fatal("Please add code to calculate addend for this architecture\n");
 		}
 
-		check_section_mismatch(modname, elf, elf->symtab_start + r_sym,
+		check_section_mismatch(mod, elf, elf->symtab_start + r_sym,
 				       fsecndx, fromsec, r.r_offset, r.r_addend);
 	}
 }
@@ -1636,15 +1672,6 @@ static void read_symbols(const char *modname)
 		handle_moddevtable(mod, &info, sym, symname);
 	}
 
-	for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
-		symname = remove_dot(info.strtab + sym->st_name);
-
-		/* Apply symbol namespaces from __kstrtabns_<symbol> entries. */
-		if (strstarts(symname, "__kstrtabns_"))
-			sym_update_namespace(symname + strlen("__kstrtabns_"),
-					     sym_get_data(&info, sym));
-	}
-
 	check_sec_ref(mod, &info);
 
 	if (!mod->is_vmlinux) {
@@ -1828,6 +1855,14 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
 {
 	struct symbol *sym;
 
+	/* generate struct for exported symbols */
+	buf_printf(buf, "\n");
+	list_for_each_entry(sym, &mod->exported_symbols, list)
+		buf_printf(buf, "KSYMTAB_%s(%s, \"%s\", \"%s\");\n",
+			   sym->is_func ? "FUNC" : "DATA", sym->name,
+			   sym->is_gpl_only ? "_gpl" : "",
+			   sym->namespace ?: "");
+
 	if (!modversions)
 		return;
 
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 1178f40a73f3..f518fbe46541 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -137,6 +137,7 @@ struct elf_info {
 	Elf_Shdr     *sechdrs;
 	Elf_Sym      *symtab_start;
 	Elf_Sym      *symtab_stop;
+	unsigned int export_symbol_secndx;	/* .export_symbol section */
 	char         *strtab;
 	char	     *modinfo;
 	unsigned int modinfo_len;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 17/21] ia64,export.h: replace EXPORT_DATA_SYMBOL* with EXPORT_SYMBOL*
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (15 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 16/21] kbuild: generate KSYMTAB entries by modpost Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:44 ` [PATCH v4 18/21] modpost: check static EXPORT_SYMBOL* by modpost again Masahiro Yamada
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

With the previous refactoring, you can always use EXPORT_SYMBOL*.

Replace two instances in ia64, then remove EXPORT_DATA_SYMBOL*.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/ia64/kernel/head.S      | 2 +-
 arch/ia64/kernel/ivt.S       | 2 +-
 include/asm-generic/export.h | 3 ---
 3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/ia64/kernel/head.S b/arch/ia64/kernel/head.S
index f22469f1c1fc..c096500590e9 100644
--- a/arch/ia64/kernel/head.S
+++ b/arch/ia64/kernel/head.S
@@ -170,7 +170,7 @@ RestRR:											\
 	__PAGE_ALIGNED_DATA
 
 	.global empty_zero_page
-EXPORT_DATA_SYMBOL_GPL(empty_zero_page)
+EXPORT_SYMBOL_GPL(empty_zero_page)
 empty_zero_page:
 	.skip PAGE_SIZE
 
diff --git a/arch/ia64/kernel/ivt.S b/arch/ia64/kernel/ivt.S
index d6d4229b28db..7a418e324d30 100644
--- a/arch/ia64/kernel/ivt.S
+++ b/arch/ia64/kernel/ivt.S
@@ -87,7 +87,7 @@
 
 	.align 32768	// align on 32KB boundary
 	.global ia64_ivt
-	EXPORT_DATA_SYMBOL(ia64_ivt)
+	EXPORT_SYMBOL(ia64_ivt)
 ia64_ivt:
 /////////////////////////////////////////////////////////////////////////////////////////
 // 0x0000 Entry 0 (size 64 bundles) VHPT Translation (8,20,47)
diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
index 0ae9f38a904c..570cd4da7210 100644
--- a/include/asm-generic/export.h
+++ b/include/asm-generic/export.h
@@ -8,7 +8,4 @@
  */
 #include <linux/export.h>
 
-#define EXPORT_DATA_SYMBOL(name)	EXPORT_SYMBOL(name)
-#define EXPORT_DATA_SYMBOL_GPL(name)	EXPORT_SYMBOL_GPL(name)
-
 #endif
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 18/21] modpost: check static EXPORT_SYMBOL* by modpost again
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (16 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 17/21] ia64,export.h: replace EXPORT_DATA_SYMBOL* with EXPORT_SYMBOL* Masahiro Yamada
@ 2023-05-13 20:44 ` Masahiro Yamada
  2023-05-13 20:45 ` [PATCH v4 19/21] modpost: squash sym_update_namespace() into sym_add_exported() Masahiro Yamada
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

Commit 31cb50b5590f ("kbuild: check static EXPORT_SYMBOL* by script
instead of modpost") moved the static EXPORT_SYMBOL* check from the
mostpost to a shell script because I thought it must be checked per
compilation unit to avoid false negatives.

I came up with an idea to do this in modpost, against combined ELF
files. The relocation entries in ELF will find the correct exported
symbol even if there exist symbols with the same name in different
compilation units.

Again, the same sample code.

  Makefile:

    obj-y += foo1.o foo2.o

  foo1.c:

    #include <linux/export.h>
    static void foo(void) {}
    EXPORT_SYMBOL(foo);

  foo2.c:

    void foo(void) {}

Then, modpost can catch it correctly.

    MODPOST Module.symvers
  ERROR: modpost: vmlinux: local symbol 'foo' was exported

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.build     |  4 ---
 scripts/check-local-export | 70 --------------------------------------
 scripts/mod/modpost.c      |  6 ++++
 3 files changed, 6 insertions(+), 74 deletions(-)
 delete mode 100755 scripts/check-local-export

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index c26c04aea652..94b3323dcea7 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -220,8 +220,6 @@ cmd_gen_ksymdeps = \
 	$(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
 endif
 
-cmd_check_local_export = $(srctree)/scripts/check-local-export $@
-
 ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)
 cmd_warn_shared_object = $(if $(word 2, $(modname-multi)),$(warning $(kbuild-file): $*.o is added to multiple modules: $(modname-multi)))
 endif
@@ -229,7 +227,6 @@ endif
 define rule_cc_o_c
 	$(call cmd_and_fixdep,cc_o_c)
 	$(call cmd,gen_ksymdeps)
-	$(call cmd,check_local_export)
 	$(call cmd,checksrc)
 	$(call cmd,checkdoc)
 	$(call cmd,gen_objtooldep)
@@ -241,7 +238,6 @@ endef
 define rule_as_o_S
 	$(call cmd_and_fixdep,as_o_S)
 	$(call cmd,gen_ksymdeps)
-	$(call cmd,check_local_export)
 	$(call cmd,gen_objtooldep)
 	$(call cmd,gen_symversions_S)
 	$(call cmd,warn_shared_object)
diff --git a/scripts/check-local-export b/scripts/check-local-export
deleted file mode 100755
index e54a1642c970..000000000000
--- a/scripts/check-local-export
+++ /dev/null
@@ -1,70 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0-only
-#
-# Copyright (C) 2022 Masahiro Yamada <masahiroy@kernel.org>
-# Copyright (C) 2022 Owen Rafferty <owen@owenrafferty.com>
-#
-# Exit with error if a local exported symbol is found.
-# EXPORT_SYMBOL should be used for global symbols.
-
-set -e
-pid=$$
-
-# If there is no symbol in the object, ${NM} (both GNU nm and llvm-nm) shows
-# 'no symbols' diagnostic (but exits with 0). It is harmless and hidden by
-# '2>/dev/null'. However, it suppresses real error messages as well. Add a
-# hand-crafted error message here.
-#
-# TODO:
-# Use --quiet instead of 2>/dev/null when we upgrade the minimum version of
-# binutils to 2.37, llvm to 13.0.0.
-# Then, the following line will be simpler:
-#   { ${NM} --quiet ${1} || kill 0; } |
-
-{ ${NM} ${1} 2>/dev/null || { echo "${0}: ${NM} failed" >&2; kill $pid; } } |
-${AWK} -v "file=${1}" '
-BEGIN {
-	i = 0
-}
-
-# Skip the line if the number of fields is less than 3.
-#
-# case 1)
-#   For undefined symbols, the first field (value) is empty.
-#   The outout looks like this:
-#     "                 U _printk"
-#   It is unneeded to record undefined symbols.
-#
-# case 2)
-#   For Clang LTO, llvm-nm outputs a line with type t but empty name:
-#     "---------------- t"
-!length($3) {
-	next
-}
-
-# save (name, type) in the associative array
-{ symbol_types[$3]=$2 }
-
-# append the exported symbol to the array
-($3 ~ /^__export_symbol.*\..*/) {
-	export_symbols[i] = $3
-	sub(/^__export_symbol.*\./, "", export_symbols[i])
-	i++
-}
-
-END {
-	exit_code = 0
-	for (j = 0; j < i; ++j) {
-		name = export_symbols[j]
-		# nm(3) says "If lowercase, the symbol is usually local"
-		if (symbol_types[name] ~ /[a-z]/) {
-			printf "%s: error: local symbol %s was exported\n",
-				file, name | "cat 1>&2"
-			exit_code = 1
-		}
-	}
-
-	exit exit_code
-}'
-
-exit $?
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 8cf6bc0ac050..3e956013a6c8 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1236,6 +1236,12 @@ static void check_export_symbol(struct module *mod, struct elf_info *elf,
 		return;
 	}
 
+	if (ELF_ST_BIND(sym->st_info) != STB_GLOBAL &&
+	    ELF_ST_BIND(sym->st_info) != STB_WEAK) {
+		error("%s: local symbol '%s' was exported\n", mod->name, name);
+		return;
+	}
+
 	s = sym_add_exported(name, mod, is_gpl);
 	sym_update_namespace(name, sym_get_data(elf, label));
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 19/21] modpost: squash sym_update_namespace() into sym_add_exported()
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (17 preceding siblings ...)
  2023-05-13 20:44 ` [PATCH v4 18/21] modpost: check static EXPORT_SYMBOL* by modpost again Masahiro Yamada
@ 2023-05-13 20:45 ` Masahiro Yamada
  2023-05-13 20:45 ` [PATCH v4 20/21] modpost: use null string instead of NULL pointer for default namespace Masahiro Yamada
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:45 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

Pass a set of the name, license, and namespace to sym_add_exported().

sym_update_namespace() is unneeded.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---

 scripts/mod/modpost.c | 27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 3e956013a6c8..7de6548103d9 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -353,26 +353,8 @@ static const char *sec_name(const struct elf_info *info, unsigned int secindex)
 
 #define strstarts(str, prefix) (strncmp(str, prefix, strlen(prefix)) == 0)
 
-static void sym_update_namespace(const char *symname, const char *namespace)
-{
-	struct symbol *s = find_symbol(symname);
-
-	/*
-	 * That symbol should have been created earlier and thus this is
-	 * actually an assertion.
-	 */
-	if (!s) {
-		error("Could not update namespace(%s) for symbol %s\n",
-		      namespace, symname);
-		return;
-	}
-
-	free(s->namespace);
-	s->namespace = namespace[0] ? NOFAIL(strdup(namespace)) : NULL;
-}
-
 static struct symbol *sym_add_exported(const char *name, struct module *mod,
-				       bool gpl_only)
+				       bool gpl_only, const char *namespace)
 {
 	struct symbol *s = find_symbol(name);
 
@@ -385,6 +367,7 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
 	s = alloc_symbol(name);
 	s->module = mod;
 	s->is_gpl_only = gpl_only;
+	s->namespace = namespace[0] ? NOFAIL(strdup(namespace)) : NULL;
 	list_add_tail(&s->list, &mod->exported_symbols);
 	hash_add_symbol(s);
 
@@ -1242,8 +1225,7 @@ static void check_export_symbol(struct module *mod, struct elf_info *elf,
 		return;
 	}
 
-	s = sym_add_exported(name, mod, is_gpl);
-	sym_update_namespace(name, sym_get_data(elf, label));
+	s = sym_add_exported(name, mod, is_gpl, sym_get_data(elf, label));
 
 	/*
 	 * We need to be aware whether we are exporting a function or
@@ -2112,9 +2094,8 @@ static void read_dump(const char *fname)
 			mod = new_module(modname, strlen(modname));
 			mod->from_dump = true;
 		}
-		s = sym_add_exported(symname, mod, gpl_only);
+		s = sym_add_exported(symname, mod, gpl_only, namespace);
 		sym_set_crc(s, crc);
-		sym_update_namespace(symname, namespace);
 	}
 	free(buf);
 	return;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 20/21] modpost: use null string instead of NULL pointer for default namespace
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (18 preceding siblings ...)
  2023-05-13 20:45 ` [PATCH v4 19/21] modpost: squash sym_update_namespace() into sym_add_exported() Masahiro Yamada
@ 2023-05-13 20:45 ` Masahiro Yamada
  2023-05-13 20:45 ` [PATCH v4 21/21] kbuild: implement CONFIG_TRIM_UNUSED_KSYMS without recursion Masahiro Yamada
  2023-05-14 12:43 ` [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:45 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

The default namespace is the null string, "".

When set, the null string "" is converted to NULL:

  s->namespace = namespace[0] ? NOFAIL(strdup(namespace)) : NULL;

When printed, the NULL pointer is get back to the null string:

  sym->namespace ?: ""

This saves 1 byte memory allocated for "", but loses the readability.

In kernel-space, we strive to save memory, but modpost is a userspace
tool used to build the kernel. On modern systems, such small piece of
memory is not a big deal.

Handle the namespace string as is.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---

 scripts/mod/modpost.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 7de6548103d9..bb9510f9645f 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -298,6 +298,13 @@ static bool contains_namespace(struct list_head *head, const char *namespace)
 {
 	struct namespace_list *list;
 
+	/*
+	 * The default namespace is null string "", which is always implicitly
+	 * contained.
+	 */
+	if (!namespace[0])
+		return true;
+
 	list_for_each_entry(list, head, list) {
 		if (!strcmp(list->namespace, namespace))
 			return true;
@@ -367,7 +374,7 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
 	s = alloc_symbol(name);
 	s->module = mod;
 	s->is_gpl_only = gpl_only;
-	s->namespace = namespace[0] ? NOFAIL(strdup(namespace)) : NULL;
+	s->namespace = NOFAIL(strdup(namespace));
 	list_add_tail(&s->list, &mod->exported_symbols);
 	hash_add_symbol(s);
 
@@ -1761,8 +1768,7 @@ static void check_exports(struct module *mod)
 		else
 			basename = mod->name;
 
-		if (exp->namespace &&
-		    !contains_namespace(&mod->imported_namespaces, exp->namespace)) {
+		if (!contains_namespace(&mod->imported_namespaces, exp->namespace)) {
 			modpost_log(allow_missing_ns_imports ? LOG_WARN : LOG_ERROR,
 				    "module %s uses symbol %s from namespace %s, but does not import it.\n",
 				    basename, exp->name, exp->namespace);
@@ -1848,8 +1854,7 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
 	list_for_each_entry(sym, &mod->exported_symbols, list)
 		buf_printf(buf, "KSYMTAB_%s(%s, \"%s\", \"%s\");\n",
 			   sym->is_func ? "FUNC" : "DATA", sym->name,
-			   sym->is_gpl_only ? "_gpl" : "",
-			   sym->namespace ?: "");
+			   sym->is_gpl_only ? "_gpl" : "", sym->namespace);
 
 	if (!modversions)
 		return;
@@ -2117,7 +2122,7 @@ static void write_dump(const char *fname)
 			buf_printf(&buf, "0x%08x\t%s\t%s\tEXPORT_SYMBOL%s\t%s\n",
 				   sym->crc, sym->name, mod->name,
 				   sym->is_gpl_only ? "_GPL" : "",
-				   sym->namespace ?: "");
+				   sym->namespace);
 		}
 	}
 	write_buf(&buf, fname);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v4 21/21] kbuild: implement CONFIG_TRIM_UNUSED_KSYMS without recursion
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (19 preceding siblings ...)
  2023-05-13 20:45 ` [PATCH v4 20/21] modpost: use null string instead of NULL pointer for default namespace Masahiro Yamada
@ 2023-05-13 20:45 ` Masahiro Yamada
  2023-05-14 12:43 ` [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-13 20:45 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier, Masahiro Yamada

When CONFIG_TRIM_UNUSED_KSYMS is enabled, Kbuild recursively traverses
the directory tree to determine which EXPORT_SYMBOL to trim. If an
EXPORT_SYMBOL turns out to be unused by anyone, Kbuild begins the
second traverse, where some source files are recompiled with their
EXPORT_SYMBOL() tuned into a no-op.

Linus stated negative opinions about this slowness in commits:

 - 5cf0fd591f2e ("Kbuild: disable TRIM_UNUSED_KSYMS option")
 - a555bdd0c58c ("Kbuild: enable TRIM_UNUSED_KSYMS again, with some guarding")

We can do this better now. The final data structures of EXPORT_SYMBOL
are generated by the modpost stage, so modpost can selectively emit
KSYMTAB entries that are really used by modules.

Commit 2cce989f8461 ("kbuild: unify two modpost invocations") is another
ground-work to do this in a one-pass algorithm. With the list of modules,
modpost sets sym->used if it is used by a module. modpost emits KSYMTAB
only for symbols with sym->used==true.

BTW, Nicolas explained why the trimming was implemented with recursion:

  https://lore.kernel.org/all/2o2rpn97-79nq-p7s2-nq5-8p83391473r@syhkavp.arg/

Actually, we never achieved that level of optimization where the chain
reaction of trimming comes into play because:

 - CONFIG_LTO_CLANG cannot remove any unused symbols
 - CONFIG_LD_DEAD_CODE_DATA_ELIMINATION is enabled only for vmlinux,
   but not modules

If deeper trimming is required, we need to revisit this, but I guess
that is unlikely to happen.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 .gitignore                  |  1 -
 Makefile                    | 19 +---------
 include/linux/export.h      | 41 ---------------------
 scripts/Makefile.build      |  7 ----
 scripts/Makefile.modpost    |  7 ++++
 scripts/adjust_autoksyms.sh | 73 -------------------------------------
 scripts/basic/fixdep.c      |  3 +-
 scripts/gen_ksymdeps.sh     | 30 ---------------
 scripts/mod/modpost.c       | 54 ++++++++++++++++++++++++---
 scripts/remove-stale-files  |  2 +
 10 files changed, 61 insertions(+), 176 deletions(-)
 delete mode 100755 scripts/adjust_autoksyms.sh
 delete mode 100755 scripts/gen_ksymdeps.sh

diff --git a/.gitignore b/.gitignore
index 7f86e0837909..172e3874adfd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -112,7 +112,6 @@ modules.order
 #
 /include/config/
 /include/generated/
-/include/ksym/
 /arch/*/include/generated/
 
 # stgit generated dirs
diff --git a/Makefile b/Makefile
index 9d765ebcccf1..2a081f2e9911 100644
--- a/Makefile
+++ b/Makefile
@@ -1193,28 +1193,13 @@ endif
 export KBUILD_VMLINUX_LIBS
 export KBUILD_LDS          := arch/$(SRCARCH)/kernel/vmlinux.lds
 
-# Recurse until adjust_autoksyms.sh is satisfied
-PHONY += autoksyms_recursive
 ifdef CONFIG_TRIM_UNUSED_KSYMS
 # For the kernel to actually contain only the needed exported symbols,
 # we have to build modules as well to determine what those symbols are.
 # (this can be evaluated only once include/config/auto.conf has been included)
 KBUILD_MODULES := 1
-
-autoksyms_recursive: $(build-dir) modules.order
-	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/adjust_autoksyms.sh \
-	  "$(MAKE) -f $(srctree)/Makefile autoksyms_recursive"
 endif
 
-autoksyms_h := $(if $(CONFIG_TRIM_UNUSED_KSYMS), include/generated/autoksyms.h)
-
-quiet_cmd_autoksyms_h = GEN     $@
-      cmd_autoksyms_h = mkdir -p $(dir $@); \
-			$(CONFIG_SHELL) $(srctree)/scripts/gen_autoksyms.sh $@
-
-$(autoksyms_h):
-	$(call cmd,autoksyms_h)
-
 # '$(AR) mPi' needs 'T' to workaround the bug of llvm-ar <= 14
 quiet_cmd_ar_vmlinux.a = AR      $@
       cmd_ar_vmlinux.a = \
@@ -1223,7 +1208,7 @@ quiet_cmd_ar_vmlinux.a = AR      $@
 	$(AR) mPiT $$($(AR) t $@ | sed -n 1p) $@ $$($(AR) t $@ | grep -F -f $(srctree)/scripts/head-object-list.txt)
 
 targets += vmlinux.a
-vmlinux.a: $(KBUILD_VMLINUX_OBJS) scripts/head-object-list.txt autoksyms_recursive FORCE
+vmlinux.a: $(KBUILD_VMLINUX_OBJS) scripts/head-object-list.txt FORCE
 	$(call if_changed,ar_vmlinux.a)
 
 PHONY += vmlinux_o
@@ -1279,7 +1264,7 @@ scripts: scripts_basic scripts_dtc
 PHONY += prepare archprepare
 
 archprepare: outputmakefile archheaders archscripts scripts include/config/kernel.release \
-	asm-generic $(version_h) $(autoksyms_h) include/generated/utsrelease.h \
+	asm-generic $(version_h) include/generated/utsrelease.h \
 	include/generated/compile.h include/generated/autoconf.h remove-stale-files
 
 prepare0: archprepare
diff --git a/include/linux/export.h b/include/linux/export.h
index 4335a17a1ea7..0b9f85628fba 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -65,47 +65,6 @@ extern struct module __this_module;
  */
 #define __EXPORT_SYMBOL(sym, sec, ns)
 
-#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
-
-#include <generated/autoksyms.h>
-
-/*
- * For fine grained build dependencies, we want to tell the build system
- * about each possible exported symbol even if they're not actually exported.
- * We use a symbol pattern __ksym_marker_<symbol> that the build system filters
- * from the $(NM) output (see scripts/gen_ksymdeps.sh). These symbols are
- * discarded in the final link stage.
- */
-
-#ifdef __ASSEMBLY__
-
-#define __ksym_marker(sym)					\
-	.section ".discard.ksym","a" ;				\
-__ksym_marker_##sym: ;						\
-	.previous
-
-#else
-
-#define __ksym_marker(sym)	\
-	static int __ksym_marker_##sym[0] __section(".discard.ksym") __used
-
-#endif
-
-#define __EXPORT_SYMBOL(sym, sec, ns)					\
-	__ksym_marker(sym);						\
-	__cond_export_sym(sym, sec, ns, __is_defined(__KSYM_##sym))
-#define __cond_export_sym(sym, sec, ns, conf)				\
-	___cond_export_sym(sym, sec, ns, conf)
-#define ___cond_export_sym(sym, sec, ns, enabled)			\
-	__cond_export_sym_##enabled(sym, sec, ns)
-#define __cond_export_sym_1(sym, sec, ns) ___EXPORT_SYMBOL(sym, sec, ns)
-
-#ifdef __GENKSYMS__
-#define __cond_export_sym_0(sym, sec, ns) __GENKSYMS_EXPORT_SYMBOL(sym)
-#else
-#define __cond_export_sym_0(sym, sec, ns) /* nothing */
-#endif
-
 #else
 
 #define __EXPORT_SYMBOL(sym, sec, ns)	___EXPORT_SYMBOL(sym, sec, ns)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 94b3323dcea7..dc0b27d1a500 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -215,18 +215,12 @@ is-standard-object = $(if $(filter-out y%, $(OBJECT_FILES_NON_STANDARD_$(basetar
 
 $(obj)/%.o: objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y))
 
-ifdef CONFIG_TRIM_UNUSED_KSYMS
-cmd_gen_ksymdeps = \
-	$(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
-endif
-
 ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)
 cmd_warn_shared_object = $(if $(word 2, $(modname-multi)),$(warning $(kbuild-file): $*.o is added to multiple modules: $(modname-multi)))
 endif
 
 define rule_cc_o_c
 	$(call cmd_and_fixdep,cc_o_c)
-	$(call cmd,gen_ksymdeps)
 	$(call cmd,checksrc)
 	$(call cmd,checkdoc)
 	$(call cmd,gen_objtooldep)
@@ -237,7 +231,6 @@ endef
 
 define rule_as_o_S
 	$(call cmd_and_fixdep,as_o_S)
-	$(call cmd,gen_ksymdeps)
 	$(call cmd,gen_objtooldep)
 	$(call cmd,gen_symversions_S)
 	$(call cmd,warn_shared_object)
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 0980c58d8afc..1e0b47cbabd9 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -90,6 +90,13 @@ targets += .vmlinux.objs
 .vmlinux.objs: vmlinux.a $(KBUILD_VMLINUX_LIBS) FORCE
 	$(call if_changed,vmlinux_objs)
 
+ifdef CONFIG_TRIM_UNUSED_KSYMS
+ksym-wl := $(CONFIG_UNUSED_KSYMS_WHITELIST)
+ksym-wl := $(if $(filter-out /%, $(ksym-wl)),$(srctree)/)$(ksym-wl)
+modpost-args += -t $(addprefix -W, $(ksym-wl))
+modpost-deps += $(ksym-wl)
+endif
+
 ifeq ($(wildcard vmlinux.o),)
 missing-input := vmlinux.o
 output-symdump := modules-only.symvers
diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
deleted file mode 100755
index f1b5ac818411..000000000000
--- a/scripts/adjust_autoksyms.sh
+++ /dev/null
@@ -1,73 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0-only
-
-# Script to update include/generated/autoksyms.h and dependency files
-#
-# Copyright:	(C) 2016  Linaro Limited
-# Created by:	Nicolas Pitre, January 2016
-#
-
-# Update the include/generated/autoksyms.h file.
-#
-# For each symbol being added or removed, the corresponding dependency
-# file's timestamp is updated to force a rebuild of the affected source
-# file. All arguments passed to this script are assumed to be a command
-# to be exec'd to trigger a rebuild of those files.
-
-set -e
-
-cur_ksyms_file="include/generated/autoksyms.h"
-new_ksyms_file="include/generated/autoksyms.h.tmpnew"
-
-info() {
-	if [ "$quiet" != "silent_" ]; then
-		printf "  %-7s %s\n" "$1" "$2"
-	fi
-}
-
-info "CHK" "$cur_ksyms_file"
-
-# Use "make V=1" to debug this script.
-case "$KBUILD_VERBOSE" in
-*1*)
-	set -x
-	;;
-esac
-
-# Generate a new symbol list file
-$CONFIG_SHELL $srctree/scripts/gen_autoksyms.sh --modorder "$new_ksyms_file"
-
-# Extract changes between old and new list and touch corresponding
-# dependency files.
-changed=$(
-count=0
-sort "$cur_ksyms_file" "$new_ksyms_file" | uniq -u |
-sed -n 's/^#define __KSYM_\(.*\) 1/\1/p' |
-while read sympath; do
-	if [ -z "$sympath" ]; then continue; fi
-	depfile="include/ksym/${sympath}"
-	mkdir -p "$(dirname "$depfile")"
-	touch "$depfile"
-	# Filesystems with coarse time precision may create timestamps
-	# equal to the one from a file that was very recently built and that
-	# needs to be rebuild. Let's guard against that by making sure our
-	# dep files are always newer than the first file we created here.
-	while [ ! "$depfile" -nt "$new_ksyms_file" ]; do
-		touch "$depfile"
-	done
-	echo $((count += 1))
-done | tail -1 )
-changed=${changed:-0}
-
-if [ $changed -gt 0 ]; then
-	# Replace the old list with tne new one
-	old=$(grep -c "^#define __KSYM_" "$cur_ksyms_file" || true)
-	new=$(grep -c "^#define __KSYM_" "$new_ksyms_file" || true)
-	info "KSYMS" "symbols: before=$old, after=$new, changed=$changed"
-	info "UPD" "$cur_ksyms_file"
-	mv -f "$new_ksyms_file" "$cur_ksyms_file"
-	# Then trigger a rebuild of affected source files
-	exec $@
-else
-	rm -f "$new_ksyms_file"
-fi
diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
index fa562806c2be..84b6efa849f4 100644
--- a/scripts/basic/fixdep.c
+++ b/scripts/basic/fixdep.c
@@ -246,8 +246,7 @@ static void *read_file(const char *filename)
 /* Ignore certain dependencies */
 static int is_ignored_file(const char *s, int len)
 {
-	return str_ends_with(s, len, "include/generated/autoconf.h") ||
-	       str_ends_with(s, len, "include/generated/autoksyms.h");
+	return str_ends_with(s, len, "include/generated/autoconf.h");
 }
 
 /* Do not parse these files */
diff --git a/scripts/gen_ksymdeps.sh b/scripts/gen_ksymdeps.sh
deleted file mode 100755
index 8ee533f33659..000000000000
--- a/scripts/gen_ksymdeps.sh
+++ /dev/null
@@ -1,30 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0
-
-set -e
-
-# List of exported symbols
-#
-# If the object has no symbol, $NM warns 'no symbols'.
-# Suppress the stderr.
-# TODO:
-#   Use -q instead of 2>/dev/null when we upgrade the minimum version of
-#   binutils to 2.37, llvm to 13.0.0.
-ksyms=$($NM $1 2>/dev/null | sed -n 's/.*__ksym_marker_\(.*\)/\1/p')
-
-if [ -z "$ksyms" ]; then
-	exit 0
-fi
-
-echo
-echo "ksymdeps_$1 := \\"
-
-for s in $ksyms
-do
-	printf '    $(wildcard include/ksym/%s) \\\n' "$s"
-done
-
-echo
-echo "$1: \$(ksymdeps_$1)"
-echo
-echo "\$(ksymdeps_$1):"
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index bb9510f9645f..4d77b83b1338 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -35,6 +35,9 @@ static bool warn_unresolved;
 
 static int sec_mismatch_count;
 static bool sec_mismatch_warn_only = true;
+/* Trim EXPORT_SYMBOLs that are unused by in-tree modules */
+static bool trim_unused_exports;
+
 /* ignore missing files */
 static bool ignore_missing_files;
 /* If set to 1, only warn (instead of error) about missing ns imports */
@@ -217,6 +220,7 @@ struct symbol {
 	bool weak;
 	bool is_func;
 	bool is_gpl_only;	/* exported by EXPORT_SYMBOL_GPL */
+	bool used;		/* there exists a user of this symbol */
 	char name[];
 };
 
@@ -1758,6 +1762,7 @@ static void check_exports(struct module *mod)
 			continue;
 		}
 
+		exp->used = true;
 		s->module = exp->module;
 		s->crc_valid = exp->crc_valid;
 		s->crc = exp->crc;
@@ -1781,6 +1786,23 @@ static void check_exports(struct module *mod)
 	}
 }
 
+static void handle_white_list_exports(const char *white_list)
+{
+	char *buf, *p, *name;
+
+	buf = read_text_file(white_list);
+	p = buf;
+
+	while ((name = strsep(&p, "\n"))) {
+		struct symbol *sym = find_symbol(name);
+
+		if (sym)
+			sym->used = true;
+	}
+
+	free(buf);
+}
+
 static void check_modname_len(struct module *mod)
 {
 	const char *mod_name;
@@ -1851,10 +1873,14 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
 
 	/* generate struct for exported symbols */
 	buf_printf(buf, "\n");
-	list_for_each_entry(sym, &mod->exported_symbols, list)
+	list_for_each_entry(sym, &mod->exported_symbols, list) {
+		if (trim_unused_exports && !sym->used)
+			continue;
+
 		buf_printf(buf, "KSYMTAB_%s(%s, \"%s\", \"%s\");\n",
 			   sym->is_func ? "FUNC" : "DATA", sym->name,
 			   sym->is_gpl_only ? "_gpl" : "", sym->namespace);
+	}
 
 	if (!modversions)
 		return;
@@ -1862,6 +1888,9 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
 	/* record CRCs for exported symbols */
 	buf_printf(buf, "\n");
 	list_for_each_entry(sym, &mod->exported_symbols, list) {
+		if (trim_unused_exports && !sym->used)
+			continue;
+
 		if (!sym->crc_valid)
 			warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n"
 			     "Is \"%s\" prototyped in <asm/asm-prototypes.h>?\n",
@@ -2025,9 +2054,6 @@ static void write_mod_c_file(struct module *mod)
 	char fname[PATH_MAX];
 	int ret;
 
-	check_modname_len(mod);
-	check_exports(mod);
-
 	add_header(&buf, mod);
 	add_exported_symbols(&buf, mod);
 	add_versions(&buf, mod);
@@ -2161,12 +2187,13 @@ int main(int argc, char **argv)
 {
 	struct module *mod;
 	char *missing_namespace_deps = NULL;
+	char *unused_exports_white_list = NULL;
 	char *dump_write = NULL, *files_source = NULL;
 	int opt;
 	LIST_HEAD(dump_lists);
 	struct dump_list *dl, *dl2;
 
-	while ((opt = getopt(argc, argv, "ei:mnT:o:awENd:")) != -1) {
+	while ((opt = getopt(argc, argv, "ei:mntT:tW:o:awENd:")) != -1) {
 		switch (opt) {
 		case 'e':
 			external_module = true;
@@ -2191,6 +2218,12 @@ int main(int argc, char **argv)
 		case 'T':
 			files_source = optarg;
 			break;
+		case 't':
+			trim_unused_exports = true;
+			break;
+		case 'W':
+			unused_exports_white_list = optarg;
+			break;
 		case 'w':
 			warn_unresolved = true;
 			break;
@@ -2220,6 +2253,17 @@ int main(int argc, char **argv)
 	if (files_source)
 		read_symbols_from_files(files_source);
 
+	list_for_each_entry(mod, &modules, list) {
+		if (mod->from_dump || mod->is_vmlinux)
+			continue;
+
+		check_modname_len(mod);
+		check_exports(mod);
+	}
+
+	if (unused_exports_white_list)
+		handle_white_list_exports(unused_exports_white_list);
+
 	list_for_each_entry(mod, &modules, list) {
 		if (mod->from_dump)
 			continue;
diff --git a/scripts/remove-stale-files b/scripts/remove-stale-files
index 7f432900671a..8502a17d47df 100755
--- a/scripts/remove-stale-files
+++ b/scripts/remove-stale-files
@@ -33,3 +33,5 @@ rm -f rust/target.json
 rm -f scripts/bin2c
 
 rm -f .scmversion
+
+rm -rf include/ksym
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS
  2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (20 preceding siblings ...)
  2023-05-13 20:45 ` [PATCH v4 21/21] kbuild: implement CONFIG_TRIM_UNUSED_KSYMS without recursion Masahiro Yamada
@ 2023-05-14 12:43 ` Masahiro Yamada
  21 siblings, 0 replies; 23+ messages in thread
From: Masahiro Yamada @ 2023-05-14 12:43 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Pitre,
	Nicolas Schier

On Sun, May 14, 2023 at 5:45 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
>
> This patch set refactors modpost first to make it easier to
> add new code.
>
> Main goals:
>
>  - Refactors EXPORT_SYMBOL, <linux/export.h> and <asm/export.h>.
>    You can still put EXPORT_SYMBOL() in *.S file, very close to the definition,
>    but you do not need to care about whether it is a function or a data.
>    This removes EXPORT_DATA_SYMBOL().
>
>  - Re-implement TRIM_UNUSED_KSYMS in one-pass.
>    This makes the building faster.
>
>  - Move the static EXPORT_SYMBOL check to modpost.
>    This also makes the building faster.
>
> Previous version
> v3: https://lore.kernel.org/all/20220928063947.299333-1-masahiroy@kernel.org/
>



After more testing, I noticed some issues.





- modpost: simplify find_elf_symbol()
  has a regression.
  I will get a wrong name for 'static' symbols.
  I will fix it or drop it.

- modpost: pass struct module pointer to check_section_mismatch()
  causes a build error, (but fixed by the next commit)

- kbuild: generate KSYMTAB entries by modpost
  This causes build errors on ARM.
  I will fix it.


I will send v5.



>
> Masahiro Yamada (21):
>   modpost: remove broken calculation of exception_table_entry size
>   modpost: remove fromsym info in __ex_table section mismatch warning
>   modpost: remove get_prettyname()
>   modpost: squash report_extable_warnings() into
>     extable_mismatch_handler()
>   modpost: squash report_sec_mismatch() into default_mismatch_handler()
>   modpost: clean up is_executable_section()
>   modpost: squash extable_mismatch_handler() into
>     default_mismatch_handler()
>   modpost: pass 'tosec' down to default_mismatch_handler()
>   modpost: pass section index to find_elf_symbol2()
>   modpost: simplify find_elf_symbol()
>   modpost: rename find_elf_symbol() and find_elf_symbol2()
>   modpost: unify 'sym' and 'to' in default_mismatch_handler()
>   modpost: replace r->r_offset, r->r_addend with faddr, taddr
>   modpost: remove is_shndx_special() check from section_rel(a)
>   modpost: pass struct module pointer to check_section_mismatch()
>   kbuild: generate KSYMTAB entries by modpost
>   ia64,export.h: replace EXPORT_DATA_SYMBOL* with EXPORT_SYMBOL*
>   modpost: check static EXPORT_SYMBOL* by modpost again
>   modpost: squash sym_update_namespace() into sym_add_exported()
>   modpost: use null string instead of NULL pointer for default namespace
>   kbuild: implement CONFIG_TRIM_UNUSED_KSYMS without recursion
>
>  .gitignore                        |   1 -
>  Makefile                          |  19 +-
>  arch/ia64/include/asm/Kbuild      |   1 +
>  arch/ia64/include/asm/export.h    |   3 -
>  arch/ia64/kernel/head.S           |   2 +-
>  arch/ia64/kernel/ivt.S            |   2 +-
>  include/asm-generic/export.h      |  83 +----
>  include/asm-generic/vmlinux.lds.h |   1 +
>  include/linux/export-internal.h   |  49 +++
>  include/linux/export.h            | 116 ++-----
>  include/linux/pm.h                |   8 +-
>  kernel/module/internal.h          |  12 +
>  scripts/Makefile.build            |  19 +-
>  scripts/Makefile.modpost          |   7 +
>  scripts/adjust_autoksyms.sh       |  73 ----
>  scripts/basic/fixdep.c            |   3 +-
>  scripts/check-local-export        |  70 ----
>  scripts/gen_ksymdeps.sh           |  30 --
>  scripts/mod/modpost.c             | 534 ++++++++++++------------------
>  scripts/mod/modpost.h             |   1 +
>  scripts/remove-stale-files        |   2 +
>  21 files changed, 317 insertions(+), 719 deletions(-)
>  delete mode 100644 arch/ia64/include/asm/export.h
>  delete mode 100755 scripts/adjust_autoksyms.sh
>  delete mode 100755 scripts/check-local-export
>  delete mode 100755 scripts/gen_ksymdeps.sh
>
> --
> 2.39.2
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2023-05-14 12:44 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-13 20:44 [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 01/21] modpost: remove broken calculation of exception_table_entry size Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 02/21] modpost: remove fromsym info in __ex_table section mismatch warning Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 03/21] modpost: remove get_prettyname() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 04/21] modpost: squash report_extable_warnings() into extable_mismatch_handler() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 05/21] modpost: squash report_sec_mismatch() into default_mismatch_handler() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 06/21] modpost: clean up is_executable_section() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 07/21] modpost: squash extable_mismatch_handler() into default_mismatch_handler() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 08/21] modpost: pass 'tosec' down to default_mismatch_handler() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 09/21] modpost: pass section index to find_elf_symbol2() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 10/21] modpost: simplify find_elf_symbol() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 11/21] modpost: rename find_elf_symbol() and find_elf_symbol2() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 12/21] modpost: unify 'sym' and 'to' in default_mismatch_handler() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 13/21] modpost: replace r->r_offset, r->r_addend with faddr, taddr Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 14/21] modpost: remove is_shndx_special() check from section_rel(a) Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 15/21] modpost: pass struct module pointer to check_section_mismatch() Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 16/21] kbuild: generate KSYMTAB entries by modpost Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 17/21] ia64,export.h: replace EXPORT_DATA_SYMBOL* with EXPORT_SYMBOL* Masahiro Yamada
2023-05-13 20:44 ` [PATCH v4 18/21] modpost: check static EXPORT_SYMBOL* by modpost again Masahiro Yamada
2023-05-13 20:45 ` [PATCH v4 19/21] modpost: squash sym_update_namespace() into sym_add_exported() Masahiro Yamada
2023-05-13 20:45 ` [PATCH v4 20/21] modpost: use null string instead of NULL pointer for default namespace Masahiro Yamada
2023-05-13 20:45 ` [PATCH v4 21/21] kbuild: implement CONFIG_TRIM_UNUSED_KSYMS without recursion Masahiro Yamada
2023-05-14 12:43 ` [PATCH v4 00/21] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS Masahiro Yamada

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