This patch set refactors modpost first to make it easier to
add new code.
My 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.
This patch set is applicable to
git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git kbuild
Previous version
v5: https://lore.kernel.org/linux-kbuild/CAK7LNARBiOywrMLbR=9N35sk19U0QM3xcPy7d1WqV-eyb4W23w@mail.gmail.com/T/#t
v4: https://lore.kernel.org/linux-kbuild/CAK7LNASDzy9RERN6+q6WgR4ROYZQue=SBqgbcoYuVePByHtk6Q@mail.gmail.com/T/#t
v3: https://lore.kernel.org/all/[email protected]/
Masahiro Yamada (20):
Revert "modpost: skip ELF local symbols during section mismatch check"
modpost: fix section mismatch message for R_ARM_ABS32
modpost: detect section mismatch for R_ARM_MOVW_ABS_NC and
R_ARM_MOVT_ABS
modpost: remove unused argument from secref_whitelist()
modpost: refactor find_fromsym() and find_tosym()
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
modpost: merge fromsec=DATA_SECTIONS entries in sectioncheck table
modpost: merge bad_tosec=ALL_EXIT_SECTIONS entries in sectioncheck
table
modpost: remove *_sections[] arrays
modpost: merge two similar section mismatch warnings
modpost: show offset from symbol for section mismatch warnings
.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 +-
arch/um/os-Linux/user_syms.c | 9 +-
include/asm-generic/export.h | 83 +-----
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/export-internal.h | 49 ++++
include/linux/export.h | 119 ++------
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 | 454 +++++++++++++++---------------
scripts/mod/modpost.h | 6 +-
scripts/remove-stale-files | 2 +
22 files changed, 345 insertions(+), 628 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
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.
Reviewed-by: Nick Desaulniers <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---
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 8fe2aa7bf71b..f14fe9301ae6 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);
@@ -1775,8 +1782,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);
@@ -1862,8 +1868,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;
@@ -2131,7 +2136,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
This check is unneeded. Without it, sec_name() will returns the null
string "", then section_mismatch() will return immediately.
Anyway, special section indices rarely appear in these loops.
Signed-off-by: Masahiro Yamada <[email protected]>
---
Changes in v6:
- Remove is_shndx_special() definition
scripts/mod/modpost.c | 16 ++++------------
scripts/mod/modpost.h | 5 -----
2 files changed, 4 insertions(+), 17 deletions(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index f364738a236e..40fac4f64fcd 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1361,7 +1361,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;
@@ -1404,11 +1403,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);
}
}
@@ -1416,7 +1412,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;
@@ -1463,11 +1458,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);
}
}
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 1178f40a73f3..b1e2d95f8047 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -151,11 +151,6 @@ struct elf_info {
Elf32_Word *symtab_shndx_stop;
};
-static inline int is_shndx_special(unsigned int i)
-{
- return i != SHN_XINDEX && i >= SHN_LORESERVE && i <= SHN_HIRESERVE;
-}
-
/* Accessor for sym->st_shndx, hides ugliness of "64k sections" */
static inline unsigned int get_secindex(const struct elf_info *info,
const Elf_Sym *sym)
--
2.39.2
There is no distinction between TEXT_TO_ANY_EXIT and DATA_TO_ANY_EXIT.
Just merge them.
Signed-off-by: Masahiro Yamada <[email protected]>
---
scripts/mod/modpost.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index aea9d6cd243d..b5f7f4811c39 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -844,8 +844,7 @@ static const char *const optim_symbols[] = { "*.constprop.*", NULL };
enum mismatch {
TEXT_TO_ANY_INIT,
DATA_TO_ANY_INIT,
- TEXT_TO_ANY_EXIT,
- DATA_TO_ANY_EXIT,
+ TEXTDATA_TO_ANY_EXIT,
XXXINIT_TO_SOME_INIT,
XXXEXIT_TO_SOME_EXIT,
ANY_INIT_TO_ANY_EXIT,
@@ -888,14 +887,9 @@ static const struct sectioncheck sectioncheck[] = {
.mismatch = DATA_TO_ANY_INIT,
},
{
- .fromsec = { TEXT_SECTIONS, NULL },
+ .fromsec = { TEXT_SECTIONS, DATA_SECTIONS, NULL },
.bad_tosec = { ALL_EXIT_SECTIONS, NULL },
- .mismatch = TEXT_TO_ANY_EXIT,
-},
-{
- .fromsec = { DATA_SECTIONS, NULL },
- .bad_tosec = { ALL_EXIT_SECTIONS, NULL },
- .mismatch = DATA_TO_ANY_EXIT,
+ .mismatch = TEXTDATA_TO_ANY_EXIT,
},
/* Do not reference init code/data from meminit code/data */
{
@@ -1162,8 +1156,7 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
switch (mismatch->mismatch) {
case TEXT_TO_ANY_INIT:
case DATA_TO_ANY_INIT:
- case TEXT_TO_ANY_EXIT:
- case DATA_TO_ANY_EXIT:
+ case TEXTDATA_TO_ANY_EXIT:
case XXXINIT_TO_SOME_INIT:
case XXXEXIT_TO_SOME_EXIT:
case ANY_INIT_TO_ANY_EXIT:
--
2.39.2
ARM defconfig misses to detect some section mismatches.
[test code]
#include <linux/init.h>
int __initdata foo;
int get_foo(int x) { return foo; }
It is apparently a bad reference, but modpost does not report anything
for ARM defconfig (i.e. multi_v7_defconfig).
The test code above produces the following relocations.
Relocation section '.rel.text' at offset 0x200 contains 2 entries:
Offset Info Type Sym.Value Sym. Name
00000000 0000062b R_ARM_MOVW_ABS_NC 00000000 .LANCHOR0
00000004 0000062c R_ARM_MOVT_ABS 00000000 .LANCHOR0
Relocation section '.rel.ARM.exidx' at offset 0x210 contains 2 entries:
Offset Info Type Sym.Value Sym. Name
00000000 0000022a R_ARM_PREL31 00000000 .text
00000000 00001000 R_ARM_NONE 00000000 __aeabi_unwind_cpp_pr0
Currently, R_ARM_MOVW_ABS_NC and R_ARM_MOVT_ABS are just skipped.
Add code to handle them. I checked arch/arm/kernel/module.c to learn
how the offset is encoded in the instruction.
The referenced symbol in relocation might be a local anchor.
If is_valid_name() returns false, let's search for a better symbol name.
Signed-off-by: Masahiro Yamada <[email protected]>
---
scripts/mod/modpost.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 34fbbd85bfde..ed2301e951a9 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1108,7 +1108,7 @@ static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym)
/**
* 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.
+ * return refsym. If is_valid_name() == true, 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.
* **/
@@ -1121,7 +1121,7 @@ static Elf_Sym *find_tosym(struct elf_info *elf, Elf64_Sword addr,
Elf64_Sword d;
unsigned int relsym_secindex;
- if (relsym->st_name != 0)
+ if (is_valid_name(elf, relsym))
return relsym;
/*
@@ -1312,11 +1312,19 @@ static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
unsigned int r_typ = ELF_R_TYPE(r->r_info);
Elf_Sym *sym = elf->symtab_start + ELF_R_SYM(r->r_info);
unsigned int inst = TO_NATIVE(*reloc_location(elf, sechdr, r));
+ int offset;
switch (r_typ) {
case R_ARM_ABS32:
r->r_addend = inst + sym->st_value;
break;
+ case R_ARM_MOVW_ABS_NC:
+ case R_ARM_MOVT_ABS:
+ offset = ((inst & 0xf0000) >> 4) | (inst & 0xfff);
+ offset = (offset ^ 0x8000) - 0x8000;
+ offset += sym->st_value;
+ r->r_addend = offset;
+ break;
case R_ARM_PC24:
case R_ARM_CALL:
case R_ARM_JUMP24:
--
2.39.2
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 <[email protected]>
---
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
You can merge these entries.
Signed-off-by: Masahiro Yamada <[email protected]>
---
scripts/mod/modpost.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 516323c3910a..aea9d6cd243d 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -884,12 +884,7 @@ static const struct sectioncheck sectioncheck[] = {
},
{
.fromsec = { DATA_SECTIONS, NULL },
- .bad_tosec = { ALL_XXXINIT_SECTIONS, NULL },
- .mismatch = DATA_TO_ANY_INIT,
-},
-{
- .fromsec = { DATA_SECTIONS, NULL },
- .bad_tosec = { INIT_SECTIONS, NULL },
+ .bad_tosec = { ALL_XXXINIT_SECTIONS, INIT_SECTIONS, NULL },
.mismatch = DATA_TO_ANY_INIT,
},
{
--
2.39.2
Use PATTERNS() macros to remove unneeded array definitions.
Signed-off-by: Masahiro Yamada <[email protected]>
---
scripts/mod/modpost.c | 36 +++++++++---------------------------
1 file changed, 9 insertions(+), 27 deletions(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index b5f7f4811c39..852cc164c77e 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -823,24 +823,6 @@ static void check_section(const char *modname, struct elf_info *elf,
#define ALL_TEXT_SECTIONS ALL_INIT_TEXT_SECTIONS, ALL_EXIT_TEXT_SECTIONS, \
TEXT_SECTIONS, OTHER_TEXT_SECTIONS
-/* init data sections */
-static const char *const init_data_sections[] =
- { ALL_INIT_DATA_SECTIONS, NULL };
-
-/* all init sections */
-static const char *const init_sections[] = { ALL_INIT_SECTIONS, NULL };
-
-/* all text sections */
-static const char *const text_sections[] = { ALL_TEXT_SECTIONS, NULL };
-
-/* data section */
-static const char *const data_sections[] = { DATA_SECTIONS, NULL };
-
-static const char *const head_sections[] = { ".head.text*", NULL };
-static const char *const linker_symbols[] =
- { "__init_begin", "_sinittext", "_einittext", NULL };
-static const char *const optim_symbols[] = { "*.constprop.*", NULL };
-
enum mismatch {
TEXT_TO_ANY_INIT,
DATA_TO_ANY_INIT,
@@ -1006,14 +988,14 @@ static int secref_whitelist(const char *fromsec, const char *fromsym,
const char *tosec, const char *tosym)
{
/* Check for pattern 1 */
- if (match(tosec, init_data_sections) &&
- match(fromsec, data_sections) &&
+ if (match(tosec, PATTERNS(ALL_INIT_DATA_SECTIONS)) &&
+ match(fromsec, PATTERNS(DATA_SECTIONS)) &&
strstarts(fromsym, "__param"))
return 0;
/* Check for pattern 1a */
if (strcmp(tosec, ".init.text") == 0 &&
- match(fromsec, data_sections) &&
+ match(fromsec, PATTERNS(DATA_SECTIONS)) &&
strstarts(fromsym, "__param_ops_"))
return 0;
@@ -1036,18 +1018,18 @@ static int secref_whitelist(const char *fromsec, const char *fromsym,
return 0;
/* Check for pattern 3 */
- if (match(fromsec, head_sections) &&
- match(tosec, init_sections))
+ if (strstarts(fromsec, ".head.text") &&
+ match(tosec, PATTERNS(ALL_INIT_SECTIONS)))
return 0;
/* Check for pattern 4 */
- if (match(tosym, linker_symbols))
+ if (match(tosym, PATTERNS("__init_begin", "_sinittext", "_einittext")))
return 0;
/* Check for pattern 5 */
- if (match(fromsec, text_sections) &&
- match(tosec, init_sections) &&
- match(fromsym, optim_symbols))
+ if (match(fromsec, PATTERNS(ALL_TEXT_SECTIONS)) &&
+ match(tosec, PATTERNS(ALL_INIT_SECTIONS)) &&
+ match(fromsym, PATTERNS("*.constprop.*")))
return 0;
return 1;
--
2.39.2
Currently, modpost only shows the symbol names and section names, so it
repeats the same message if there are multiple relocations in the same
symbol. It is common the relocation spans across multiple instructions.
It is better to show the offset from the symbol.
Signed-off-by: Masahiro Yamada <[email protected]>
---
scripts/mod/modpost.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index e7561fa57478..4da96746a03b 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1135,8 +1135,8 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
sec_mismatch_count++;
- warn("%s: section mismatch in reference: %s (section: %s) -> %s (section: %s)\n",
- modname, fromsym, fromsec, tosym, tosec);
+ warn("%s: section mismatch in reference: %s+0x%x (section: %s) -> %s (section: %s)\n",
+ modname, fromsym, (unsigned int)(faddr - from->st_value), fromsec, tosym, tosec);
if (mismatch->mismatch == EXTABLE_TO_NON_TEXT) {
if (match(tosec, mismatch->bad_tosec))
--
2.39.2
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 f73edc8951b2 ("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/[email protected]/
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 <[email protected]>
---
Changes in v5:
- Clean up more
.gitignore | 1 -
Makefile | 19 +---------
include/linux/export.h | 65 +++++----------------------------
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, 70 insertions(+), 191 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 f836936fb4d8..ffc2c9b632fd 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 32461a01608c..9bf081ff9903 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -37,30 +37,13 @@ extern struct module __this_module;
#define __EXPORT_SYMBOL_REF(sym) .balign 4; .long sym
#endif
-#define ____EXPORT_SYMBOL(sym, license, ns) \
+#define ___EXPORT_SYMBOL(sym, license, ns) \
.section ".export_symbol","a" ; \
__export_symbol_##license##_##sym: ; \
.asciz ns ; \
__EXPORT_SYMBOL_REF(sym) ; \
.previous
-#ifdef __GENKSYMS__
-
-#define ___EXPORT_SYMBOL(sym, sec, ns) __GENKSYMS_EXPORT_SYMBOL(sym)
-
-#elif defined(__ASSEMBLY__)
-
-#define ___EXPORT_SYMBOL(sym, license, ns) \
- ____EXPORT_SYMBOL(sym, license, ns)
-
-#else
-
-#define ___EXPORT_SYMBOL(sym, license, ns) \
- __ADDRESSABLE(sym) \
- asm(__stringify(____EXPORT_SYMBOL(sym, license, ns)))
-
-#endif
-
#if !defined(CONFIG_MODULES) || defined(__DISABLE_EXPORTS)
/*
@@ -70,50 +53,20 @@ extern struct module __this_module;
*/
#define __EXPORT_SYMBOL(sym, sec, ns)
-#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
+#elif defined(__GENKSYMS__)
-#include <generated/autoksyms.h>
+#define __EXPORT_SYMBOL(sym, sec, ns) __GENKSYMS_EXPORT_SYMBOL(sym)
-/*
- * 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.
- */
+#elif defined(__ASSEMBLY__)
-#ifdef __ASSEMBLY__
-
-#define __ksym_marker(sym) \
- .section ".discard.ksym","a" ; \
-__ksym_marker_##sym: ; \
- .previous
+#define __EXPORT_SYMBOL(sym, license, ns) \
+ ___EXPORT_SYMBOL(sym, license, ns)
#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)
+#define __EXPORT_SYMBOL(sym, license, ns) \
+ __ADDRESSABLE(sym) \
+ asm(__stringify(___EXPORT_SYMBOL(sym, license, ns)))
#endif /* CONFIG_MODULES */
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index bd4123795299..8154bd962eea 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 f14fe9301ae6..516323c3910a 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[];
};
@@ -1772,6 +1776,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;
@@ -1795,6 +1800,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;
@@ -1865,10 +1887,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;
@@ -1876,6 +1902,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",
@@ -2039,9 +2068,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);
@@ -2175,12 +2201,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;
@@ -2205,6 +2232,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;
@@ -2234,6 +2267,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
Pass a set of the name, license, and namespace to sym_add_exported().
sym_update_namespace() is unneeded.
Reviewed-by: Nick Desaulniers <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---
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 dd1d066f1214..8fe2aa7bf71b 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);
@@ -1248,8 +1231,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
@@ -2126,9 +2108,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
find_tosym() takes 'sym' and stores the return value to another
variable 'to'. You can use the same variable because we want to
replace the original one when appropriate.
Signed-off-by: Masahiro Yamada <[email protected]>
---
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 6ac0d571542c..7848bacd4b42 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1166,11 +1166,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;
@@ -1178,8 +1177,8 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
from = find_fromsym(elf, r->r_offset, fsecndx);
fromsym = sym_name(elf, from);
- to = find_tosym(elf, r->r_addend, sym);
- tosym = sym_name(elf, to);
+ tsym = find_tosym(elf, r->r_addend, tsym);
+ tosym = sym_name(elf, tsym);
/* check whitelist - we may ignore it */
if (!secref_whitelist(fromsec, fromsym, tosec, tosym))
@@ -1214,7 +1213,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
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 <[email protected]>
---
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 7848bacd4b42..f364738a236e 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1166,18 +1166,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_fromsym(elf, r->r_offset, fsecndx);
+ from = find_fromsym(elf, faddr, fsecndx);
fromsym = sym_name(elf, from);
- tsym = find_tosym(elf, r->r_addend, tsym);
+ tsym = find_tosym(elf, taddr, tsym);
tosym = sym_name(elf, tsym);
/* check whitelist - we may ignore it */
@@ -1204,7 +1204,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"
@@ -1212,7 +1212,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"
@@ -1221,17 +1221,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);
@@ -1239,8 +1240,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,
@@ -1406,7 +1408,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);
}
}
@@ -1464,7 +1467,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
On Mon, May 22, 2023 at 1:05 AM Masahiro Yamada <[email protected]> wrote:
>
>
> This patch set refactors modpost first to make it easier to
> add new code.
>
> My 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.
>
> This patch set is applicable to
> git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git kbuild
I pushed this series to
git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git
modpost-v6-testing
--
Best Regards
Masahiro Yamada
+ linux-arm-kernel
On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
>
> ARM defconfig misses to detect some section mismatches.
>
> [test code]
>
> #include <linux/init.h>
>
> int __initdata foo;
> int get_foo(int x) { return foo; }
>
> It is apparently a bad reference, but modpost does not report anything
> for ARM defconfig (i.e. multi_v7_defconfig).
>
> The test code above produces the following relocations.
>
> Relocation section '.rel.text' at offset 0x200 contains 2 entries:
> Offset Info Type Sym.Value Sym. Name
> 00000000 0000062b R_ARM_MOVW_ABS_NC 00000000 .LANCHOR0
> 00000004 0000062c R_ARM_MOVT_ABS 00000000 .LANCHOR0
>
> Relocation section '.rel.ARM.exidx' at offset 0x210 contains 2 entries:
> Offset Info Type Sym.Value Sym. Name
> 00000000 0000022a R_ARM_PREL31 00000000 .text
> 00000000 00001000 R_ARM_NONE 00000000 __aeabi_unwind_cpp_pr0
>
> Currently, R_ARM_MOVW_ABS_NC and R_ARM_MOVT_ABS are just skipped.
>
> Add code to handle them. I checked arch/arm/kernel/module.c to learn
> how the offset is encoded in the instruction.
>
> The referenced symbol in relocation might be a local anchor.
> If is_valid_name() returns false, let's search for a better symbol name.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> scripts/mod/modpost.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 34fbbd85bfde..ed2301e951a9 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1108,7 +1108,7 @@ static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym)
> /**
> * 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.
> + * return refsym. If is_valid_name() == true, 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.
> * **/
> @@ -1121,7 +1121,7 @@ static Elf_Sym *find_tosym(struct elf_info *elf, Elf64_Sword addr,
> Elf64_Sword d;
> unsigned int relsym_secindex;
>
> - if (relsym->st_name != 0)
> + if (is_valid_name(elf, relsym))
> return relsym;
>
> /*
> @@ -1312,11 +1312,19 @@ static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> unsigned int r_typ = ELF_R_TYPE(r->r_info);
> Elf_Sym *sym = elf->symtab_start + ELF_R_SYM(r->r_info);
> unsigned int inst = TO_NATIVE(*reloc_location(elf, sechdr, r));
> + int offset;
>
> switch (r_typ) {
> case R_ARM_ABS32:
> r->r_addend = inst + sym->st_value;
> break;
> + case R_ARM_MOVW_ABS_NC:
> + case R_ARM_MOVT_ABS:
> + offset = ((inst & 0xf0000) >> 4) | (inst & 0xfff);
> + offset = (offset ^ 0x8000) - 0x8000;
The code in arch/arm/kernel/module.c then right shifts the offset by
16 for R_ARM_MOVT_ABS. Is that necessary?
> + offset += sym->st_value;
> + r->r_addend = offset;
> + break;
> case R_ARM_PC24:
> case R_ARM_CALL:
> case R_ARM_JUMP24:
> --
> 2.39.2
>
--
Thanks,
~Nick Desaulniers
On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
>
> find_tosym() takes 'sym' and stores the return value to another
> variable 'to'. You can use the same variable because we want to
> replace the original one when appropriate.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
Thanks for the patch!
Reviewed-by: Nick Desaulniers <[email protected]>
> ---
>
> 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 6ac0d571542c..7848bacd4b42 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1166,11 +1166,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;
> @@ -1178,8 +1177,8 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
> from = find_fromsym(elf, r->r_offset, fsecndx);
> fromsym = sym_name(elf, from);
>
> - to = find_tosym(elf, r->r_addend, sym);
> - tosym = sym_name(elf, to);
> + tsym = find_tosym(elf, r->r_addend, tsym);
> + tosym = sym_name(elf, tsym);
>
> /* check whitelist - we may ignore it */
> if (!secref_whitelist(fromsec, fromsym, tosec, tosym))
> @@ -1214,7 +1213,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
>
--
Thanks,
~Nick Desaulniers
On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
>
> 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 <[email protected]>
Do you care to rewrap the parameter lists?
```
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 4da96746a03b..8a787521963d 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1115,8 +1115,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_Sym *tsym,
- unsigned int fsecndx, const char
*fromsec, Elf_Addr faddr,
+ Elf_Sym *tsym, unsigned int fsecndx,
+ const char *fromsec, Elf_Addr faddr,
const char *tosec, Elf_Addr taddr)
{
Elf_Sym *from;
@@ -1217,9 +1217,9 @@ static void check_export_symbol(struct module
*mod, 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)
+ 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;
```
Either way:
Reviewed-by: Nick Desaulniers <[email protected]>
> ---
>
> 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 7848bacd4b42..f364738a236e 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1166,18 +1166,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_fromsym(elf, r->r_offset, fsecndx);
> + from = find_fromsym(elf, faddr, fsecndx);
> fromsym = sym_name(elf, from);
>
> - tsym = find_tosym(elf, r->r_addend, tsym);
> + tsym = find_tosym(elf, taddr, tsym);
> tosym = sym_name(elf, tsym);
>
> /* check whitelist - we may ignore it */
> @@ -1204,7 +1204,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"
> @@ -1212,7 +1212,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"
> @@ -1221,17 +1221,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);
> @@ -1239,8 +1240,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,
> @@ -1406,7 +1408,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);
> }
> }
>
> @@ -1464,7 +1467,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
>
--
Thanks,
~Nick Desaulniers
On Mon, 22 May 2023 at 20:03, Nick Desaulniers <[email protected]> wrote:
>
> + linux-arm-kernel
>
> On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
> >
> > ARM defconfig misses to detect some section mismatches.
> >
> > [test code]
> >
> > #include <linux/init.h>
> >
> > int __initdata foo;
> > int get_foo(int x) { return foo; }
> >
> > It is apparently a bad reference, but modpost does not report anything
> > for ARM defconfig (i.e. multi_v7_defconfig).
> >
> > The test code above produces the following relocations.
> >
> > Relocation section '.rel.text' at offset 0x200 contains 2 entries:
> > Offset Info Type Sym.Value Sym. Name
> > 00000000 0000062b R_ARM_MOVW_ABS_NC 00000000 .LANCHOR0
> > 00000004 0000062c R_ARM_MOVT_ABS 00000000 .LANCHOR0
> >
> > Relocation section '.rel.ARM.exidx' at offset 0x210 contains 2 entries:
> > Offset Info Type Sym.Value Sym. Name
> > 00000000 0000022a R_ARM_PREL31 00000000 .text
> > 00000000 00001000 R_ARM_NONE 00000000 __aeabi_unwind_cpp_pr0
> >
> > Currently, R_ARM_MOVW_ABS_NC and R_ARM_MOVT_ABS are just skipped.
> >
> > Add code to handle them. I checked arch/arm/kernel/module.c to learn
> > how the offset is encoded in the instruction.
> >
> > The referenced symbol in relocation might be a local anchor.
> > If is_valid_name() returns false, let's search for a better symbol name.
> >
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > ---
> >
> > scripts/mod/modpost.c | 12 ++++++++++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index 34fbbd85bfde..ed2301e951a9 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -1108,7 +1108,7 @@ static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym)
> > /**
> > * 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.
> > + * return refsym. If is_valid_name() == true, 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.
> > * **/
> > @@ -1121,7 +1121,7 @@ static Elf_Sym *find_tosym(struct elf_info *elf, Elf64_Sword addr,
> > Elf64_Sword d;
> > unsigned int relsym_secindex;
> >
> > - if (relsym->st_name != 0)
> > + if (is_valid_name(elf, relsym))
> > return relsym;
> >
> > /*
> > @@ -1312,11 +1312,19 @@ static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> > unsigned int r_typ = ELF_R_TYPE(r->r_info);
> > Elf_Sym *sym = elf->symtab_start + ELF_R_SYM(r->r_info);
> > unsigned int inst = TO_NATIVE(*reloc_location(elf, sechdr, r));
> > + int offset;
> >
> > switch (r_typ) {
> > case R_ARM_ABS32:
> > r->r_addend = inst + sym->st_value;
> > break;
> > + case R_ARM_MOVW_ABS_NC:
> > + case R_ARM_MOVT_ABS:
> > + offset = ((inst & 0xf0000) >> 4) | (inst & 0xfff);
> > + offset = (offset ^ 0x8000) - 0x8000;
>
> The code in arch/arm/kernel/module.c then right shifts the offset by
> 16 for R_ARM_MOVT_ABS. Is that necessary?
>
MOVW/MOVT pairs are limited to an addend of -/+ 32 KiB, and the same
value must be encoded in both instructions.
When constructing the actual immediate value from the symbol value and
the addend, only the top 16 bits are used in MOVT and the bottom 16
bits in MOVW.
However, this code seems to borrow the Elf_Rela::addend field (which
ARM does not use natively) to record the intermediate value, which
would need to be split if it is used to fix up instruction opcodes.
Btw the Thumb2 encodings of MOVT and MOVW seem to be missing here.
> > + offset += sym->st_value;
> > + r->r_addend = offset;
> > + break;
> > case R_ARM_PC24:
> > case R_ARM_CALL:
> > case R_ARM_JUMP24:
> > --
> > 2.39.2
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
On Tue, May 23, 2023 at 6:50 AM Ard Biesheuvel <[email protected]> wrote:
>
> On Mon, 22 May 2023 at 20:03, Nick Desaulniers <[email protected]> wrote:
> >
> > + linux-arm-kernel
> >
> > On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
> > >
> > > ARM defconfig misses to detect some section mismatches.
> > >
> > > [test code]
> > >
> > > #include <linux/init.h>
> > >
> > > int __initdata foo;
> > > int get_foo(int x) { return foo; }
> > >
> > > It is apparently a bad reference, but modpost does not report anything
> > > for ARM defconfig (i.e. multi_v7_defconfig).
> > >
> > > The test code above produces the following relocations.
> > >
> > > Relocation section '.rel.text' at offset 0x200 contains 2 entries:
> > > Offset Info Type Sym.Value Sym. Name
> > > 00000000 0000062b R_ARM_MOVW_ABS_NC 00000000 .LANCHOR0
> > > 00000004 0000062c R_ARM_MOVT_ABS 00000000 .LANCHOR0
> > >
> > > Relocation section '.rel.ARM.exidx' at offset 0x210 contains 2 entries:
> > > Offset Info Type Sym.Value Sym. Name
> > > 00000000 0000022a R_ARM_PREL31 00000000 .text
> > > 00000000 00001000 R_ARM_NONE 00000000 __aeabi_unwind_cpp_pr0
> > >
> > > Currently, R_ARM_MOVW_ABS_NC and R_ARM_MOVT_ABS are just skipped.
> > >
> > > Add code to handle them. I checked arch/arm/kernel/module.c to learn
> > > how the offset is encoded in the instruction.
> > >
> > > The referenced symbol in relocation might be a local anchor.
> > > If is_valid_name() returns false, let's search for a better symbol name.
> > >
> > > Signed-off-by: Masahiro Yamada <[email protected]>
> > > ---
> > >
> > > scripts/mod/modpost.c | 12 ++++++++++--
> > > 1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > > index 34fbbd85bfde..ed2301e951a9 100644
> > > --- a/scripts/mod/modpost.c
> > > +++ b/scripts/mod/modpost.c
> > > @@ -1108,7 +1108,7 @@ static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym)
> > > /**
> > > * 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.
> > > + * return refsym. If is_valid_name() == true, 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.
> > > * **/
> > > @@ -1121,7 +1121,7 @@ static Elf_Sym *find_tosym(struct elf_info *elf, Elf64_Sword addr,
> > > Elf64_Sword d;
> > > unsigned int relsym_secindex;
> > >
> > > - if (relsym->st_name != 0)
> > > + if (is_valid_name(elf, relsym))
> > > return relsym;
> > >
> > > /*
> > > @@ -1312,11 +1312,19 @@ static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> > > unsigned int r_typ = ELF_R_TYPE(r->r_info);
> > > Elf_Sym *sym = elf->symtab_start + ELF_R_SYM(r->r_info);
> > > unsigned int inst = TO_NATIVE(*reloc_location(elf, sechdr, r));
> > > + int offset;
> > >
> > > switch (r_typ) {
> > > case R_ARM_ABS32:
> > > r->r_addend = inst + sym->st_value;
> > > break;
> > > + case R_ARM_MOVW_ABS_NC:
> > > + case R_ARM_MOVT_ABS:
> > > + offset = ((inst & 0xf0000) >> 4) | (inst & 0xfff);
> > > + offset = (offset ^ 0x8000) - 0x8000;
> >
> > The code in arch/arm/kernel/module.c then right shifts the offset by
> > 16 for R_ARM_MOVT_ABS. Is that necessary?
> >
>
> MOVW/MOVT pairs are limited to an addend of -/+ 32 KiB, and the same
> value must be encoded in both instructions.
In my understanding, 'movt' loads the immediate value to
the upper 16-bit of the register.
I am just curious about the code in arch/arm/kernel/module.c.
Please see 'case R_ARM_MOVT_ABS:' part.
[1] 'offset' is the immediate value encoded in instruction
[2] Add sym->st_value
[3] Right-shift 'offset' by 16
[4] Write it back to the instruction
So, the immediate value encoded in the instruction
is divided by 65536.
I guess we need something like the following?
(left-shift by 16).
if (ELF32_R_TYPE(rel->r_info) == R_ARM_MOVT_ABS ||
ELF32_R_TYPE(rel->r_info) == R_ARM_MOVT_PREL)
offset <<= 16;
>
> When constructing the actual immediate value from the symbol value and
> the addend, only the top 16 bits are used in MOVT and the bottom 16
> bits in MOVW.
>
> However, this code seems to borrow the Elf_Rela::addend field (which
> ARM does not use natively) to record the intermediate value, which
> would need to be split if it is used to fix up instruction opcodes.
At first, modpost supported only RELA for section mismatch checks.
Later, 2c1a51f39d95 ("[PATCH] kbuild: check SHT_REL sections")
added REL support.
But, the common code still used Elf_Rela.
modpost does not need to write back the fixed instruction.
modpost is only interested in the offset address.
Currently, modpost saves the offset address in
r->r_offset even for Rel. I do not like this code.
So, I am trying to reduce the use of Elf_Rela.
For example, this patch.
https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/
> Btw the Thumb2 encodings of MOVT and MOVW seem to be missing here.
Right, if CONFIG_THUMB2_KERNEL=y, section mismatch check.
Several relocation types are just skipped.
>
>
> > > + offset += sym->st_value;
> > > + r->r_addend = offset;
> > > + break;
> > > case R_ARM_PC24:
> > > case R_ARM_CALL:
> > > case R_ARM_JUMP24:
> > > --
> > > 2.39.2
> > >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
--
Best Regards
Masahiro Yamada
On Tue, 23 May 2023 at 13:59, Masahiro Yamada <[email protected]> wrote:
>
> On Tue, May 23, 2023 at 6:50 AM Ard Biesheuvel <[email protected]> wrote:
> >
> > On Mon, 22 May 2023 at 20:03, Nick Desaulniers <[email protected]> wrote:
> > >
> > > + linux-arm-kernel
> > >
> > > On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
> > > >
> > > > ARM defconfig misses to detect some section mismatches.
> > > >
> > > > [test code]
> > > >
> > > > #include <linux/init.h>
> > > >
> > > > int __initdata foo;
> > > > int get_foo(int x) { return foo; }
> > > >
> > > > It is apparently a bad reference, but modpost does not report anything
> > > > for ARM defconfig (i.e. multi_v7_defconfig).
> > > >
> > > > The test code above produces the following relocations.
> > > >
> > > > Relocation section '.rel.text' at offset 0x200 contains 2 entries:
> > > > Offset Info Type Sym.Value Sym. Name
> > > > 00000000 0000062b R_ARM_MOVW_ABS_NC 00000000 .LANCHOR0
> > > > 00000004 0000062c R_ARM_MOVT_ABS 00000000 .LANCHOR0
> > > >
> > > > Relocation section '.rel.ARM.exidx' at offset 0x210 contains 2 entries:
> > > > Offset Info Type Sym.Value Sym. Name
> > > > 00000000 0000022a R_ARM_PREL31 00000000 .text
> > > > 00000000 00001000 R_ARM_NONE 00000000 __aeabi_unwind_cpp_pr0
> > > >
> > > > Currently, R_ARM_MOVW_ABS_NC and R_ARM_MOVT_ABS are just skipped.
> > > >
> > > > Add code to handle them. I checked arch/arm/kernel/module.c to learn
> > > > how the offset is encoded in the instruction.
> > > >
> > > > The referenced symbol in relocation might be a local anchor.
> > > > If is_valid_name() returns false, let's search for a better symbol name.
> > > >
> > > > Signed-off-by: Masahiro Yamada <[email protected]>
> > > > ---
> > > >
> > > > scripts/mod/modpost.c | 12 ++++++++++--
> > > > 1 file changed, 10 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > > > index 34fbbd85bfde..ed2301e951a9 100644
> > > > --- a/scripts/mod/modpost.c
> > > > +++ b/scripts/mod/modpost.c
> > > > @@ -1108,7 +1108,7 @@ static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym)
> > > > /**
> > > > * 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.
> > > > + * return refsym. If is_valid_name() == true, 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.
> > > > * **/
> > > > @@ -1121,7 +1121,7 @@ static Elf_Sym *find_tosym(struct elf_info *elf, Elf64_Sword addr,
> > > > Elf64_Sword d;
> > > > unsigned int relsym_secindex;
> > > >
> > > > - if (relsym->st_name != 0)
> > > > + if (is_valid_name(elf, relsym))
> > > > return relsym;
> > > >
> > > > /*
> > > > @@ -1312,11 +1312,19 @@ static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> > > > unsigned int r_typ = ELF_R_TYPE(r->r_info);
> > > > Elf_Sym *sym = elf->symtab_start + ELF_R_SYM(r->r_info);
> > > > unsigned int inst = TO_NATIVE(*reloc_location(elf, sechdr, r));
> > > > + int offset;
> > > >
> > > > switch (r_typ) {
> > > > case R_ARM_ABS32:
> > > > r->r_addend = inst + sym->st_value;
> > > > break;
> > > > + case R_ARM_MOVW_ABS_NC:
> > > > + case R_ARM_MOVT_ABS:
> > > > + offset = ((inst & 0xf0000) >> 4) | (inst & 0xfff);
> > > > + offset = (offset ^ 0x8000) - 0x8000;
> > >
> > > The code in arch/arm/kernel/module.c then right shifts the offset by
> > > 16 for R_ARM_MOVT_ABS. Is that necessary?
> > >
> >
> > MOVW/MOVT pairs are limited to an addend of -/+ 32 KiB, and the same
> > value must be encoded in both instructions.
>
>
> In my understanding, 'movt' loads the immediate value to
> the upper 16-bit of the register.
>
Correct. It sets the upper 16 bits of a register without corrupting
the lower 16 bits.
> I am just curious about the code in arch/arm/kernel/module.c.
>
> Please see 'case R_ARM_MOVT_ABS:' part.
>
> [1] 'offset' is the immediate value encoded in instruction
> [2] Add sym->st_value
> [3] Right-shift 'offset' by 16
> [4] Write it back to the instruction
>
> So, the immediate value encoded in the instruction
> is divided by 65536.
>
> I guess we need something like the following?
> (left-shift by 16).
>
> if (ELF32_R_TYPE(rel->r_info) == R_ARM_MOVT_ABS ||
> ELF32_R_TYPE(rel->r_info) == R_ARM_MOVT_PREL)
> offset <<= 16;
>
No. The addend is not encoded in the same way as the effective immediate value.
The addend is limited to -/+ 32 KiB (range of s16), and the MOVT
instruction must use the same addend value as the MOVW instruction it
is paired with, without shifting.
This is necessary because otherwise, there is no way to handle an
addend/symbol combination that results in a carry between the lower
and upper 16 bit words. This is a consequence of the use of REL format
rather than RELA, where the addend is part of the relocation and not
encoded in the instructions.
>
>
>
> >
> > When constructing the actual immediate value from the symbol value and
> > the addend, only the top 16 bits are used in MOVT and the bottom 16
> > bits in MOVW.
> >
> > However, this code seems to borrow the Elf_Rela::addend field (which
> > ARM does not use natively) to record the intermediate value, which
> > would need to be split if it is used to fix up instruction opcodes.
>
> At first, modpost supported only RELA for section mismatch checks.
>
> Later, 2c1a51f39d95 ("[PATCH] kbuild: check SHT_REL sections")
> added REL support.
>
> But, the common code still used Elf_Rela.
>
>
> modpost does not need to write back the fixed instruction.
> modpost is only interested in the offset address.
>
> Currently, modpost saves the offset address in
> r->r_offset even for Rel. I do not like this code.
>
> So, I am trying to reduce the use of Elf_Rela.
> For example, this patch.
> https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/
>
Yeah, that looks better to me.
>
> > Btw the Thumb2 encodings of MOVT and MOVW seem to be missing here.
>
> Right, if CONFIG_THUMB2_KERNEL=y, section mismatch check.
>
> Several relocation types are just skipped.
>
Skipped entirely? Or only for the diagnostic print that outputs the symbol name?
On Tue, May 23, 2023 at 9:21 PM Ard Biesheuvel <[email protected]> wrote:
>
> On Tue, 23 May 2023 at 13:59, Masahiro Yamada <[email protected]> wrote:
> >
> > On Tue, May 23, 2023 at 6:50 AM Ard Biesheuvel <[email protected]> wrote:
> > >
> > > On Mon, 22 May 2023 at 20:03, Nick Desaulniers <[email protected]> wrote:
> > > >
> > > > + linux-arm-kernel
> > > >
> > > > On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
> > > > >
> > > > > ARM defconfig misses to detect some section mismatches.
> > > > >
> > > > > [test code]
> > > > >
> > > > > #include <linux/init.h>
> > > > >
> > > > > int __initdata foo;
> > > > > int get_foo(int x) { return foo; }
> > > > >
> > > > > It is apparently a bad reference, but modpost does not report anything
> > > > > for ARM defconfig (i.e. multi_v7_defconfig).
> > > > >
> > > > > The test code above produces the following relocations.
> > > > >
> > > > > Relocation section '.rel.text' at offset 0x200 contains 2 entries:
> > > > > Offset Info Type Sym.Value Sym. Name
> > > > > 00000000 0000062b R_ARM_MOVW_ABS_NC 00000000 .LANCHOR0
> > > > > 00000004 0000062c R_ARM_MOVT_ABS 00000000 .LANCHOR0
> > > > >
> > > > > Relocation section '.rel.ARM.exidx' at offset 0x210 contains 2 entries:
> > > > > Offset Info Type Sym.Value Sym. Name
> > > > > 00000000 0000022a R_ARM_PREL31 00000000 .text
> > > > > 00000000 00001000 R_ARM_NONE 00000000 __aeabi_unwind_cpp_pr0
> > > > >
> > > > > Currently, R_ARM_MOVW_ABS_NC and R_ARM_MOVT_ABS are just skipped.
> > > > >
> > > > > Add code to handle them. I checked arch/arm/kernel/module.c to learn
> > > > > how the offset is encoded in the instruction.
> > > > >
> > > > > The referenced symbol in relocation might be a local anchor.
> > > > > If is_valid_name() returns false, let's search for a better symbol name.
> > > > >
> > > > > Signed-off-by: Masahiro Yamada <[email protected]>
> > > > > ---
> > > > >
> > > > > scripts/mod/modpost.c | 12 ++++++++++--
> > > > > 1 file changed, 10 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > > > > index 34fbbd85bfde..ed2301e951a9 100644
> > > > > --- a/scripts/mod/modpost.c
> > > > > +++ b/scripts/mod/modpost.c
> > > > > @@ -1108,7 +1108,7 @@ static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym)
> > > > > /**
> > > > > * 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.
> > > > > + * return refsym. If is_valid_name() == true, 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.
> > > > > * **/
> > > > > @@ -1121,7 +1121,7 @@ static Elf_Sym *find_tosym(struct elf_info *elf, Elf64_Sword addr,
> > > > > Elf64_Sword d;
> > > > > unsigned int relsym_secindex;
> > > > >
> > > > > - if (relsym->st_name != 0)
> > > > > + if (is_valid_name(elf, relsym))
> > > > > return relsym;
> > > > >
> > > > > /*
> > > > > @@ -1312,11 +1312,19 @@ static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> > > > > unsigned int r_typ = ELF_R_TYPE(r->r_info);
> > > > > Elf_Sym *sym = elf->symtab_start + ELF_R_SYM(r->r_info);
> > > > > unsigned int inst = TO_NATIVE(*reloc_location(elf, sechdr, r));
> > > > > + int offset;
> > > > >
> > > > > switch (r_typ) {
> > > > > case R_ARM_ABS32:
> > > > > r->r_addend = inst + sym->st_value;
> > > > > break;
> > > > > + case R_ARM_MOVW_ABS_NC:
> > > > > + case R_ARM_MOVT_ABS:
> > > > > + offset = ((inst & 0xf0000) >> 4) | (inst & 0xfff);
> > > > > + offset = (offset ^ 0x8000) - 0x8000;
> > > >
> > > > The code in arch/arm/kernel/module.c then right shifts the offset by
> > > > 16 for R_ARM_MOVT_ABS. Is that necessary?
> > > >
> > >
> > > MOVW/MOVT pairs are limited to an addend of -/+ 32 KiB, and the same
> > > value must be encoded in both instructions.
> >
> >
> > In my understanding, 'movt' loads the immediate value to
> > the upper 16-bit of the register.
> >
>
> Correct. It sets the upper 16 bits of a register without corrupting
> the lower 16 bits.
>
> > I am just curious about the code in arch/arm/kernel/module.c.
> >
> > Please see 'case R_ARM_MOVT_ABS:' part.
> >
> > [1] 'offset' is the immediate value encoded in instruction
> > [2] Add sym->st_value
> > [3] Right-shift 'offset' by 16
> > [4] Write it back to the instruction
> >
> > So, the immediate value encoded in the instruction
> > is divided by 65536.
> >
> > I guess we need something like the following?
> > (left-shift by 16).
> >
> > if (ELF32_R_TYPE(rel->r_info) == R_ARM_MOVT_ABS ||
> > ELF32_R_TYPE(rel->r_info) == R_ARM_MOVT_PREL)
> > offset <<= 16;
> >
>
> No. The addend is not encoded in the same way as the effective immediate value.
>
> The addend is limited to -/+ 32 KiB (range of s16), and the MOVT
> instruction must use the same addend value as the MOVW instruction it
> is paired with, without shifting.
>
> This is necessary because otherwise, there is no way to handle an
> addend/symbol combination that results in a carry between the lower
> and upper 16 bit words. This is a consequence of the use of REL format
> rather than RELA, where the addend is part of the relocation and not
> encoded in the instructions.
Ah, OK.
Now I understand.
> >
> >
> >
> > >
> > > When constructing the actual immediate value from the symbol value and
> > > the addend, only the top 16 bits are used in MOVT and the bottom 16
> > > bits in MOVW.
> > >
> > > However, this code seems to borrow the Elf_Rela::addend field (which
> > > ARM does not use natively) to record the intermediate value, which
> > > would need to be split if it is used to fix up instruction opcodes.
> >
> > At first, modpost supported only RELA for section mismatch checks.
> >
> > Later, 2c1a51f39d95 ("[PATCH] kbuild: check SHT_REL sections")
> > added REL support.
> >
> > But, the common code still used Elf_Rela.
> >
> >
> > modpost does not need to write back the fixed instruction.
> > modpost is only interested in the offset address.
> >
> > Currently, modpost saves the offset address in
> > r->r_offset even for Rel. I do not like this code.
> >
> > So, I am trying to reduce the use of Elf_Rela.
> > For example, this patch.
> > https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/
> >
>
> Yeah, that looks better to me.
>
> >
> > > Btw the Thumb2 encodings of MOVT and MOVW seem to be missing here.
> >
> > Right, if CONFIG_THUMB2_KERNEL=y, section mismatch check.
> >
> > Several relocation types are just skipped.
> >
>
> Skipped entirely? Or only for the diagnostic print that outputs the symbol name?
Skipped entirely.
modpost cannot detect section mismatches
if you enable CONFIG_THUMB2_KERNEL.
--
Best Regards
Masahiro Yamada
On Tue, May 23, 2023 at 3:03 AM Nick Desaulniers
<[email protected]> wrote:
>
> + linux-arm-kernel
>
> On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
> >
> > ARM defconfig misses to detect some section mismatches.
> >
> > [test code]
> >
> > #include <linux/init.h>
> >
> > int __initdata foo;
> > int get_foo(int x) { return foo; }
> >
> > It is apparently a bad reference, but modpost does not report anything
> > for ARM defconfig (i.e. multi_v7_defconfig).
> >
> > The test code above produces the following relocations.
> >
> > Relocation section '.rel.text' at offset 0x200 contains 2 entries:
> > Offset Info Type Sym.Value Sym. Name
> > 00000000 0000062b R_ARM_MOVW_ABS_NC 00000000 .LANCHOR0
> > 00000004 0000062c R_ARM_MOVT_ABS 00000000 .LANCHOR0
> >
> > Relocation section '.rel.ARM.exidx' at offset 0x210 contains 2 entries:
> > Offset Info Type Sym.Value Sym. Name
> > 00000000 0000022a R_ARM_PREL31 00000000 .text
> > 00000000 00001000 R_ARM_NONE 00000000 __aeabi_unwind_cpp_pr0
> >
> > Currently, R_ARM_MOVW_ABS_NC and R_ARM_MOVT_ABS are just skipped.
> >
> > Add code to handle them. I checked arch/arm/kernel/module.c to learn
> > how the offset is encoded in the instruction.
> >
> > The referenced symbol in relocation might be a local anchor.
> > If is_valid_name() returns false, let's search for a better symbol name.
> >
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > ---
> >
> > scripts/mod/modpost.c | 12 ++++++++++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index 34fbbd85bfde..ed2301e951a9 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -1108,7 +1108,7 @@ static inline int is_valid_name(struct elf_info *elf, Elf_Sym *sym)
> > /**
> > * 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.
> > + * return refsym. If is_valid_name() == true, 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.
> > * **/
> > @@ -1121,7 +1121,7 @@ static Elf_Sym *find_tosym(struct elf_info *elf, Elf64_Sword addr,
> > Elf64_Sword d;
> > unsigned int relsym_secindex;
> >
> > - if (relsym->st_name != 0)
> > + if (is_valid_name(elf, relsym))
> > return relsym;
> >
> > /*
> > @@ -1312,11 +1312,19 @@ static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> > unsigned int r_typ = ELF_R_TYPE(r->r_info);
> > Elf_Sym *sym = elf->symtab_start + ELF_R_SYM(r->r_info);
> > unsigned int inst = TO_NATIVE(*reloc_location(elf, sechdr, r));
> > + int offset;
> >
> > switch (r_typ) {
> > case R_ARM_ABS32:
> > r->r_addend = inst + sym->st_value;
> > break;
> > + case R_ARM_MOVW_ABS_NC:
> > + case R_ARM_MOVT_ABS:
> > + offset = ((inst & 0xf0000) >> 4) | (inst & 0xfff);
> > + offset = (offset ^ 0x8000) - 0x8000;
>
> The code in arch/arm/kernel/module.c then right shifts the offset by
> 16 for R_ARM_MOVT_ABS. Is that necessary?
I replied to Ard's email, but just in case.
modpost does not need to write back the fixed instruction.
modpost is only interested in the offset address.
So, the right-shift by 16 is unneeded.
--
Best Regards
Masahiro Yamada
On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
>
> This check is unneeded. Without it, sec_name() will returns the null
> string "", then section_mismatch() will return immediately.
>
> Anyway, special section indices rarely appear in these loops.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
Thanks for the patch!
Reviewed-by: Nick Desaulniers <[email protected]>
> ---
>
> Changes in v6:
> - Remove is_shndx_special() definition
>
> scripts/mod/modpost.c | 16 ++++------------
> scripts/mod/modpost.h | 5 -----
> 2 files changed, 4 insertions(+), 17 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index f364738a236e..40fac4f64fcd 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1361,7 +1361,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;
> @@ -1404,11 +1403,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);
> }
> }
> @@ -1416,7 +1412,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;
> @@ -1463,11 +1458,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);
> }
> }
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 1178f40a73f3..b1e2d95f8047 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -151,11 +151,6 @@ struct elf_info {
> Elf32_Word *symtab_shndx_stop;
> };
>
> -static inline int is_shndx_special(unsigned int i)
> -{
> - return i != SHN_XINDEX && i >= SHN_LORESERVE && i <= SHN_HIRESERVE;
> -}
> -
> /* Accessor for sym->st_shndx, hides ugliness of "64k sections" */
> static inline unsigned int get_secindex(const struct elf_info *info,
> const Elf_Sym *sym)
> --
> 2.39.2
>
--
Thanks,
~Nick Desaulniers
On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
>
> 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 <[email protected]>
Thanks for the patch!
Reviewed-by: Nick Desaulniers <[email protected]>
> ---
>
> 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
>
--
Thanks,
~Nick Desaulniers
On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
>
> 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 f73edc8951b2 ("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/[email protected]/
>
> 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.
I think this patch removes the only 2 references to
scripts/gen_autoksyms.sh in the tree. Can or should that be removed as
well?
The rest of the patch LGTM.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> Changes in v5:
> - Clean up more
>
> .gitignore | 1 -
> Makefile | 19 +---------
> include/linux/export.h | 65 +++++----------------------------
> 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, 70 insertions(+), 191 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 f836936fb4d8..ffc2c9b632fd 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 32461a01608c..9bf081ff9903 100644
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -37,30 +37,13 @@ extern struct module __this_module;
> #define __EXPORT_SYMBOL_REF(sym) .balign 4; .long sym
> #endif
>
> -#define ____EXPORT_SYMBOL(sym, license, ns) \
> +#define ___EXPORT_SYMBOL(sym, license, ns) \
> .section ".export_symbol","a" ; \
> __export_symbol_##license##_##sym: ; \
> .asciz ns ; \
> __EXPORT_SYMBOL_REF(sym) ; \
> .previous
>
> -#ifdef __GENKSYMS__
> -
> -#define ___EXPORT_SYMBOL(sym, sec, ns) __GENKSYMS_EXPORT_SYMBOL(sym)
> -
> -#elif defined(__ASSEMBLY__)
> -
> -#define ___EXPORT_SYMBOL(sym, license, ns) \
> - ____EXPORT_SYMBOL(sym, license, ns)
> -
> -#else
> -
> -#define ___EXPORT_SYMBOL(sym, license, ns) \
> - __ADDRESSABLE(sym) \
> - asm(__stringify(____EXPORT_SYMBOL(sym, license, ns)))
> -
> -#endif
> -
> #if !defined(CONFIG_MODULES) || defined(__DISABLE_EXPORTS)
>
> /*
> @@ -70,50 +53,20 @@ extern struct module __this_module;
> */
> #define __EXPORT_SYMBOL(sym, sec, ns)
>
> -#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
> +#elif defined(__GENKSYMS__)
>
> -#include <generated/autoksyms.h>
> +#define __EXPORT_SYMBOL(sym, sec, ns) __GENKSYMS_EXPORT_SYMBOL(sym)
>
> -/*
> - * 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.
> - */
> +#elif defined(__ASSEMBLY__)
>
> -#ifdef __ASSEMBLY__
> -
> -#define __ksym_marker(sym) \
> - .section ".discard.ksym","a" ; \
> -__ksym_marker_##sym: ; \
> - .previous
> +#define __EXPORT_SYMBOL(sym, license, ns) \
> + ___EXPORT_SYMBOL(sym, license, ns)
>
> #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)
> +#define __EXPORT_SYMBOL(sym, license, ns) \
> + __ADDRESSABLE(sym) \
> + asm(__stringify(___EXPORT_SYMBOL(sym, license, ns)))
>
> #endif /* CONFIG_MODULES */
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index bd4123795299..8154bd962eea 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 f14fe9301ae6..516323c3910a 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[];
> };
>
> @@ -1772,6 +1776,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;
> @@ -1795,6 +1800,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;
> @@ -1865,10 +1887,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;
> @@ -1876,6 +1902,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",
> @@ -2039,9 +2068,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);
> @@ -2175,12 +2201,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;
> @@ -2205,6 +2232,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;
> @@ -2234,6 +2267,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
>
--
Thanks,
~Nick Desaulniers
On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
>
> Currently, modpost only shows the symbol names and section names, so it
> repeats the same message if there are multiple relocations in the same
> symbol. It is common the relocation spans across multiple instructions.
>
> It is better to show the offset from the symbol.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> scripts/mod/modpost.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index e7561fa57478..4da96746a03b 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1135,8 +1135,8 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
>
> sec_mismatch_count++;
>
> - warn("%s: section mismatch in reference: %s (section: %s) -> %s (section: %s)\n",
> - modname, fromsym, fromsec, tosym, tosec);
> + warn("%s: section mismatch in reference: %s+0x%x (section: %s) -> %s (section: %s)\n",
> + modname, fromsym, (unsigned int)(faddr - from->st_value), fromsec, tosym, tosec);
Is the cast necessary if you use the %p format specifier instead of 0x%x?
>
> if (mismatch->mismatch == EXTABLE_TO_NON_TEXT) {
> if (match(tosec, mismatch->bad_tosec))
> --
> 2.39.2
>
--
Thanks,
~Nick Desaulniers
On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
>
> There is no distinction between TEXT_TO_ANY_EXIT and DATA_TO_ANY_EXIT.
> Just merge them.
Can something similar be done for ALL_PCI_INIT_SECTIONS and
ALL_XXXINIT_SECTIONS? (as a follow up)
Reviewed-by: Nick Desaulniers <[email protected]>
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> scripts/mod/modpost.c | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index aea9d6cd243d..b5f7f4811c39 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -844,8 +844,7 @@ static const char *const optim_symbols[] = { "*.constprop.*", NULL };
> enum mismatch {
> TEXT_TO_ANY_INIT,
> DATA_TO_ANY_INIT,
> - TEXT_TO_ANY_EXIT,
> - DATA_TO_ANY_EXIT,
> + TEXTDATA_TO_ANY_EXIT,
> XXXINIT_TO_SOME_INIT,
> XXXEXIT_TO_SOME_EXIT,
> ANY_INIT_TO_ANY_EXIT,
> @@ -888,14 +887,9 @@ static const struct sectioncheck sectioncheck[] = {
> .mismatch = DATA_TO_ANY_INIT,
> },
> {
> - .fromsec = { TEXT_SECTIONS, NULL },
> + .fromsec = { TEXT_SECTIONS, DATA_SECTIONS, NULL },
> .bad_tosec = { ALL_EXIT_SECTIONS, NULL },
> - .mismatch = TEXT_TO_ANY_EXIT,
> -},
> -{
> - .fromsec = { DATA_SECTIONS, NULL },
> - .bad_tosec = { ALL_EXIT_SECTIONS, NULL },
> - .mismatch = DATA_TO_ANY_EXIT,
> + .mismatch = TEXTDATA_TO_ANY_EXIT,
> },
> /* Do not reference init code/data from meminit code/data */
> {
> @@ -1162,8 +1156,7 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
> switch (mismatch->mismatch) {
> case TEXT_TO_ANY_INIT:
> case DATA_TO_ANY_INIT:
> - case TEXT_TO_ANY_EXIT:
> - case DATA_TO_ANY_EXIT:
> + case TEXTDATA_TO_ANY_EXIT:
> case XXXINIT_TO_SOME_INIT:
> case XXXEXIT_TO_SOME_EXIT:
> case ANY_INIT_TO_ANY_EXIT:
> --
> 2.39.2
>
--
Thanks,
~Nick Desaulniers
On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
>
> You can merge these entries.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
Thanks for the patch!
Reviewed-by: Nick Desaulniers <[email protected]>
> ---
>
> scripts/mod/modpost.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 516323c3910a..aea9d6cd243d 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -884,12 +884,7 @@ static const struct sectioncheck sectioncheck[] = {
> },
> {
> .fromsec = { DATA_SECTIONS, NULL },
> - .bad_tosec = { ALL_XXXINIT_SECTIONS, NULL },
> - .mismatch = DATA_TO_ANY_INIT,
> -},
> -{
> - .fromsec = { DATA_SECTIONS, NULL },
> - .bad_tosec = { INIT_SECTIONS, NULL },
> + .bad_tosec = { ALL_XXXINIT_SECTIONS, INIT_SECTIONS, NULL },
> .mismatch = DATA_TO_ANY_INIT,
> },
> {
> --
> 2.39.2
>
--
Thanks,
~Nick Desaulniers
On Fri, May 26, 2023 at 3:27 AM Nick Desaulniers
<[email protected]> wrote:
>
> On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
> >
> > Currently, modpost only shows the symbol names and section names, so it
> > repeats the same message if there are multiple relocations in the same
> > symbol. It is common the relocation spans across multiple instructions.
> >
> > It is better to show the offset from the symbol.
> >
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > ---
> >
> > scripts/mod/modpost.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index e7561fa57478..4da96746a03b 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -1135,8 +1135,8 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
> >
> > sec_mismatch_count++;
> >
> > - warn("%s: section mismatch in reference: %s (section: %s) -> %s (section: %s)\n",
> > - modname, fromsym, fromsec, tosym, tosec);
> > + warn("%s: section mismatch in reference: %s+0x%x (section: %s) -> %s (section: %s)\n",
> > + modname, fromsym, (unsigned int)(faddr - from->st_value), fromsec, tosym, tosec);
>
> Is the cast necessary if you use the %p format specifier instead of 0x%x?
No.
faddr and from->st_value are offsets from
the start of the section. They are not pointers.
%p does not make sense.
>
> >
> > if (mismatch->mismatch == EXTABLE_TO_NON_TEXT) {
> > if (match(tosec, mismatch->bad_tosec))
> > --
> > 2.39.2
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
--
Best Regards
Masahiro Yamada
On Fri, May 26, 2023 at 3:15 AM Nick Desaulniers
<[email protected]> wrote:
>
> On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
> >
> > 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 f73edc8951b2 ("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/[email protected]/
> >
> > 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.
>
> I think this patch removes the only 2 references to
> scripts/gen_autoksyms.sh in the tree. Can or should that be removed as
> well?
Good catch.
That script is no longer needed.
I will remove it too.
> The rest of the patch LGTM.
>
> >
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > ---
> >
> > Changes in v5:
> > - Clean up more
> >
> > .gitignore | 1 -
> > Makefile | 19 +---------
> > include/linux/export.h | 65 +++++----------------------------
> > 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, 70 insertions(+), 191 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 f836936fb4d8..ffc2c9b632fd 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 32461a01608c..9bf081ff9903 100644
> > --- a/include/linux/export.h
> > +++ b/include/linux/export.h
> > @@ -37,30 +37,13 @@ extern struct module __this_module;
> > #define __EXPORT_SYMBOL_REF(sym) .balign 4; .long sym
> > #endif
> >
> > -#define ____EXPORT_SYMBOL(sym, license, ns) \
> > +#define ___EXPORT_SYMBOL(sym, license, ns) \
> > .section ".export_symbol","a" ; \
> > __export_symbol_##license##_##sym: ; \
> > .asciz ns ; \
> > __EXPORT_SYMBOL_REF(sym) ; \
> > .previous
> >
> > -#ifdef __GENKSYMS__
> > -
> > -#define ___EXPORT_SYMBOL(sym, sec, ns) __GENKSYMS_EXPORT_SYMBOL(sym)
> > -
> > -#elif defined(__ASSEMBLY__)
> > -
> > -#define ___EXPORT_SYMBOL(sym, license, ns) \
> > - ____EXPORT_SYMBOL(sym, license, ns)
> > -
> > -#else
> > -
> > -#define ___EXPORT_SYMBOL(sym, license, ns) \
> > - __ADDRESSABLE(sym) \
> > - asm(__stringify(____EXPORT_SYMBOL(sym, license, ns)))
> > -
> > -#endif
> > -
> > #if !defined(CONFIG_MODULES) || defined(__DISABLE_EXPORTS)
> >
> > /*
> > @@ -70,50 +53,20 @@ extern struct module __this_module;
> > */
> > #define __EXPORT_SYMBOL(sym, sec, ns)
> >
> > -#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
> > +#elif defined(__GENKSYMS__)
> >
> > -#include <generated/autoksyms.h>
> > +#define __EXPORT_SYMBOL(sym, sec, ns) __GENKSYMS_EXPORT_SYMBOL(sym)
> >
> > -/*
> > - * 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.
> > - */
> > +#elif defined(__ASSEMBLY__)
> >
> > -#ifdef __ASSEMBLY__
> > -
> > -#define __ksym_marker(sym) \
> > - .section ".discard.ksym","a" ; \
> > -__ksym_marker_##sym: ; \
> > - .previous
> > +#define __EXPORT_SYMBOL(sym, license, ns) \
> > + ___EXPORT_SYMBOL(sym, license, ns)
> >
> > #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)
> > +#define __EXPORT_SYMBOL(sym, license, ns) \
> > + __ADDRESSABLE(sym) \
> > + asm(__stringify(___EXPORT_SYMBOL(sym, license, ns)))
> >
> > #endif /* CONFIG_MODULES */
> >
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index bd4123795299..8154bd962eea 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 f14fe9301ae6..516323c3910a 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[];
> > };
> >
> > @@ -1772,6 +1776,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;
> > @@ -1795,6 +1800,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;
> > @@ -1865,10 +1887,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;
> > @@ -1876,6 +1902,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",
> > @@ -2039,9 +2068,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);
> > @@ -2175,12 +2201,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;
> > @@ -2205,6 +2232,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;
> > @@ -2234,6 +2267,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
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
--
Best Regards
Masahiro Yamada
On Fri, May 26, 2023 at 3:36 AM Nick Desaulniers
<[email protected]> wrote:
>
> On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <[email protected]> wrote:
> >
> > There is no distinction between TEXT_TO_ANY_EXIT and DATA_TO_ANY_EXIT.
> > Just merge them.
>
> Can something similar be done for ALL_PCI_INIT_SECTIONS and
> ALL_XXXINIT_SECTIONS? (as a follow up)
Yes, I think so.
BTW, the enum does not have a sensible name.
Commit bb15d8db7cce7 should have added
PCI_INIT_TO_ANY_INIT instead of
ANY_INIT_TO_ANY_EXIT.
--
Best Regards
Masahiro Yamada