[PATCH bpf-next 0/3] BTF inline functionality followups

[PATCH bpf-next 0/3] BTF inline functionality followups

[Alan Maguire]

Include additional vlen/flags verification for BTF_KIND_LOC_PARAM
(patch 1), and add function signatures to LOCSEC entries (patch 2),
adjusting test to cover these (patch 3).

Alan Maguire (3):
  bpf: Verify BTF_KIND_LOC_PARAM vlen, flags
  bpftool: Update func representation to include function signature
  selftests/bpf: Fix up bpftool btf dump test for signatures

 kernel/bpf/btf.c                              |  12 +++
 tools/bpf/bpftool/btf.c                       | 100 ++++++++++++++++--
 .../bpf/prog_tests/bpftool_btf_dump.c         |  14 +--
 3 files changed, 111 insertions(+), 15 deletions(-)

-- 
2.43.5

[PATCH bpf-next 1/3] bpf: Verify BTF_KIND_LOC_PARAM vlen, flags

[Alan Maguire]

Ensure that vlen is at least 1, at most 8 for LOC_PARAMs
and ensure that flags are a combination of expected values.

Fixes: 33c5a3278bdb ("btf: Extend UAPI to support BTF location (inline site) info")
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 kernel/bpf/btf.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 5a0179cc1676..dd7ac42649b2 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4789,6 +4789,18 @@ static s32 btf_loc_param_check_meta(struct btf_verifier_env *env,
 		btf_verifier_log_type(env, t, "Invalid btf_info kind_flag");
 		return -EINVAL;
 	}
+	/* All LOC_PARAMs have vlen of at least 1, none have vlen > 8 */
+	if (vlen < 1 || vlen > 8) {
+		btf_verifier_log_type(env, t, "Invalid vlen");
+		return -EINVAL;
+	}
+	if (p->flags & ~(BTF_LOC_PARAM_SIGNED | BTF_LOC_PARAM_CONST |
+			 BTF_LOC_PARAM_ADDR | BTF_LOC_PARAM_REG |
+			 BTF_LOC_PARAM_DEREF | BTF_LOC_PARAM_OFFSET) ||
+	    !p->flags) {
+		btf_verifier_log_type(env, t, "Invalid flags");
+		return -EINVAL;
+	}
 
 	btf_verifier_log_type(env, t, NULL);
 
-- 
2.43.5

[PATCH bpf-next 2/3] bpftool: Update func representation to include function signature

[Alan Maguire]

Augment func= output for LOCSEC entries to include a mapping from
function signature to where parameters are stored; for example:

[290179] LOCSEC 'inline.text' vlen=524941
        func='task_pid_nr(tsk [reg0])' func_type_id=136691 loc_proto_type_id=136693 offset=2097226
        func='get_current()' func_type_id=136694 loc_proto_type_id=136695 offset=2097247
        func='arch_static_branch(key [address 0x2e275e8], branch [const 0x0])' func_type_id=136697 loc_proto_type_id=136700 offset=2097296

Fixes: 321562c34d5b ("bpftool: Add ability to dump LOC_PARAM, LOC_PROTO and LOCSEC")
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/bpf/bpftool/btf.c | 100 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 91 insertions(+), 9 deletions(-)

diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
index e29c8a84e224..6e569ace6add 100644
--- a/tools/bpf/bpftool/btf.c
+++ b/tools/bpf/bpftool/btf.c
@@ -8,6 +8,7 @@
 #include <fcntl.h>
 #include <linux/err.h>
 #include <stdbool.h>
+#include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -234,8 +235,10 @@ static void btf_loc_param_str(const struct btf_type *t, char *str, size_t sz)
 						(1ULL << bits) - value;
 			}
 		}
-		snprintf(num, sizeof(num), "0x%llx%s", (unsigned long long)value,
-			 p->flags & BTF_LOC_PARAM_ADDR ? " (addr)" : "");
+		snprintf(num, sizeof(num), "%s0x%llx",
+			 p->flags & BTF_LOC_PARAM_ADDR ? "address " :
+			 p->flags & BTF_LOC_PARAM_CONST ? "const " : "",
+			 (unsigned long long)value);
 	}
 	if (i != vlen) {
 		btf_loc_param_raw_str(p, vlen, str, sz);
@@ -252,6 +255,87 @@ static void btf_loc_param_str(const struct btf_type *t, char *str, size_t sz)
 		 p->flags & BTF_LOC_PARAM_DEREF ? ")" : "");
 }
 
