BPF List
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	kernel-team@fb.com, Martin KaFai Lau <martin.lau@kernel.org>
Subject: [PATCH bpf-next v2 3/5] libbpf: Handle <orig_name>.llvm.<hash> symbol properly
Date: Thu, 21 Mar 2024 13:01:14 -0700	[thread overview]
Message-ID: <20240321200114.2219721-1-yonghong.song@linux.dev> (raw)
In-Reply-To: <20240321200058.2218328-1-yonghong.song@linux.dev>

With CONFIG_LTO_CLANG_THIN enabled, with some of previous
version of kernel code base ([1]), I hit the following
error:
   test_ksyms:PASS:kallsyms_fopen 0 nsec
   test_ksyms:FAIL:ksym_find symbol 'bpf_link_fops' not found
   #118     ksyms:FAIL

The reason is that 'bpf_link_fops' is renamed to
   bpf_link_fops.llvm.8325593422554671469
Due to cross-file inlining, the static variable 'bpf_link_fops'
in syscall.c is used by a function in another file. To avoid
potential duplicated names, the llvm added suffix
'.llvm.<hash>' ([2]) to 'bpf_link_fops' variable.
Such renaming caused a problem in libbpf if 'bpf_link_fops'
is used in bpf prog as a ksym as 'bpf_link_fops' does not
match any symbol in /proc/kallsyms.

To fix this issue, libbpf needs to understand that suffix '.llvm.<hash>'
is caused by clang lto kernel and to process such symbols properly.

With latest bpf-next code base built with CONFIG_LTO_CLANG_THIN,
I cannot reproduce the above failure any more. But such an issue
could happen with other symbols.

For example, with my current kernel, I got the following from
/proc/kallsyms:
  ffffffff84782154 d __func__.net_ratelimit.llvm.6135436931166841955
  ffffffff85f0a500 d tk_core.llvm.726630847145216431
  ffffffff85fdb960 d __fs_reclaim_map.llvm.10487989720912350772
  ffffffff864c7300 d fake_dst_ops.llvm.54750082607048300

I could not easily create a selftest to test newly-added
libbpf functionality with a static C test since I do not know
which symbol is cross-file inlined. But based on my particular kernel,
the following test change can run successfully.

  diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms.c b/tools/testing/selftests/bpf/prog_tests/ksyms.c
  index 6a86d1f07800..904a103f7b1d 100644
  --- a/tools/testing/selftests/bpf/prog_tests/ksyms.c
  +++ b/tools/testing/selftests/bpf/prog_tests/ksyms.c
  @@ -42,6 +42,7 @@ void test_ksyms(void)
          ASSERT_EQ(data->out__bpf_link_fops, link_fops_addr, "bpf_link_fops");
          ASSERT_EQ(data->out__bpf_link_fops1, 0, "bpf_link_fops1");
          ASSERT_EQ(data->out__btf_size, btf_size, "btf_size");
  +       ASSERT_NEQ(data->out__fake_dst_ops, 0, "fake_dst_ops");
          ASSERT_EQ(data->out__per_cpu_start, per_cpu_start_addr, "__per_cpu_start");

   cleanup:
  diff --git a/tools/testing/selftests/bpf/progs/test_ksyms.c b/tools/testing/selftests/bpf/progs/test_ksyms.c
  index 6c9cbb5a3bdf..fe91eef54b66 100644
  --- a/tools/testing/selftests/bpf/progs/test_ksyms.c
  +++ b/tools/testing/selftests/bpf/progs/test_ksyms.c
  @@ -9,11 +9,13 @@ __u64 out__bpf_link_fops = -1;
   __u64 out__bpf_link_fops1 = -1;
   __u64 out__btf_size = -1;
   __u64 out__per_cpu_start = -1;
  +__u64 out__fake_dst_ops = -1;

   extern const void bpf_link_fops __ksym;
   extern const void __start_BTF __ksym;
   extern const void __stop_BTF __ksym;
   extern const void __per_cpu_start __ksym;
  +extern const void fake_dst_ops __ksym;
   /* non-existing symbol, weak, default to zero */
   extern const void bpf_link_fops1 __ksym __weak;

  @@ -23,6 +25,7 @@ int handler(const void *ctx)
          out__bpf_link_fops = (__u64)&bpf_link_fops;
          out__btf_size = (__u64)(&__stop_BTF - &__start_BTF);
          out__per_cpu_start = (__u64)&__per_cpu_start;
  +       out__fake_dst_ops = (__u64)&fake_dst_ops;

          out__bpf_link_fops1 = (__u64)&bpf_link_fops1;

This patch fixed the issue in libbpf such that if clang lto kernel
is enabled and the symbol resolution is for ksym's,
the suffix '.llvm.<hash>' will be ignored during comparison of
bpf prog ksym vs. symbols in /proc/kallsyms, this resolved the issue.

Note that currently kernel does not support gcc build with lto.

  [1] https://lore.kernel.org/bpf/20240302165017.1627295-1-yonghong.song@linux.dev/
  [2] https://github.com/llvm/llvm-project/blob/release/18.x/llvm/include/llvm/IR/ModuleSummaryIndex.h#L1714-L1719

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 tools/lib/bpf/libbpf.c | 64 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index a7a89269148c..8c3861192bc8 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -664,6 +664,7 @@ struct bpf_object {
 	bool loaded;
 	bool has_subcalls;
 	bool has_rodata;
+	bool need_kallsyms;
 
 	struct bpf_gen *gen_loader;
 
@@ -8016,14 +8017,73 @@ static int libbpf_kallsyms_parse(kallsyms_cb_t cb, void *ctx)
 	return err;
 }
 
