Re: [PATCH v2 2/3] kallsyms: Add APIs to match symbol without .XXXX suffix.

Hi Masami,

On Mon, Aug 5, 2024 at 6:45 AM Masami Hiramatsu [off-list ref] wrote:

On Fri,  2 Aug 2024 14:08:34 -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 match only the part
without .XXXX suffix. Specifically, the following two APIs are added.

1. kallsyms_lookup_name_without_suffix()
2. kallsyms_on_each_match_symbol_without_suffix()

These APIs will be used by kprobe.

Also cleanup some code and update kallsyms_selftests accordingly.

Signed-off-by: Song Liu <song@kernel.org>

Looks good to me, but I have a nitpick.

quoted

--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c

@@ -164,30 +164,27 @@ static void cleanup_symbol_name(char *s)
 {
      char *res;

-     if (!IS_ENABLED(CONFIG_LTO_CLANG))
-             return;
-
      /*
       * 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 only in LLVM LTO observed:
-      * - foo.llvm.[0-9a-f]+
+      * character in an identifier in C, so we can just remove the
+      * suffix.
       */
-     res = strstr(s, ".llvm.");
+     res = strstr(s, ".");

nit: "strchr(s, '.')" ?

Anyway,

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thanks for your kind review!

If we have another version, I will fold this change (and the one in
kallsyms_selftest.c) in.

Song