Re: [PATCH] bpftool: fix line info in multi-function CFG dumps

[Quentin Monnet <qmo@kernel.org>]

2026-10-24 08:23 UTC+0200 ~ Eduard Zateev <hackerowskiy@gmail.com>
> The CFG dumper computes BTF line-info offsets relative to each function, but bpf_prog_linfo__lfind() expects offsets from the start of the whole program. As a result, line annotations in later functions can resolve to source lines from the first function.
> 
> Pass the program instruction base to the graph dumper and use the global instruction offset for line-info lookup and display. Keep function-info matching relative to the function as before.


Please wrap your commit description. Also capitalise your title , and
target your patch to the bpf-next tree ("[PATCH bpf-next v2] bpftool:
Fix line info...")


> Add a selftest with a BPF-to-BPF call that checks the second function source line in visual output.
> 
> Link: https://github.com/libbpf/bpftool/issues/178


Thanks for picking up this issue. Since you found that page, you must
also have seen the patch I posted there, and on which yours is based, so
some crediting would be appreciated.


> Signed-off-by: Eduard Zateev <hackerowskiy@gmail.com>
> ---
>  tools/bpf/bpftool/cfg.c                       | 19 +++++----
>  tools/bpf/bpftool/xlated_dumper.c             | 10 +++--
>  tools/bpf/bpftool/xlated_dumper.h             |  3 +-
>  .../selftests/bpf/prog_tests/bpftool_cfg.c    | 41 +++++++++++++++++++
>  .../testing/selftests/bpf/progs/bpftool_cfg.c | 16 ++++++++
>  5 files changed, 75 insertions(+), 14 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/bpftool_cfg.c
>  create mode 100644 tools/testing/selftests/bpf/progs/bpftool_cfg.c
> 
> diff --git a/tools/bpf/bpftool/cfg.c b/tools/bpf/bpftool/cfg.c
> diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c

> diff --git a/tools/bpf/bpftool/xlated_dumper.h b/tools/bpf/bpftool/xlated_dumper.h


The fix itself looks good to me.


> diff --git a/tools/testing/selftests/bpf/prog_tests/bpftool_cfg.c b/tools/testing/selftests/bpf/prog_tests/bpftool_cfg.c
> new file mode 100644
> index 000000000..7d5b80927
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/bpftool_cfg.c
> @@ -0,0 +1,41 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <bpftool_helpers.h>
> +#include <test_progs.h>
> +#include <linux/bpf.h>
> +#include "bpftool_cfg.skel.h"
> +
> +#define MAX_BPFTOOL_OUTPUT_LEN (64 * 1024)
> +
> +void test_bpftool_cfg(void)
> +{
> +	struct bpftool_cfg *skel = NULL;
> +	struct bpf_prog_info info = {};
> +	__u32 info_len = sizeof(info);
> +	char output[MAX_BPFTOOL_OUTPUT_LEN];
> +	char cmd[MAX_BPFTOOL_CMD_LEN];
> +	const char *expected = "return\\ bpf_ktime_get_ns()\\ ?\\ skb->len\\ :\\ 0;";


How did you test this? I would expect bpftool to print "skb-\>len" and
not "skb->len", so the expected string should never be found?


> +	int err;
> +
> +	skel = bpftool_cfg__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "bpftool_cfg__open_and_load"))
> +		return;
> +
> +	err = bpf_obj_get_info_by_fd(bpf_program__fd(skel->progs.bpftool_cfg),
> +				     &info, &info_len);
> +	if (!ASSERT_OK(err, "bpf_obj_get_info_by_fd"))
> +		goto cleanup;
> +
> +	err = snprintf(cmd, sizeof(cmd), "prog dump xlated id %u visual linum",
> +		       info.id);
> +	if (!ASSERT_GT(err, 0, "format bpftool command"))
> +		goto cleanup;
> +
> +	err = get_bpftool_command_output(cmd, output, sizeof(output));
> +	if (ASSERT_OK(err, "dump program CFG")) {
> +		ASSERT_HAS_SUBSTR(output, "cluster_1", "second function in CFG");


Escaping issue aside, I don't think this check would catch the issue in
the first place? The string is present in bpftool's output regardless of
your fix. Maybe check that "return\ helper(skb);" does not appear after
the main block, instead? And please make sure to test it.

pw-bot: cr


> +		ASSERT_HAS_SUBSTR(output, expected, "second function line info");
> +	}
> +
> +cleanup:
> +	bpftool_cfg__destroy(skel);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/bpftool_cfg.c b/tools/testing/selftests/bpf/progs/bpftool_cfg.c



Nit: You're not doing a generic validation of the CFG from bpftool here,
so I think the names for the new test files should be more descriptive
to better indicate what the test it about ("bpftool_cfg_func_offset.c"?)



> new file mode 100644
> index 000000000..db01f20c8
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/bpftool_cfg.c
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +
> +static __noinline int helper(struct __sk_buff *skb)
> +{
> +	return bpf_ktime_get_ns() ? skb->len : 0;
> +}
> +
> +SEC("socket")
> +int bpftool_cfg(struct __sk_buff *skb)
> +{
> +	return helper(skb);
> +}
> +
> +char _license[] SEC("license") = "GPL";