From 2cb291596e2c1837238bc322ae3545dacb99d584 Mon Sep 17 00:00:00 2001 From: Rong Xu Date: Fri, 25 Apr 2025 13:05:41 -0700 Subject: [PATCH 1/2] objtool: Fix up st_info in COMDAT group section When __elf_create_symbol creates a local symbol, it relocates the first global symbol upwards to make space. Subsequently, elf_update_symbol() is called to refresh the symbol table section. However, this isn't sufficient, as other sections might have the reference to the old symbol index, for instance, the sh_info field of an SHT_GROUP section. This patch updates the `sh_info` field when necessary. This field serves as the key for the COMDAT group. An incorrect key would prevent the linker's from deduplicating COMDAT symbols, leading to duplicate definitions in the final link. Signed-off-by: Rong Xu Signed-off-by: Peter Zijlstra (Intel) Link: https://lkml.kernel.org/r/20250425200541.113015-1-xur@google.com --- tools/objtool/elf.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index 727a3a4fd9d7..8dffe68d705c 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -572,6 +572,30 @@ err: return -1; } +/* + * @sym's idx has changed. Update the sh_info in group sections. + */ +static void elf_update_group_sh_info(struct elf *elf, Elf32_Word symtab_idx, + Elf32_Word new_idx, Elf32_Word old_idx) +{ + struct section *sec; + + list_for_each_entry(sec, &elf->sections, list) { + if (sec->sh.sh_type != SHT_GROUP) + continue; + if (sec->sh.sh_link == symtab_idx && + sec->sh.sh_info == old_idx) { + sec->sh.sh_info = new_idx; + mark_sec_changed(elf, sec, true); + /* + * Each ELF group should have a unique symbol key. + * Return early on match. + */ + return; + } + } +} + /* * @sym's idx has changed. Update the relocs which reference it. */ @@ -745,7 +769,7 @@ __elf_create_symbol(struct elf *elf, struct symbol *sym) /* * Move the first global symbol, as per sh_info, into a new, higher - * symbol index. This fees up a spot for a new local symbol. + * symbol index. This frees up a spot for a new local symbol. */ first_non_local = symtab->sh.sh_info; old = find_symbol_by_index(elf, first_non_local); @@ -763,6 +787,7 @@ __elf_create_symbol(struct elf *elf, struct symbol *sym) if (elf_update_sym_relocs(elf, old)) return NULL; + elf_update_group_sh_info(elf, symtab->idx, new_idx, first_non_local); new_idx = first_non_local; } From 4ed9d82bf5b21d65e2f18249eec89a6a84df8f23 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Wed, 7 May 2025 16:56:55 -0700 Subject: [PATCH 2/2] objtool: Speed up SHT_GROUP reindexing After elf_update_group_sh_info() was introduced, a prototype version of "objtool klp diff" went from taking ~1s to several minutes, due to looping almost endlessly in elf_update_group_sh_info() while creating thousands of local symbols in a file with thousands of sections. Dramatically improve the performance by marking all symbols' correlated SHT_GROUP sections while reading the object. That way there's no need to search for it every time a symbol gets reindexed. Fixes: 2cb291596e2c ("objtool: Fix up st_info in COMDAT group section") Signed-off-by: Josh Poimboeuf Signed-off-by: Peter Zijlstra (Intel) Tested-by: Rong Xu Link: https://lkml.kernel.org/r/2a33e583c87e3283706f346f9d59aac20653b7fd.1746662991.git.jpoimboe@kernel.org --- tools/objtool/elf.c | 47 ++++++++++++++++++----------- tools/objtool/include/objtool/elf.h | 1 + 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index 8dffe68d705c..ca5d77db692a 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -572,28 +572,32 @@ err: return -1; } -/* - * @sym's idx has changed. Update the sh_info in group sections. - */ -static void elf_update_group_sh_info(struct elf *elf, Elf32_Word symtab_idx, - Elf32_Word new_idx, Elf32_Word old_idx) +static int mark_group_syms(struct elf *elf) { - struct section *sec; + struct section *symtab, *sec; + struct symbol *sym; + + symtab = find_section_by_name(elf, ".symtab"); + if (!symtab) { + ERROR("no .symtab"); + return -1; + } list_for_each_entry(sec, &elf->sections, list) { - if (sec->sh.sh_type != SHT_GROUP) - continue; - if (sec->sh.sh_link == symtab_idx && - sec->sh.sh_info == old_idx) { - sec->sh.sh_info = new_idx; - mark_sec_changed(elf, sec, true); - /* - * Each ELF group should have a unique symbol key. - * Return early on match. - */ - return; + if (sec->sh.sh_type == SHT_GROUP && + sec->sh.sh_link == symtab->idx) { + sym = find_symbol_by_index(elf, sec->sh.sh_info); + if (!sym) { + ERROR("%s: can't find SHT_GROUP signature symbol", + sec->name); + return -1; + } + + sym->group_sec = sec; } } + + return 0; } /* @@ -787,7 +791,11 @@ __elf_create_symbol(struct elf *elf, struct symbol *sym) if (elf_update_sym_relocs(elf, old)) return NULL; - elf_update_group_sh_info(elf, symtab->idx, new_idx, first_non_local); + if (old->group_sec) { + old->group_sec->sh.sh_info = new_idx; + mark_sec_changed(elf, old->group_sec, true); + } + new_idx = first_non_local; } @@ -1060,6 +1068,9 @@ struct elf *elf_open_read(const char *name, int flags) if (read_symbols(elf)) goto err; + if (mark_group_syms(elf)) + goto err; + if (read_relocs(elf)) goto err; diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h index c7c4e87ebe88..0a2fa3ac0079 100644 --- a/tools/objtool/include/objtool/elf.h +++ b/tools/objtool/include/objtool/elf.h @@ -72,6 +72,7 @@ struct symbol { u8 ignore : 1; struct list_head pv_target; struct reloc *relocs; + struct section *group_sec; }; struct reloc {