public inbox for live-patching@vger.kernel.org
 help / color / mirror / Atom feed
* [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