> On Jun 19, 2023, at 4:32 AM, Petr Mladek <pmladek@xxxxxxxx> wrote:
>
> On Sun 2023-06-18 22:05:19, Song Liu wrote:
>> On Sun, Jun 18, 2023 at 8:32 PM Leizhen (ThunderTown)
>> <thunder.leizhen@xxxxxxxxxx> wrote:
[...]
>>>>>>
>>>>>> @@ -195,7 +195,7 @@ static int compare_symbol_name(const char *name, char *namebuf)
>>>>>> if (!ret)
>>>>>> return ret;
>>>>>>
>>>>>> - if (cleanup_symbol_name(namebuf) && !strcmp(name, namebuf))
>>>>>> + if (!match_exactly && cleanup_symbol_name(namebuf) && !strcmp(name, namebuf))
>>>>>
>>>>> This may affect the lookup of static functions.
>>>>
>>>> I am not following why would this be a problem. Could you give an
>>>> example of it?
>>>
>>> Here are the comments in cleanup_symbol_name(). If the compiler adds a suffix to the
>>> static function, but we do not remove the suffix, will the symbol match fail?
>>>
>>> /*
>>> * LLVM appends various suffixes for local functions and variables that
>>> * must be promoted to global scope as part of LTO. This can break
>>> * hooking of static functions with kprobes. '.' is not a valid
>>> * character in an identifier in C. Suffixes observed:
>>> * - foo.llvm.[0-9a-f]+
>>> * - foo.[0-9a-f]+
>>> */
>>
>> I think livepatch will not fail, as the tool chain should already match the
>> suffix for the function being patched. If the tool chain failed to do so,
>> livepatch can fail for other reasons (missing symbols, etc.)
>
> cleanup_symbol_name() has been added by the commit 8b8e6b5d3b013b0bd8
> ("kallsyms: strip ThinLTO hashes from static functions"). The
> motivation is that user space tools pass the symbol names found
> in sources. They do not know about the "random" suffix added
> by the "random" compiler.
I am not quite sure how tracing tools would work without knowing about
what the compiler did to the code. But I guess we are not addressing
that part here.
>
> While livepatching might want to work with the full symbol names.
> It helps to locate avoid duplication and find the right symbol.
>
> At least, this should be beneficial for kpatch tool which works directly
> with the generated symbols.
>
> Well, in theory, the cleaned symbol names might be useful for
> source-based livepatches. But there might be problem to
> distinguish different symbols with the same name and symbols
> duplicated because of inlining. Well, we tend to livepatch
> the caller anyway.
I am not quite following the direction here. Do we need more
work for this patch?
Thanks,
Song
