* [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