+static int btf_locsec_append(char *str, size_t sz, size_t *off,
+			      const char *fmt, ...)
+{
+	va_list args;
+	int ret;
+
+	if (!sz || *off >= sz - 1)
+		return -ENOSPC;
+
+	va_start(args, fmt);
+	ret = vsnprintf(str + *off, sz - *off, fmt, args);
+	va_end(args);
+	if (ret < 0 || (size_t)ret >= sz - *off) {
+		*off = sz - 1;
+		return -ENOSPC;
+	}
+	*off += ret;
+	return 0;
+}
+
+static void btf_locsec_func_str(const struct btf *btf,
+				const struct btf_loc *loc, char *str, size_t sz)
+{
+	const struct btf_type *func, *func_proto, *loc_proto;
+	const struct btf_param *params;
+	const __u32 *loc_params;
+	const char *name;
+	__u32 i, vlen;
+	size_t off = 0;
+
+	if (!sz)
+		return;
+
+	str[0] = '\0';
+	func = btf__type_by_id(btf, loc->func);
+	if (!func || !btf_is_func(func))
+		goto invalid;
+
+	name = btf_str(btf, func->name_off);
+	func_proto = btf__type_by_id(btf, func->type);
+	loc_proto = btf__type_by_id(btf, loc->loc_proto);
+	if (!func_proto || !btf_is_func_proto(func_proto) ||
+	    !loc_proto || !btf_is_loc_proto(loc_proto) ||
+	    btf_vlen(func_proto) != btf_vlen(loc_proto))
+		goto invalid;
+
+	params = (const void *)(func_proto + 1);
+	loc_params = btf_loc_proto_params(loc_proto);
+	vlen = btf_vlen(func_proto);
+	if (btf_locsec_append(str, sz, &off, "%s(", name))
+		return;
+	for (i = 0; i < vlen; i++) {
+		const struct btf_type *param_loc;
+		char param_str[256] = {};
+
+		if (!params[i].type) {
+			/* Handle varargs func proto, must be last parameter */
+			if (i != vlen - 1)
+				goto invalid;
+			if (btf_locsec_append(str, sz, &off, "%s...", i ? ", " : ""))
+				return;
+			break;
+		} else if (loc_params[i]) {
+			param_loc = btf__type_by_id(btf, loc_params[i]);
+			btf_loc_param_str(param_loc, param_str, sizeof(param_str));
+		} else {
+			snprintf(param_str, sizeof(param_str), "<unavailable>");
+		}
+
+		if (btf_locsec_append(str, sz, &off, "%s%s [%s]",
+				      i ? ", " : "", btf_str(btf, params[i].name_off),
+				      param_str))
+			return;
+	}
+	(void) btf_locsec_append(str, sz, &off, ")");
+	return;
+
+invalid:
+	snprintf(str, sz, "<invalid>");
+}
+
 static int dump_btf_type(const struct btf *btf, __u32 id,
 			 const struct btf_type *t)
 {
@@ -617,22 +701,20 @@ static int dump_btf_type(const struct btf *btf, __u32 id,
 		}
 
 		for (i = 0; i < vlen; i++, locs++) {
-			const struct btf_type *f = btf__type_by_id(btf, locs->func);
-			const char *name = "<invalid>";
+			char func_str[1024] = {};
 
-			if (f && btf_is_func(f))
-				name = btf_str(btf, f->name_off);
+			btf_locsec_func_str(btf, locs, func_str, sizeof(func_str));
 
 			if (json_output) {
 				jsonw_start_object(w);
 				jsonw_uint_field(w, "func_type_id", locs->func);
-				jsonw_string_field(w, "name", name);
+				jsonw_string_field(w, "func", func_str);
 				jsonw_uint_field(w, "loc_proto_type_id", locs->loc_proto);
 				jsonw_uint_field(w, "offset", locs->offset);
 				jsonw_end_object(w);
 			} else {
-				printf("\n\tname='%s' func_type_id=%u loc_proto_type_id=%u offset=%u",
-				       name, locs->func, locs->loc_proto, locs->offset);
+				printf("\n\tfunc='%s' func_type_id=%u loc_proto_type_id=%u offset=%u",
+				       func_str, locs->func, locs->loc_proto, locs->offset);
 			}
 		}
 		if (json_output)
-- 
2.43.5

Re: [PATCH bpf-next 2/3] bpftool: Update func representation to include function signature

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 5991 bytes --]

