Thread (17 messages) 17 messages, 5 authors, 2024-08-05

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

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Date: 2024-08-02 13:04:35
Also in: live-patching, lkml

Hi,

Sorry for late reply.

On Fri, 2 Aug 2024 03:45:42 +0000
Song Liu [off-list ref] wrote:
quoted
On Aug 1, 2024, at 6:18 PM, Leizhen (ThunderTown) [off-list ref] wrote:

On 2024/7/31 9:00, Song Liu wrote:
quoted
Hi Masami, 
quoted
On Jul 30, 2024, at 6:03 AM, Masami Hiramatsu [off-list ref] wrote:

On Mon, 29 Jul 2024 17:54:32 -0700
Song Liu [off-list ref] wrote:
quoted
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()
No, I think "_or_without_suffix" is a bit complicated. If we have

0x1234 "foo.llvm.XXX"
0x2356 "bar"

and user calls

  kallsyms_lookup_name_without_suffix("bar");

This returns "bar"'s address(0x2356). Also,

  kallsyms_lookup_name_without_suffix("foo");

this will returns 0x1234. These commonly just ignore the suffix.
The difference of kallsyms_lookup_name() is that point.
quoted
quoted
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()
But this is not correctly check the symbol is static or not. If you
check the symbol is really static, it is OK. (return NULL if the symbol
is global.)

Thank you,
While these names are shorter, I think they are more confusing. Not all
folks know that only static functions can have suffixes. 

Maybe we should not hide the "try match full name first first" in the
API, and let the users handle it. Then, we can safely call the new APIs
*_without_suffix(), as Masami suggested. 

If there is no objections, I will send v2 based on this direction. 

Thanks,
Song

-- 
Masami Hiramatsu (Google) [off-list ref]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help