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

From: Google
Date: Tue Jul 30 2024 - 09:03:21 EST

Next message: Google: "Re: [PATCH 3/3] tracing/kprobes: Use APIs that matches symbols with .llvm.<hash> suffix"
Previous message: Jerome Brunet: "[PATCH] pmdomain: amlogic: remove obsolete vpu domain driver"
In reply to: Song Liu: "[PATCH 2/3] kallsyms: Add APIs to match symbol without .llmv.<hash> suffix."
Next in thread: Song Liu: "Re: [PATCH 2/3] kallsyms: Add APIs to match symbol without .llmv.<hash> suffix."
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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()

>
> These APIs will be used by kprobe.

No other user need this?

Thank you,

>
> Also cleanup some code and adjust kallsyms_selftests accordingly.
>
> Signed-off-by: Song Liu <song@xxxxxxxxxx>
> ---
> include/linux/kallsyms.h | 14 +++++++
> kernel/kallsyms.c | 83 ++++++++++++++++++++++++++------------
> kernel/kallsyms_selftest.c | 22 +---------
> 3 files changed, 73 insertions(+), 46 deletions(-)
>
> diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
> index c3f075e8f60c..09b2d2099107 100644
> --- a/include/linux/kallsyms.h
> +++ b/include/linux/kallsyms.h
> @@ -74,9 +74,12 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, unsigned long),
> void *data);
> int kallsyms_on_each_match_symbol(int (*fn)(void *, unsigned long),
> const char *name, void *data);
> +int kallsyms_on_each_match_symbol_or_prefix(int (*fn)(void *, unsigned long),
> + const char *name, void *data);
>
> /* Lookup the address for a symbol. Returns 0 if not found. */
> unsigned long kallsyms_lookup_name(const char *name);
> +unsigned long kallsyms_lookup_name_or_prefix(const char *name);
>
> extern int kallsyms_lookup_size_offset(unsigned long addr,
> unsigned long *symbolsize,
> @@ -104,6 +107,11 @@ static inline unsigned long kallsyms_lookup_name(const char *name)
> return 0;
> }
>
> +static inline unsigned long kallsyms_lookup_name_or_prefix(const char *name)
> +{
> + return 0;
> +}
> +
> static inline int kallsyms_lookup_size_offset(unsigned long addr,
> unsigned long *symbolsize,
> unsigned long *offset)
> @@ -165,6 +173,12 @@ static inline int kallsyms_on_each_match_symbol(int (*fn)(void *, unsigned long)
> {
> return -EOPNOTSUPP;
> }
> +
> +static inline int kallsyms_on_each_match_symbol_or_prefix(int (*fn)(void *, unsigned long),
> + const char *name, void *data)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif /*CONFIG_KALLSYMS*/
>
> static inline void print_ip_sym(const char *loglvl, unsigned long ip)
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index fb2c77368d18..4285dd85d814 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -164,9 +164,6 @@ 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
> @@ -181,13 +178,13 @@ static void cleanup_symbol_name(char *s)
> 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;
> +
> low = 0;
> high = kallsyms_num_syms - 1;
>
> @@ -219,7 +220,7 @@ static int kallsyms_lookup_names(const char *name,
> seq = get_symbol_seq(mid);
> off = get_symbol_offset(seq);
> kallsyms_expand_symbol(off, namebuf, ARRAY_SIZE(namebuf));
> - ret = compare_symbol_name(name, namebuf);
> + ret = compare_symbol_name(name, namebuf, exact_match);
> if (ret > 0)
> low = mid + 1;
> else if (ret < 0)
> @@ -236,7 +237,7 @@ static int kallsyms_lookup_names(const char *name,
> seq = get_symbol_seq(low - 1);
> off = get_symbol_offset(seq);
> kallsyms_expand_symbol(off, namebuf, ARRAY_SIZE(namebuf));
> - if (compare_symbol_name(name, namebuf))
> + if (compare_symbol_name(name, namebuf, exact_match))
> break;
> low--;
> }
> @@ -248,7 +249,7 @@ static int kallsyms_lookup_names(const char *name,
> seq = get_symbol_seq(high + 1);
> off = get_symbol_offset(seq);
> kallsyms_expand_symbol(off, namebuf, ARRAY_SIZE(namebuf));
> - if (compare_symbol_name(name, namebuf))
> + if (compare_symbol_name(name, namebuf, exact_match))
> break;
> high++;
> }
> @@ -268,13 +269,35 @@ unsigned long kallsyms_lookup_name(const char *name)
> if (!*name)
> return 0;
>
> - ret = kallsyms_lookup_names(name, &i, NULL);
> + ret = kallsyms_lookup_names(name, &i, NULL, true);
> if (!ret)
> return kallsyms_sym_address(get_symbol_seq(i));
>
> return module_kallsyms_lookup_name(name);
> }
>
> +/*
> + * Lookup the address for this symbol.
> + *
> + * With CONFIG_LTO_CLANG=y, if there is no exact match, also try lookup
> + * symbol.llvm.<hash>.
> + */
> +unsigned long kallsyms_lookup_name_or_prefix(const char *name)
> +{
> + unsigned long addr;
> +
> + addr = kallsyms_lookup_name(name);
> +
> + if (!addr && IS_ENABLED(CONFIG_LTO_CLANG)) {
> + int ret, i;
> +
> + ret = kallsyms_lookup_names(name, &i, NULL, false);
> + if (!ret)
> + addr = kallsyms_sym_address(get_symbol_seq(i));
> + }
> + return addr;
> +}
> +
> /*
> * Iterate over all symbols in vmlinux. For symbols from modules use
> * module_kallsyms_on_each_symbol instead.
> @@ -303,7 +326,25 @@ int kallsyms_on_each_match_symbol(int (*fn)(void *, unsigned long),
> int ret;
> unsigned int i, start, end;
>
> - ret = kallsyms_lookup_names(name, &start, &end);
> + ret = kallsyms_lookup_names(name, &start, &end, true);
> + if (ret)
> + return 0;
> +
> + for (i = start; !ret && i <= end; i++) {
> + ret = fn(data, kallsyms_sym_address(get_symbol_seq(i)));
> + cond_resched();
> + }
> +
> + return ret;
> +}
> +
> +int kallsyms_on_each_match_symbol_or_prefix(int (*fn)(void *, unsigned long),
> + const char *name, void *data)
> +{
> + int ret;
> + unsigned int i, start, end;
> +
> + ret = kallsyms_lookup_names(name, &start, &end, false);
> if (ret)
> return 0;
>
> @@ -450,8 +491,6 @@ const char *kallsyms_lookup(unsigned long addr,
>
> int lookup_symbol_name(unsigned long addr, char *symname)
> {
> - int res;
> -
> symname[0] = '\0';
> symname[KSYM_NAME_LEN - 1] = '\0';
>
> @@ -462,16 +501,10 @@ int lookup_symbol_name(unsigned long addr, char *symname)
> /* Grab name */
> kallsyms_expand_symbol(get_symbol_offset(pos),
> symname, KSYM_NAME_LEN);
> - goto found;
> + return 0;
> }
> /* See if it's in a module. */
> - res = lookup_module_symbol_name(addr, symname);
> - if (res)
> - return res;
> -
> -found:
> - cleanup_symbol_name(symname);
> - return 0;
> + return lookup_module_symbol_name(addr, symname);
> }
>
> /* Look up a kernel symbol and return it in a text buffer. */
> diff --git a/kernel/kallsyms_selftest.c b/kernel/kallsyms_selftest.c
> index 2f84896a7bcb..873f7c445488 100644
> --- a/kernel/kallsyms_selftest.c
> +++ b/kernel/kallsyms_selftest.c
> @@ -187,31 +187,11 @@ static void test_perf_kallsyms_lookup_name(void)
> stat.min, stat.max, div_u64(stat.sum, stat.real_cnt));
> }
>
> -static bool match_cleanup_name(const char *s, const char *name)
> -{
> - char *p;
> - int len;
> -
> - if (!IS_ENABLED(CONFIG_LTO_CLANG))
> - return false;
> -
> - p = strstr(s, ".llvm.");
> - if (!p)
> - return false;
> -
> - len = strlen(name);
> - if (p - s != len)
> - return false;
> -
> - return !strncmp(s, name, len);
> -}
> -
> static int find_symbol(void *data, const char *name, unsigned long addr)
> {
> struct test_stat *stat = (struct test_stat *)data;
>
> - if (strcmp(name, stat->name) == 0 ||
> - (!stat->perf && match_cleanup_name(name, stat->name))) {
> + if (!strcmp(name, stat->name)) {
> stat->real_cnt++;
> stat->addr = addr;
>
> --
> 2.43.0
>

--
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>

Next message: Google: "Re: [PATCH 3/3] tracing/kprobes: Use APIs that matches symbols with .llvm.<hash> suffix"
Previous message: Jerome Brunet: "[PATCH] pmdomain: amlogic: remove obsolete vpu domain driver"
In reply to: Song Liu: "[PATCH 2/3] kallsyms: Add APIs to match symbol without .llmv.<hash> suffix."
Next in thread: Song Liu: "Re: [PATCH 2/3] kallsyms: Add APIs to match symbol without .llmv.<hash> suffix."
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]