Re: [PATCH] kallsyms, livepatch: Fix livepatch with CONFIG_LTO_CLANG

[Song Liu <songliubraving@meta.com>]

> On Jul 9, 2024, at 8:07 AM, Sami Tolvanen <samitolvanen@google.com> wrote:

[...]

> 
>>> I am a bit scared because using hashed symbol names in backtraces, gdb,
>>> ... would be a nightmare. Hashes are not human readable and
>>> they would complicate the life a lot. And using different names
>>> in different interfaces would complicate the life either.
>> 
>> All great points.
>> 
>> The scope of the Rust issue is self contained to modversion_info,
>> whereas for CONFIG_LTO_CLANG issue commit 8b8e6b5d3b013b0
>> ("kallsyms: strip ThinLTO hashes from static functions") describes
>> the issue with userspace tools (it doesn't explain which ones)
>> which don't expect the function name to change. This seems to happen
>> to static routines so I can only suspect this isn't an issue with
>> modversioning as the only symbols that would be used there wouldn't be
>> static.
>> 
>> Sami, what was the exact userspace issue with CONFIG_LTO_CLANG and these
>> long symbols?
> 
> The issue with LTO wasn't symbol length. IIRC the compiler renaming
> symbols with ThinLTO caused issues for folks using dynamic kprobes,
> and I seem to recall it also breaking systrace in Android, at which
> point we decided to strip the postfix in kallsyms to avoid breaking
> anything else.

Trying to understand all the requirements and constraints. IIUC, we
can mostly agree: 

(1) A way to match a symbol exactly is crucial for users like live 
    patching. 
(2) Original symbol name is useful for backtrace, etc. (IOW hash 
    alone is not enough)

With these two requirements/constraints, we need 

   original symbol name + something 

for duplicate symbols. "Something" here could be a path name 
(xxx_driver_xxx_yyy_c), or a hash, or sympos. 

At the moment, (1) is not met with CONFIG_LTO_CLANG. The original
patch tries to fix this, but the solution seems not optimal. I will 
send another version that allows kallsyms match exactly or without
suffix. 

This work shouldn't cause any problem for Rust, as Rust also need 
original symbol name + "something". If we finally decide "something" 
should be some format of hash, we can change all users (live patch, 
etc.) to use hash, which might be better than sympos. Note: I am
not trying to say "something" should be hash.  

OTOH, there is also an open question: Shall we allow tracing with
only original symbol name (without specifying _something_). I think
this a separate question and we don't have to answer it here. 

Does this make sense?

Thanks,
Song