+static int check_lto_kernel(void)
+{
+	static int check_lto = 2;
+	char buf[PATH_MAX];
+	struct utsname uts;
+	gzFile file;
+	int len;
+
+	if (check_lto != 2)
+		return check_lto;
+
+	uname(&uts);
+	len = snprintf(buf, PATH_MAX, "/boot/config-%s", uts.release);
+	if (len < 0) {
+		check_lto = -EINVAL;
+		goto out;
+	} else if (len >= PATH_MAX) {
+		check_lto = -ENAMETOOLONG;
+		goto out;
+	}
+
+	/* gzopen also accepts uncompressed files. */
+	file = gzopen(buf, "re");
+	if (!file)
+		file = gzopen("/proc/config.gz", "re");
+
+	if (!file) {
+		check_lto = -ENOENT;
+		goto out;
+	}
+
+	check_lto = 0;
+	while (gzgets(file, buf, sizeof(buf))) {
+		/* buf also contains '\n', skip it during comparison. */
+		if (!strncmp(buf, "CONFIG_LTO_CLANG=y", 18)) {
+			check_lto = 1;
+			break;
+		}
+	}
+
+	gzclose(file);
+out:
+	return check_lto;
+}
+
 static int kallsyms_cb(unsigned long long sym_addr, char sym_type,
 		       const char *sym_name, void *ctx)
 {
+	int lto_enabled = check_lto_kernel();
+	char orig_name[PATH_MAX], *res;
 	struct bpf_object *obj = ctx;
 	const struct btf_type *t;
 	struct extern_desc *ext;
 
-	ext = find_extern_by_name(obj, sym_name);
+	/* Only check static variables in data sections */
+	if (sym_type == 'd' && obj->need_kallsyms && lto_enabled == 1) {
+		strcpy(orig_name, sym_name);
+		res = strstr(orig_name, ".llvm.");
+		if (res) {
+			*res = '\0';
+			pr_debug("extern (ksym) '%s': use original name '%s' for comparison\n",
+				 sym_name, orig_name);
+		}
+		ext = find_extern_by_name(obj, orig_name);
+	} else {
+		ext = find_extern_by_name(obj, sym_name);
+	}
 	if (!ext || ext->type != EXT_KSYM)
 		return 0;
 
@@ -8322,7 +8382,9 @@ static int bpf_object__resolve_externs(struct bpf_object *obj,
 			return -EINVAL;
 	}
 	if (need_kallsyms) {
+		obj->need_kallsyms = true;
 		err = bpf_object__read_kallsyms_file(obj);
+		obj->need_kallsyms = false;
 		if (err)
 			return -EINVAL;
 	}
-- 
2.43.0


  parent reply	other threads:[~2024-03-21 20:01 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-21 20:00 [PATCH bpf-next v2 0/5] bpf: Fix a couple of test failures with LTO kernel Yonghong Song
2024-03-21 20:01 ` [PATCH bpf-next v2 1/5] selftests/bpf: Replace CHECK with ASSERT macros for ksyms test Yonghong Song
2024-03-22 12:38   ` Jiri Olsa
2024-03-22 16:13   ` Andrii Nakryiko
2024-03-22 16:41     ` Yonghong Song
2024-03-22 16:48       ` Andrii Nakryiko
2024-03-22 17:28         ` Yonghong Song
2024-03-21 20:01 ` [PATCH bpf-next v2 2/5] libbpf: Mark libbpf_kallsyms_parse static function Yonghong Song
2024-03-22 12:37   ` Jiri Olsa
2024-03-22 15:37     ` Yonghong Song
2024-03-21 20:01 ` Yonghong Song [this message]
2024-03-21 21:54   ` [PATCH bpf-next v2 3/5] libbpf: Handle <orig_name>.llvm.<hash> symbol properly Alexei Starovoitov
2024-03-21 23:55     ` Yonghong Song
2024-03-22  0:02       ` Alexei Starovoitov
2024-03-22  0:17         ` Andrii Nakryiko
2024-03-22  0:32           ` Yonghong Song
2024-03-22  0:18   ` Andrii Nakryiko
2024-03-22  0:34     ` Yonghong Song
2024-03-22 21:50   ` Andrii Nakryiko
2024-03-22 22:09     ` Yonghong Song
2024-03-21 20:01 ` [PATCH bpf-next v2 4/5] selftests/bpf: Fix kprobe_multi_bench_attach test failure with LTO kernel Yonghong Song
2024-03-22 12:37   ` Jiri Olsa
2024-03-22 16:01     ` Yonghong Song
2024-03-22 21:53       ` Andrii Nakryiko
2024-03-22 22:20         ` Yonghong Song
2024-03-21 20:01 ` [PATCH bpf-next v2 5/5] selftests/bpf: Add a selftest with available_filter_functions_addrs Yonghong Song
2024-03-22 12:26   ` Jiri Olsa
2024-03-22 16:07     ` Yonghong Song
2024-03-22 21:58       ` Andrii Nakryiko
2024-03-22 22:23         ` Yonghong Song

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240321200114.2219721-1-yonghong.song@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox