Re: [PATCH 2/3] kallsyms: Add APIs to match symbol without .llmv.<hash> suffix.

[Leizhen (ThunderTown)]

On 2024/7/31 9:00, Song Liu wrote:
> Hi Masami, 
> 
>> On Jul 30, 2024, at 6:03 AM, Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
>>
>> On Mon, 29 Jul 2024 17:54:32 -0700
>> Song Liu <song@xxxxxxxxxx> wrote:
>>
>>> With CONFIG_LTO_CLANG=y, the compiler may add suffix to function names
>>> to avoid duplication. This causes confusion with users of kallsyms.
>>> On one hand, users like livepatch are required to match the symbols
>>> exactly. On the other hand, users like kprobe would like to match to
>>> original function names.
>>>
>>> Solve this by splitting kallsyms APIs. Specifically, existing APIs now
>>> should match the symbols exactly. Add two APIs that matches the full
>>> symbol, or only the part without .llvm.suffix. Specifically, the following
>>> two APIs are added:
>>>
>>> 1. kallsyms_lookup_name_or_prefix()
>>> 2. kallsyms_on_each_match_symbol_or_prefix()
>>
>> Since this API only removes the suffix, "match prefix" is a bit confusing.
>> (this sounds like matching "foo" with "foo" and "foo_bar", but in reality,
>> it only matches "foo" and "foo.llvm.*")
>> What about the name below?
>>
>> kallsyms_lookup_name_without_suffix()
>> kallsyms_on_each_match_symbol_without_suffix()
> 
> I am open to name suggestions. I named it as xx or prefix to highlight
> that these two APIs will try match full name first, and they only match
> the symbol without suffix when there is no full name match. 
> 
> Maybe we can call them: 
> - kallsyms_lookup_name_or_without_suffix()
> - kallsyms_on_each_match_symbol_or_without_suffix()
> 
> Again, I am open to any name selections here. 

Only static functions have suffixes. In my opinion, explicitly marking static
might be a little clearer.
kallsyms_lookup_static_name()
kallsyms_on_each_match_static_symbol()

> 
>>
>>>
>>> These APIs will be used by kprobe.
>>
>> No other user need this?
> 
> AFACIT, kprobe is the only use case here. Sami, please correct 
> me if I missed any users. 
> 
> 
> More thoughts on this: 
> 
> I actually hope we don't need these two new APIs, as they are 
> confusing. Modern compilers can do many things to the code 
> (inlining, etc.). So when we are tracing a function, we are not 
> really tracing "function in the source code". Instead, we are 
> tracing "function in the binary". If a function is inlined, it 
> will not show up in the binary. If a function is _partially_ 
> inlined (inlined by some callers, but not by others), it will 
> show up in the binary, but we won't be tracing it as it appears
> in the source code. Therefore, tracing functions by their names 
> in the source code only works under certain assumptions. And 
> these assumptions may not hold with modern compilers. Ideally, 
> I think we cannot promise the user can use name "ping_table" to
> trace function "ping_table.llvm.15394922576589127018"
> 
> Does this make sense?
> 
> Thanks,
> Song
> 
> 
> [...]
> 

-- 
Regards,
  Zhen Lei