On Aug 8, 2024, at 3:20 AM, Petr Mladek [off-list ref] wrote:

On Fri 2024-08-02 14:08:34, Song Liu 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.

--- 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, ".");

IMHO, we should not remove the suffixes like .constprop*, .part*,
.isra*. They implement a special optimized variant of the function.
It is not longer the original full-featured one.

quoted

if (res)
*res = '\0';

return;
}

-static int compare_symbol_name(const char *name, char *namebuf)
+static int compare_symbol_name(const char *name, char *namebuf, bool exact_match)
{
- /* The kallsyms_seqs_of_names is sorted based on names after
- * cleanup_symbol_name() (see scripts/kallsyms.c) if clang lto is enabled.
- * To ensure correct bisection in kallsyms_lookup_names(), do
- * cleanup_symbol_name(namebuf) before comparing name and namebuf.
- */
+ int ret = strcmp(name, namebuf);
+
+ if (exact_match || !ret)
+ return ret;
+
cleanup_symbol_name(namebuf);
return strcmp(name, namebuf);
}

@@ -204,13 +201,17 @@ static unsigned int get_symbol_seq(int index)

static int kallsyms_lookup_names(const char *name,
unsigned int *start,
- unsigned int *end)
+ unsigned int *end,
+ bool exact_match)
{
int ret;
int low, mid, high;
unsigned int seq, off;
char namebuf[KSYM_NAME_LEN];

+ if (!IS_ENABLED(CONFIG_LTO_CLANG))
+ exact_match = true;

IMHO, this is very a bad design. It causes that

    kallsyms_on_each_match_symbol_without_suffix(,,, false);

does not longer work as expected. It creates a hard to maintain
code. The code does not do what it looks like.

Indeed. It actually caused issue with GCC-built kernel in my 
tests after submitting v2. 

Thanks,
Song