> commit 46a8924a62f4ce47f7ea0951644e3f0fb4ac14c2
> Author: Alan Maguire <alan.maguire@oracle.com>
>
>     bpftool: Update func representation to include function signature
>
>     Augment func= output for LOCSEC entries to include a mapping from
>     function signature to where parameters are stored; for example:
>
>     [290179] LOCSEC 'inline.text' vlen=524941
>             func='task_pid_nr(tsk [reg0])' func_type_id=136691 loc_proto_type_id=136693 offset=2097226
>             func='get_current()' func_type_id=136694 loc_proto_type_id=136695 offset=2097247
>             func='arch_static_branch(key [address 0x2e275e8], branch [const 0x0])' func_type_id=136697 loc_proto_type_id=136700 offset=2097296
>
>     Fixes: 321562c34d5b ("bpftool: Add ability to dump LOC_PARAM, LOC_PROTO and LOCSEC")
>     Suggested-by: Alexei Starovoitov <ast@kernel.org>
>     Signed-off-by: Alan Maguire <alan.maguire@oracle.com>

The changelog only describes the LOCSEC func= change. Does the commit
message need to mention the other user-visible output changes?

The patch also changes:

1. btf_loc_param_str() now prints "address 0x..." instead of
   "0x... (addr)", and adds a new "const " prefix. This changes the
   values= output of every BTF_KIND_LOC_PARAM dump line and the value=
   output of every BTF_KIND_LOC_PROTO line, not just LOCSEC entries.

2. LOCSEC entries printed name='foo' before this patch, and the JSON
   key was "name". The patch renames the plain-text field to func= and
   replaces the JSON "name" key with "func", which now holds a
   formatted signature string.

> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> index e29c8a84e224..6e569ace6add 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c

[ ... ]

> @@ -234,8 +235,10 @@ static void btf_loc_param_str(const struct btf_type *t, char *str, size_t sz)
>  						(1ULL << bits) - value;
>  			}
>  		}
> -		snprintf(num, sizeof(num), "0x%llx%s", (unsigned long long)value,
> -			 p->flags & BTF_LOC_PARAM_ADDR ? " (addr)" : "");
> +		snprintf(num, sizeof(num), "%s0x%llx",
> +			 p->flags & BTF_LOC_PARAM_ADDR ? "address " :
> +			 p->flags & BTF_LOC_PARAM_CONST ? "const " : "",
> +			 (unsigned long long)value);
>  	}
>  	if (i != vlen) {
>  		btf_loc_param_raw_str(p, vlen, str, sz);

Will negative signed constants now print as "-const 0x16" instead of
"const -0x16"?

The patch moves the kind label (e.g., "const ") into num as a prefix.
The sign, however, still lives in op, and the final format string puts
op in front of num:

    snprintf(str, sz, "%s%s%s%s%s", deref, regs, op, num, deref_close);

With no register, op is "-" when negative is true, so the '-' ends up
in front of the new label. Take a LOC_PARAM with flags =
BTF_LOC_PARAM_CONST | BTF_LOC_PARAM_SIGNED, size 4, and value
0xffffffea (-22). The SIGNED block sets negative = true and value =
0x16. num becomes "const 0x16", op is "-", and the output is:

    values='-const 0x16'

In the new LOCSEC signature it shows up as, for example,
"foo(err [-const 0x16])". Before this patch the same parameter printed
as "-0x16".

The UAPI comment in include/uapi/linux/btf.h says CONST values are
interpreted using "size, vlen and _SIGNED flag". The kernel's
btf_loc_param_check_meta() accepts CONST|SIGNED. btf_loc_param_str()
also has explicit code to handle negative constants ("we need to
determine if the value is negative so we can prepend a \"-\""). So
this input is expected, for example an inlined call site that passes
-EINVAL or -1.

Register+offset output ("fbreg - 0x10") is not affected, because
neither CONST nor ADDR is set there.

[ ... ]

> @@ -617,22 +701,20 @@ static int dump_btf_type(const struct btf *btf, __u32 id,
>  		}
>
>  		for (i = 0; i < vlen; i++, locs++) {
> -			const struct btf_type *f = btf__type_by_id(btf, locs->func);
> -			const char *name = "<invalid>";
> +			char func_str[1024] = {};
>
> -			if (f && btf_is_func(f))
> -				name = btf_str(btf, f->name_off);
> +			btf_locsec_func_str(btf, locs, func_str, sizeof(func_str));
>
>  			if (json_output) {
>  				jsonw_start_object(w);
>  				jsonw_uint_field(w, "func_type_id", locs->func);
> -				jsonw_string_field(w, "name", name);
> +				jsonw_string_field(w, "func", func_str);
>  				jsonw_uint_field(w, "loc_proto_type_id", locs->loc_proto);
>  				jsonw_uint_field(w, "offset", locs->offset);
>  				jsonw_end_object(w);
>  			} else {
> -				printf("\n\tname='%s' func_type_id=%u loc_proto_type_id=%u offset=%u",
> -				       name, locs->func, locs->loc_proto, locs->offset);
> +				printf("\n\tfunc='%s' func_type_id=%u loc_proto_type_id=%u offset=%u",
> +				       func_str, locs->func, locs->loc_proto, locs->offset);
>  			}
>  		}
>  		if (json_output)

Does this commit break the bpftool_btf_dump selftest?

At this commit, test_loc_dump() in
tools/testing/selftests/bpf/prog_tests/bpftool_btf_dump.c still
compares the output exactly against the old format:

    "[7] LOC_PARAM ... values='0x123456789abcdef0 (addr)'\n"
    "[8] LOC_PARAM ... values='0xdeadbeeffeedface'\n"
    ...
    "\tname='foo' func_type_id=3 loc_proto_type_id=10 offset=64\n";

    ASSERT_OK(compare_text_to_expected(dump, expected), "cmp_loc_dump");

This patch changes all of these lines: the 'address '/'const '
prefixes, and name= becoming func=. The test's FUNC_PROTO also has
vlen=5 while the LOC_PROTO has vlen=6. The new btf_locsec_func_str()
treats that vlen mismatch as "<invalid>", so cmp_loc_dump fails at
this commit.

The selftest update in f2a598589f90 ("selftests/bpf: Fix up bpftool
btf dump test for signatures") fixes it later in the series. BPF CI
runs test_progs per commit, so would it be better to fold that test
update into this patch to keep the series bisectable?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/36122090541

Re: [PATCH bpf-next 2/3] bpftool: Update func representation to include function signature

[Quentin Monnet]

2026-09-25 10:52 UTC+0100 ~ Alan Maguire <alan.maguire@oracle.com>
> Augment func= output for LOCSEC entries to include a mapping from
> function signature to where parameters are stored; for example:
> 
> [290179] LOCSEC 'inline.text' vlen=524941
>         func='task_pid_nr(tsk [reg0])' func_type_id=136691 loc_proto_type_id=136693 offset=2097226
>         func='get_current()' func_type_id=136694 loc_proto_type_id=136695 offset=2097247
>         func='arch_static_branch(key [address 0x2e275e8], branch [const 0x0])' func_type_id=136697 loc_proto_type_id=136700 offset=2097296
> 
> Fixes: 321562c34d5b ("bpftool: Add ability to dump LOC_PARAM, LOC_PROTO and LOCSEC")
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/bpf/bpftool/btf.c | 100 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 91 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
> index e29c8a84e224..6e569ace6add 100644
> --- a/tools/bpf/bpftool/btf.c
> +++ b/tools/bpf/bpftool/btf.c
> @@ -8,6 +8,7 @@
>  #include <fcntl.h>
>  #include <linux/err.h>
>  #include <stdbool.h>
> +#include <stdarg.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> @@ -234,8 +235,10 @@ static void btf_loc_param_str(const struct btf_type *t, char *str, size_t sz)
>  						(1ULL << bits) - value;
>  			}
>  		}
> -		snprintf(num, sizeof(num), "0x%llx%s", (unsigned long long)value,
> -			 p->flags & BTF_LOC_PARAM_ADDR ? " (addr)" : "");
> +		snprintf(num, sizeof(num), "%s0x%llx",
> +			 p->flags & BTF_LOC_PARAM_ADDR ? "address " :
> +			 p->flags & BTF_LOC_PARAM_CONST ? "const " : "",
> +			 (unsigned long long)value);
>  	}
>  	if (i != vlen) {
>  		btf_loc_param_raw_str(p, vlen, str, sz);


Thanks Alan!

bpf-ci's comment about "-const 0x16" instead of "const -0x16" seems
legit, please take a look.


> @@ -252,6 +255,87 @@ static void btf_loc_param_str(const struct btf_type *t, char *str, size_t sz)
>  		 p->flags & BTF_LOC_PARAM_DEREF ? ")" : "");
>  }
>  
> +static int btf_locsec_append(char *str, size_t sz, size_t *off,
> +			      const char *fmt, ...)
> +{
> +	va_list args;
> +	int ret;
> +
> +	if (!sz || *off >= sz - 1)
> +		return -ENOSPC;
> +
> +	va_start(args, fmt);
> +	ret = vsnprintf(str + *off, sz - *off, fmt, args);


Nit: Do you really need vsnprintf()? It looks like you always
concatenate, never format any number, so it's probably not the most
efficient. I don't mind much, though.


> +	va_end(args);
> +	if (ret < 0 || (size_t)ret >= sz - *off) {
> +		*off = sz - 1;
> +		return -ENOSPC;


It seems unlikely we'll hit this, but maybe warn that the string is
truncated in that case, or replace the last characters with "..." or
"[truncated]" or something like this??


> +	}
> +	*off += ret;
> +	return 0;
> +}
> +
> +static void btf_locsec_func_str(const struct btf *btf,
> +				const struct btf_loc *loc, char *str, size_t sz)
> +{
> +	const struct btf_type *func, *func_proto, *loc_proto;
> +	const struct btf_param *params;
> +	const __u32 *loc_params;
> +	const char *name;
> +	__u32 i, vlen;
> +	size_t off = 0;
> +
> +	if (!sz)
> +		return;
> +
> +	str[0] = '\0';
> +	func = btf__type_by_id(btf, loc->func);
> +	if (!func || !btf_is_func(func))
> +		goto invalid;
> +
> +	name = btf_str(btf, func->name_off);
> +	func_proto = btf__type_by_id(btf, func->type);
> +	loc_proto = btf__type_by_id(btf, loc->loc_proto);
> +	if (!func_proto || !btf_is_func_proto(func_proto) ||
> +	    !loc_proto || !btf_is_loc_proto(loc_proto) ||
> +	    btf_vlen(func_proto) != btf_vlen(loc_proto))
> +		goto invalid;
> +
> +	params = (const void *)(func_proto + 1);
> +	loc_params = btf_loc_proto_params(loc_proto);
> +	vlen = btf_vlen(func_proto);
> +	if (btf_locsec_append(str, sz, &off, "%s(", name))
> +		return;
> +	for (i = 0; i < vlen; i++) {
> +		const struct btf_type *param_loc;
> +		char param_str[256] = {};
> +
> +		if (!params[i].type) {
> +			/* Handle varargs func proto, must be last parameter */
> +			if (i != vlen - 1)
> +				goto invalid;
> +			if (btf_locsec_append(str, sz, &off, "%s...", i ? ", " : ""))
> +				return;
> +			break;
> +		} else if (loc_params[i]) {
> +			param_loc = btf__type_by_id(btf, loc_params[i]);
> +			btf_loc_param_str(param_loc, param_str, sizeof(param_str));
> +		} else {
> +			snprintf(param_str, sizeof(param_str), "<unavailable>");
> +		}
> +
> +		if (btf_locsec_append(str, sz, &off, "%s%s [%s]",
> +				      i ? ", " : "", btf_str(btf, params[i].name_off),
> +				      param_str))
> +			return;
> +	}
> +	(void) btf_locsec_append(str, sz, &off, ")");
> +	return;
> +
> +invalid:
> +	snprintf(str, sz, "<invalid>");
> +}
> +
>  static int dump_btf_type(const struct btf *btf, __u32 id,
>  			 const struct btf_type *t)
>  {
> @@ -617,22 +701,20 @@ static int dump_btf_type(const struct btf *btf, __u32 id,
>  		}
>  
>  		for (i = 0; i < vlen; i++, locs++) {
> -			const struct btf_type *f = btf__type_by_id(btf, locs->func);
> -			const char *name = "<invalid>";
> +			char func_str[1024] = {};
>  
> -			if (f && btf_is_func(f))
> -				name = btf_str(btf, f->name_off);
> +			btf_locsec_func_str(btf, locs, func_str, sizeof(func_str));
>  
>  			if (json_output) {
>  				jsonw_start_object(w);
>  				jsonw_uint_field(w, "func_type_id", locs->func);
> -				jsonw_string_field(w, "name", name);
> +				jsonw_string_field(w, "func", func_str);


Would it be worth keeping the name, too, in the JSON? So that if
somebody wants the name only, they don't have to parse it from "func"?


>  				jsonw_uint_field(w, "loc_proto_type_id", locs->loc_proto);
>  				jsonw_uint_field(w, "offset", locs->offset);
>  				jsonw_end_object(w);
>  			} else {
> -				printf("\n\tname='%s' func_type_id=%u loc_proto_type_id=%u offset=%u",
> -				       name, locs->func, locs->loc_proto, locs->offset);
> +				printf("\n\tfunc='%s' func_type_id=%u loc_proto_type_id=%u offset=%u",
> +				       func_str, locs->func, locs->loc_proto, locs->offset);
>  			}
>  		}
>  		if (json_output)

Re: [PATCH bpf-next 2/3] bpftool: Update func representation to include function signature

[Alan Maguire]

On 25/09/2026 13:07, Quentin Monnet wrote:
> 2026-09-25 10:52 UTC+0100 ~ Alan Maguire <alan.maguire@oracle.com>
>> Augment func= output for LOCSEC entries to include a mapping from
>> function signature to where parameters are stored; for example:
>>
>> [290179] LOCSEC 'inline.text' vlen=524941
>>         func='task_pid_nr(tsk [reg0])' func_type_id=136691 loc_proto_type_id=136693 offset=2097226
>>         func='get_current()' func_type_id=136694 loc_proto_type_id=136695 offset=2097247
>>         func='arch_static_branch(key [address 0x2e275e8], branch [const 0x0])' func_type_id=136697 loc_proto_type_id=136700 offset=2097296
>>
>> Fixes: 321562c34d5b ("bpftool: Add ability to dump LOC_PARAM, LOC_PROTO and LOCSEC")
>> Suggested-by: Alexei Starovoitov <ast@kernel.org>
>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>> ---
>>  tools/bpf/bpftool/btf.c | 100 ++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 91 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/btf.c b/tools/bpf/bpftool/btf.c
>> index e29c8a84e224..6e569ace6add 100644
>> --- a/tools/bpf/bpftool/btf.c
>> +++ b/tools/bpf/bpftool/btf.c
>> @@ -8,6 +8,7 @@
>>  #include <fcntl.h>
>>  #include <linux/err.h>
>>  #include <stdbool.h>
>> +#include <stdarg.h>
>>  #include <stdio.h>
>>  #include <stdlib.h>
>>  #include <string.h>
>> @@ -234,8 +235,10 @@ static void btf_loc_param_str(const struct btf_type *t, char *str, size_t sz)
>>  						(1ULL << bits) - value;
>>  			}
>>  		}
>> -		snprintf(num, sizeof(num), "0x%llx%s", (unsigned long long)value,
>> -			 p->flags & BTF_LOC_PARAM_ADDR ? " (addr)" : "");
>> +		snprintf(num, sizeof(num), "%s0x%llx",
>> +			 p->flags & BTF_LOC_PARAM_ADDR ? "address " :
>> +			 p->flags & BTF_LOC_PARAM_CONST ? "const " : "",
>> +			 (unsigned long long)value);
>>  	}
>>  	if (i != vlen) {
>>  		btf_loc_param_raw_str(p, vlen, str, sz);
> 
> 
> Thanks Alan!
> 
> bpf-ci's comment about "-const 0x16" instead of "const -0x16" seems
> legit, please take a look.
>

Yep, will fix, thanks!
 
> 
>> @@ -252,6 +255,87 @@ static void btf_loc_param_str(const struct btf_type *t, char *str, size_t sz)
>>  		 p->flags & BTF_LOC_PARAM_DEREF ? ")" : "");
>>  }
>>  
>> +static int btf_locsec_append(char *str, size_t sz, size_t *off,
>> +			      const char *fmt, ...)
>> +{
>> +	va_list args;
>> +	int ret;
>> +
>> +	if (!sz || *off >= sz - 1)
>> +		return -ENOSPC;
>> +
>> +	va_start(args, fmt);
>> +	ret = vsnprintf(str + *off, sz - *off, fmt, args);
> 
> 
> Nit: Do you really need vsnprintf()? It looks like you always
> concatenate, never format any number, so it's probably not the most
> efficient. I don't mind much, though.
>

Sure, I'll take a look, might require a few extra append()s but would
probably be simpler overall.
 
> 
>> +	va_end(args);
>> +	if (ret < 0 || (size_t)ret >= sz - *off) {
>> +		*off = sz - 1;
>> +		return -ENOSPC;
> 
> 
> It seems unlikely we'll hit this, but maybe warn that the string is
> truncated in that case, or replace the last characters with "..." or
> "[truncated]" or something like this??
> 

yeah we could replace last few chars with ...

> 
>> +	}
>> +	*off += ret;
>> +	return 0;
>> +}
>> +
>> +static void btf_locsec_func_str(const struct btf *btf,
>> +				const struct btf_loc *loc, char *str, size_t sz)
>> +{
>> +	const struct btf_type *func, *func_proto, *loc_proto;
>> +	const struct btf_param *params;
>> +	const __u32 *loc_params;
>> +	const char *name;
>> +	__u32 i, vlen;
>> +	size_t off = 0;
>> +
>> +	if (!sz)
>> +		return;
>> +
>> +	str[0] = '\0';
>> +	func = btf__type_by_id(btf, loc->func);
>> +	if (!func || !btf_is_func(func))
>> +		goto invalid;
>> +
>> +	name = btf_str(btf, func->name_off);
>> +	func_proto = btf__type_by_id(btf, func->type);
>> +	loc_proto = btf__type_by_id(btf, loc->loc_proto);
>> +	if (!func_proto || !btf_is_func_proto(func_proto) ||
>> +	    !loc_proto || !btf_is_loc_proto(loc_proto) ||
>> +	    btf_vlen(func_proto) != btf_vlen(loc_proto))
>> +		goto invalid;
>> +
>> +	params = (const void *)(func_proto + 1);
>> +	loc_params = btf_loc_proto_params(loc_proto);
>> +	vlen = btf_vlen(func_proto);
>> +	if (btf_locsec_append(str, sz, &off, "%s(", name))
>> +		return;
>> +	for (i = 0; i < vlen; i++) {
>> +		const struct btf_type *param_loc;
>> +		char param_str[256] = {};
>> +
>> +		if (!params[i].type) {
>> +			/* Handle varargs func proto, must be last parameter */
>> +			if (i != vlen - 1)
>> +				goto invalid;
>> +			if (btf_locsec_append(str, sz, &off, "%s...", i ? ", " : ""))
>> +				return;
>> +			break;
>> +		} else if (loc_params[i]) {
>> +			param_loc = btf__type_by_id(btf, loc_params[i]);
>> +			btf_loc_param_str(param_loc, param_str, sizeof(param_str));
>> +		} else {
>> +			snprintf(param_str, sizeof(param_str), "<unavailable>");
>> +		}
>> +
>> +		if (btf_locsec_append(str, sz, &off, "%s%s [%s]",
>> +				      i ? ", " : "", btf_str(btf, params[i].name_off),
>> +				      param_str))
>> +			return;
>> +	}
>> +	(void) btf_locsec_append(str, sz, &off, ")");
>> +	return;
>> +
>> +invalid:
>> +	snprintf(str, sz, "<invalid>");
>> +}
>> +
>>  static int dump_btf_type(const struct btf *btf, __u32 id,
>>  			 const struct btf_type *t)
>>  {
>> @@ -617,22 +701,20 @@ static int dump_btf_type(const struct btf *btf, __u32 id,
>>  		}
>>  
>>  		for (i = 0; i < vlen; i++, locs++) {
>> -			const struct btf_type *f = btf__type_by_id(btf, locs->func);
>> -			const char *name = "<invalid>";
>> +			char func_str[1024] = {};
>>  
>> -			if (f && btf_is_func(f))
>> -				name = btf_str(btf, f->name_off);
>> +			btf_locsec_func_str(btf, locs, func_str, sizeof(func_str));
>>  
>>  			if (json_output) {
>>  				jsonw_start_object(w);
>>  				jsonw_uint_field(w, "func_type_id", locs->func);
>> -				jsonw_string_field(w, "name", name);
>> +				jsonw_string_field(w, "func", func_str);
> 
> 
> Would it be worth keeping the name, too, in the JSON? So that if
> somebody wants the name only, they don't have to parse it from "func"?
>

good idea, will do.
 
> 
>>  				jsonw_uint_field(w, "loc_proto_type_id", locs->loc_proto);
>>  				jsonw_uint_field(w, "offset", locs->offset);
>>  				jsonw_end_object(w);
>>  			} else {
>> -				printf("\n\tname='%s' func_type_id=%u loc_proto_type_id=%u offset=%u",
>> -				       name, locs->func, locs->loc_proto, locs->offset);
>> +				printf("\n\tfunc='%s' func_type_id=%u loc_proto_type_id=%u offset=%u",
>> +				       func_str, locs->func, locs->loc_proto, locs->offset);
>>  			}
>>  		}
>>  		if (json_output)
> 

[PATCH bpf-next 3/3] selftests/bpf: Fix up bpftool btf dump test for signatures

[Alan Maguire]

Now we show a function signature, fix up the test to expect it.
Also fix the fact that we did not add the right number of parameters
to the FUNC_PROTO, and ensure consts/addresses are prefixed
appropriately.

Fixes: 321562c34d5b ("bpftool: Add ability to dump LOC_PARAM, LOC_PROTO and LOCSEC")
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 .../selftests/bpf/prog_tests/bpftool_btf_dump.c    | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/bpftool_btf_dump.c b/tools/testing/selftests/bpf/prog_tests/bpftool_btf_dump.c
index bed506badc75..abc62958b6ed 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpftool_btf_dump.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpftool_btf_dump.c
@@ -72,6 +72,7 @@ static struct btf *mk_loc_btf(void)
 	btf__add_func_param(btf, "arg3", 1);
 	btf__add_func_param(btf, "arg4", 1);
 	btf__add_func_param(btf, "arg5", 1);
+	btf__add_func_param(btf, "arg6", 1);
 	btf__add_func(btf, "foo", BTF_FUNC_STATIC, 2);
 
 	btf__add_loc_param(btf, 4, BTF_LOC_PARAM_REG);
@@ -230,28 +231,29 @@ static void test_loc_dump(const char *btf_path)
 {
 	const char expected[] =
 		"[1] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED\n"
-		"[2] FUNC_PROTO '(anon)' ret_type_id=1 vlen=5\n"
+		"[2] FUNC_PROTO '(anon)' ret_type_id=1 vlen=6\n"
 		"\t'arg1' type_id=1\n"
 		"\t'arg2' type_id=1\n"
 		"\t'arg3' type_id=1\n"
 		"\t'arg4' type_id=1\n"
 		"\t'arg5' type_id=1\n"
+		"\t'arg6' type_id=1\n"
 		"[3] FUNC 'foo' type_id=2 linkage=static\n"
 		"[4] LOC_PARAM '(anon)' size=4 flags=0x8 vlen=1 values='reg1'\n"
 		"[5] LOC_PARAM '(anon)' size=8 flags=0x38 vlen=2 values='*(reg2 + 0x10)'\n"
 		"[6] LOC_PARAM '(anon)' size=8 flags=0x29 vlen=2 values='fbreg - 0x10'\n"
-		"[7] LOC_PARAM '(anon)' size=8 flags=0x6 vlen=2 values='0x123456789abcdef0 (addr)'\n"
-		"[8] LOC_PARAM '(anon)' size=8 flags=0x2 vlen=2 values='0xdeadbeeffeedface'\n"
+		"[7] LOC_PARAM '(anon)' size=8 flags=0x6 vlen=2 values='address 0x123456789abcdef0'\n"
+		"[8] LOC_PARAM '(anon)' size=8 flags=0x2 vlen=2 values='const 0xdeadbeeffeedface'\n"
 		"[9] LOC_PARAM '(anon)' size=8 flags=0x39 vlen=2 values='*(fbreg - 0x20)'\n"
 		"[10] LOC_PROTO '(anon)' vlen=6\n"
 		"\ttype_id=4 value='reg1'\n"
 		"\ttype_id=5 value='*(reg2 + 0x10)'\n"
 		"\ttype_id=6 value='fbreg - 0x10'\n"
-		"\ttype_id=7 value='0x123456789abcdef0 (addr)'\n"
-		"\ttype_id=8 value='0xdeadbeeffeedface'\n"
+		"\ttype_id=7 value='address 0x123456789abcdef0'\n"
+		"\ttype_id=8 value='const 0xdeadbeeffeedface'\n"
 		"\ttype_id=9 value='*(fbreg - 0x20)'\n"
 		"[11] LOCSEC 'inline.text' vlen=1\n"
-		"\tname='foo' func_type_id=3 loc_proto_type_id=10 offset=64\n";
+		"\tfunc='foo(arg1 [reg1], arg2 [*(reg2 + 0x10)], arg3 [fbreg - 0x10], arg4 [address 0x123456789abcdef0], arg5 [const 0xdeadbeeffeedface], arg6 [*(fbreg - 0x20)])' func_type_id=3 loc_proto_type_id=10 offset=64\n";
 	char *dump;
 
 	dump = dump_raw(btf_path);
-- 
2.43.5

Re: [PATCH bpf-next 3/3] selftests/bpf: Fix up bpftool btf dump test for signatures

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]

> selftests/bpf: Fix up bpftool btf dump test for signatures
>
> Now we show a function signature, fix up the test to expect it.
> Also fix the fact that we did not add the right number of parameters
> to the FUNC_PROTO, and ensure consts/addresses are prefixed
> appropriately.
>
> Fixes: 321562c34d5b ("bpftool: Add ability to dump LOC_PARAM, LOC_PROTO and LOCSEC")
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>

Does the Fixes: tag point to the right commit? The commit message
references 321562c34d5b ("bpftool: Add ability to dump LOC_PARAM,
LOC_PROTO and LOCSEC"), but that commit only added the bpftool feature
and did not include the test file being fixed here.

The bug being fixed (incorrect FUNC_PROTO vlen=5 when it should be
vlen=6 to match the 6 LOC_PARAMs) was actually introduced in commit
3429e01578b6 ("selftests/bpf: Test bpftool dump of BTF location info"),
which added the test with only 5 function parameters (arg1-arg5) but
6 LOC_PARAM entries.

Should the Fixes: tag be:

  Fixes: 3429e01578b6 ("selftests/bpf: Test bpftool dump of BTF location info")


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/36122090541