* [PATCH] klp-build: Update demangle_name for LTO @ 2026-02-03 21:40 Song Liu 2026-02-03 23:52 ` Josh Poimboeuf 0 siblings, 1 reply; 5+ messages in thread From: Song Liu @ 2026-02-03 21:40 UTC (permalink / raw) To: live-patching Cc: jpoimboe, kernel-team, jikos, mbenes, pmladek, joe.lawrence, Song Liu With CONFIG_LTO_CLANG_THIN, __UNIQUE_ID_* can be global. Therefore, it is necessary to demangle global symbols. Also, LTO may generate symbols like: __UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar_694_695 Remove trailing '_' together with numbers and '.' so that both numbers added to the end of the symbol are removed. For example, the above s ymbol will be demangled as __UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar Signed-off-by: Song Liu <song@kernel.org> --- tools/objtool/elf.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index 2c02c7b49265..b4a7ea4720e1 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -445,9 +445,6 @@ static const char *demangle_name(struct symbol *sym) { char *str; - if (!is_local_sym(sym)) - return sym->name; - if (!is_func_sym(sym) && !is_object_sym(sym)) return sym->name; @@ -463,7 +460,13 @@ static const char *demangle_name(struct symbol *sym) for (int i = strlen(str) - 1; i >= 0; i--) { char c = str[i]; - if (!isdigit(c) && c != '.') { + /* + * With CONFIG_LTO_CLANG_THIN, the UNIQUE_ID field could + * be like: + * __UNIQUE_ID_addressable___UNIQUE_ID_<name>_628_629 + * Remove all the trailing number, '.', and '_'. + */ + if (!isdigit(c) && c != '.' && c != '_') { str[i + 1] = '\0'; break; } -- 2.47.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] klp-build: Update demangle_name for LTO 2026-02-03 21:40 [PATCH] klp-build: Update demangle_name for LTO Song Liu @ 2026-02-03 23:52 ` Josh Poimboeuf 2026-02-04 0:24 ` Song Liu 0 siblings, 1 reply; 5+ messages in thread From: Josh Poimboeuf @ 2026-02-03 23:52 UTC (permalink / raw) To: Song Liu; +Cc: live-patching, kernel-team, jikos, mbenes, pmladek, joe.lawrence On Tue, Feb 03, 2026 at 01:40:06PM -0800, Song Liu wrote: > With CONFIG_LTO_CLANG_THIN, __UNIQUE_ID_* can be global. Therefore, it > is necessary to demangle global symbols. Ouch, so LTO is changing symbol bindings :-/ If a patch causes a symbol to change from LOCAL to GLOBAL between the original and patched builds, that will break some fundamental assumptions in the correlation logic. Also, notice sym->demangled_name isn't used for correlating global symbols in correlate_symbols(). That code currently assumes all global symbols are uniquely named (and don't change between orig and patched). So this first fix seems incomplete. > Also, LTO may generate symbols like: The "also" is a clue that this should probably be two separate patches. Also, for objtool patches, please prefix the subject with "objtool:", or in this case, for klp-specific code, "objtool/klp:". > __UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar_694_695 > > Remove trailing '_' together with numbers and '.' so that both numbers > added to the end of the symbol are removed. For example, the above s > ymbol will be demangled as > > __UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar This is indeed a bug in demangle_name(), but not specific to LTO. The unique number is added by the __UNIQUE_ID() macro. I guess in this case LTO is doing some kind of nested __UNIQUE_ID() to get two "__UNIQUE_ID" strings and two numbers? But the bug still exists for the non-nested case. > Signed-off-by: Song Liu <song@kernel.org> > --- > tools/objtool/elf.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c > index 2c02c7b49265..b4a7ea4720e1 100644 > --- a/tools/objtool/elf.c > +++ b/tools/objtool/elf.c > @@ -445,9 +445,6 @@ static const char *demangle_name(struct symbol *sym) > { > char *str; > > - if (!is_local_sym(sym)) > - return sym->name; > - > if (!is_func_sym(sym) && !is_object_sym(sym)) > return sym->name; > > @@ -463,7 +460,13 @@ static const char *demangle_name(struct symbol *sym) > for (int i = strlen(str) - 1; i >= 0; i--) { > char c = str[i]; > > - if (!isdigit(c) && c != '.') { > + /* > + * With CONFIG_LTO_CLANG_THIN, the UNIQUE_ID field could > + * be like: > + * __UNIQUE_ID_addressable___UNIQUE_ID_<name>_628_629 > + * Remove all the trailing number, '.', and '_'. > + */ A comment is indeed probably warranted, though I'm thinking it should instead go above the function, with examples of both __UNIQUE_ID and "." symbols. > + if (!isdigit(c) && c != '.' && c != '_') { Ack. -- Josh ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] klp-build: Update demangle_name for LTO 2026-02-03 23:52 ` Josh Poimboeuf @ 2026-02-04 0:24 ` Song Liu 2026-02-04 1:24 ` Josh Poimboeuf 0 siblings, 1 reply; 5+ messages in thread From: Song Liu @ 2026-02-04 0:24 UTC (permalink / raw) To: Josh Poimboeuf Cc: live-patching, kernel-team, jikos, mbenes, pmladek, joe.lawrence On Tue, Feb 3, 2026 at 3:53 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > On Tue, Feb 03, 2026 at 01:40:06PM -0800, Song Liu wrote: > > With CONFIG_LTO_CLANG_THIN, __UNIQUE_ID_* can be global. Therefore, it > > is necessary to demangle global symbols. > > Ouch, so LTO is changing symbol bindings :-/ > > If a patch causes a symbol to change from LOCAL to GLOBAL between the > original and patched builds, that will break some fundamental > assumptions in the correlation logic. This can indeed happen. A function can be "LOCAL DEFAULT" XXXX in original, and "GLOBAL HIDDEN" XXXX.llvm.<hash> in patched. I am trying to fix this with incremental steps. > Also, notice sym->demangled_name isn't used for correlating global > symbols in correlate_symbols(). That code currently assumes all global > symbols are uniquely named (and don't change between orig and patched). > So this first fix seems incomplete. We still need to fix correlate_symbols(). I am not 100% sure how to do that part yet. OTOH, this part still helps. This is because checksum_update_insn() uses demangled_name. After the fix, if a function is renamed from XXXX to XXXX.llvm.<hash> after the patch, functions that call the function are not considered changed. > > Also, LTO may generate symbols like: > > The "also" is a clue that this should probably be two separate patches. > > Also, for objtool patches, please prefix the subject with "objtool:", or > in this case, for klp-specific code, "objtool/klp:". > > > __UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar_694_695 > > > > Remove trailing '_' together with numbers and '.' so that both numbers > > added to the end of the symbol are removed. For example, the above s > > ymbol will be demangled as > > > > __UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar > > This is indeed a bug in demangle_name(), but not specific to LTO. The > unique number is added by the __UNIQUE_ID() macro. > > I guess in this case LTO is doing some kind of nested __UNIQUE_ID() to > get two "__UNIQUE_ID" strings and two numbers? But the bug still exists > for the non-nested case. I don't see nested __UNIQUE_ID() without LTO. Both gcc and clang without LTO only sees one level of __UNIQUE_ID. With one level of __UNIQUE_ID(), existing code works fine. We just get one extra "_" at the end of the demanged_name. > > > Signed-off-by: Song Liu <song@kernel.org> > > --- > > tools/objtool/elf.c | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c > > index 2c02c7b49265..b4a7ea4720e1 100644 > > --- a/tools/objtool/elf.c > > +++ b/tools/objtool/elf.c > > @@ -445,9 +445,6 @@ static const char *demangle_name(struct symbol *sym) > > { > > char *str; > > > > - if (!is_local_sym(sym)) > > - return sym->name; > > - > > if (!is_func_sym(sym) && !is_object_sym(sym)) > > return sym->name; > > > > @@ -463,7 +460,13 @@ static const char *demangle_name(struct symbol *sym) > > for (int i = strlen(str) - 1; i >= 0; i--) { > > char c = str[i]; > > > > - if (!isdigit(c) && c != '.') { > > + /* > > + * With CONFIG_LTO_CLANG_THIN, the UNIQUE_ID field could > > + * be like: > > + * __UNIQUE_ID_addressable___UNIQUE_ID_<name>_628_629 > > + * Remove all the trailing number, '.', and '_'. > > + */ > > A comment is indeed probably warranted, though I'm thinking it should > instead go above the function, with examples of both __UNIQUE_ID and "." > symbols. I will split the patch into two, and fix the comment and commit log. Thanks, Song ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] klp-build: Update demangle_name for LTO 2026-02-04 0:24 ` Song Liu @ 2026-02-04 1:24 ` Josh Poimboeuf 2026-02-04 6:38 ` Song Liu 0 siblings, 1 reply; 5+ messages in thread From: Josh Poimboeuf @ 2026-02-04 1:24 UTC (permalink / raw) To: Song Liu; +Cc: live-patching, kernel-team, jikos, mbenes, pmladek, joe.lawrence On Tue, Feb 03, 2026 at 04:24:06PM -0800, Song Liu wrote: > On Tue, Feb 3, 2026 at 3:53 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > > > On Tue, Feb 03, 2026 at 01:40:06PM -0800, Song Liu wrote: > > > With CONFIG_LTO_CLANG_THIN, __UNIQUE_ID_* can be global. Therefore, it > > > is necessary to demangle global symbols. > > > > Ouch, so LTO is changing symbol bindings :-/ > > > > If a patch causes a symbol to change from LOCAL to GLOBAL between the > > original and patched builds, that will break some fundamental > > assumptions in the correlation logic. > > This can indeed happen. A function can be "LOCAL DEFAULT" XXXX > in original, and "GLOBAL HIDDEN" XXXX.llvm.<hash> in patched. > > I am trying to fix this with incremental steps. > > > Also, notice sym->demangled_name isn't used for correlating global > > symbols in correlate_symbols(). That code currently assumes all global > > symbols are uniquely named (and don't change between orig and patched). > > So this first fix seems incomplete. > > We still need to fix correlate_symbols(). I am not 100% sure how to do > that part yet. > > OTOH, this part still helps. This is because checksum_update_insn() > uses demangled_name. After the fix, if a function is renamed from > XXXX to XXXX.llvm.<hash> after the patch, functions that call the > function are not considered changed. Hm, wouldn't that still leave the .llvm at the end? > > > > Also, LTO may generate symbols like: > > > > The "also" is a clue that this should probably be two separate patches. > > > > Also, for objtool patches, please prefix the subject with "objtool:", or > > in this case, for klp-specific code, "objtool/klp:". > > > > > __UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar_694_695 > > > > > > Remove trailing '_' together with numbers and '.' so that both numbers > > > added to the end of the symbol are removed. For example, the above s > > > ymbol will be demangled as > > > > > > __UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar > > > > This is indeed a bug in demangle_name(), but not specific to LTO. The > > unique number is added by the __UNIQUE_ID() macro. > > > > I guess in this case LTO is doing some kind of nested __UNIQUE_ID() to > > get two "__UNIQUE_ID" strings and two numbers? But the bug still exists > > for the non-nested case. > > I don't see nested __UNIQUE_ID() without LTO. Both gcc and clang without > LTO only sees one level of __UNIQUE_ID. > > With one level of __UNIQUE_ID(), existing code works fine. We just get one > extra "_" at the end of the demanged_name. Ah, I see. -- Josh ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] klp-build: Update demangle_name for LTO 2026-02-04 1:24 ` Josh Poimboeuf @ 2026-02-04 6:38 ` Song Liu 0 siblings, 0 replies; 5+ messages in thread From: Song Liu @ 2026-02-04 6:38 UTC (permalink / raw) To: Josh Poimboeuf Cc: live-patching, kernel-team, jikos, mbenes, pmladek, joe.lawrence On Tue, Feb 3, 2026 at 5:24 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > On Tue, Feb 03, 2026 at 04:24:06PM -0800, Song Liu wrote: > > On Tue, Feb 3, 2026 at 3:53 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > > > > > On Tue, Feb 03, 2026 at 01:40:06PM -0800, Song Liu wrote: > > > > With CONFIG_LTO_CLANG_THIN, __UNIQUE_ID_* can be global. Therefore, it > > > > is necessary to demangle global symbols. > > > > > > Ouch, so LTO is changing symbol bindings :-/ > > > > > > If a patch causes a symbol to change from LOCAL to GLOBAL between the > > > original and patched builds, that will break some fundamental > > > assumptions in the correlation logic. > > > > This can indeed happen. A function can be "LOCAL DEFAULT" XXXX > > in original, and "GLOBAL HIDDEN" XXXX.llvm.<hash> in patched. > > > > I am trying to fix this with incremental steps. > > > > > Also, notice sym->demangled_name isn't used for correlating global > > > symbols in correlate_symbols(). That code currently assumes all global > > > symbols are uniquely named (and don't change between orig and patched). > > > So this first fix seems incomplete. > > > > We still need to fix correlate_symbols(). I am not 100% sure how to do > > that part yet. > > > > OTOH, this part still helps. This is because checksum_update_insn() > > uses demangled_name. After the fix, if a function is renamed from > > XXXX to XXXX.llvm.<hash> after the patch, functions that call the > > function are not considered changed. > > Hm, wouldn't that still leave the .llvm at the end? E.. Right. I described a different case. The actual case being fixed here is when foo.llvm.<hash_1> got changed to foo.llvm.<hash_2>. Let me think more about all the cases and have a better solution. Thanks, Song ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-02-04 6:38 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-02-03 21:40 [PATCH] klp-build: Update demangle_name for LTO Song Liu 2026-02-03 23:52 ` Josh Poimboeuf 2026-02-04 0:24 ` Song Liu 2026-02-04 1:24 ` Josh Poimboeuf 2026-02-04 6:38 ` Song Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox