All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH perf-tools-next 0/3] perf trace: Symbolise kernel virtual addresses and function pointers
@ 2026-08-15 23:36 Aaron Tomlin
  2026-08-15 23:36 ` [PATCH perf-tools-next 1/3] perf trace: Introduce kernel symbol beautifier for virtual addresses Aaron Tomlin
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Aaron Tomlin @ 2026-08-15 23:36 UTC (permalink / raw)
  To: peterz, mingo, acme, namhyung
  Cc: mark.rutland, alexander.shishkin, jolsa, irogers, adrian.hunter,
	james.clark, howardchu95, atomlin, neelx, chjohnst, sean, steve,
	rishil1999, linux-perf-users, linux-kernel

Hi Namhyung, Arnaldo, Ian,

This has been rebased against perf-tools-next/perf-tools-next.
Kindly ignore the previous submission [1].

[1]: https://lore.kernel.org/lkml/20260803210842.245789-1-atomlin@atomlin.com/#r

When inspecting kernel execution flows using perf trace (e.g., when
monitoring workqueues delayed work items, timer callbacks, etc.),
tracepoint payload arguments containing raw kernel virtual addresses are
currently rendered as hexadecimal values (e.g., 0xffffffff81234567).

This requires manual symbol lookups against /proc/kallsyms or vmlinux to
identify the underlying kernel function being executed.

This patch series enhances perf trace by introducing kernel virtual
address and function pointer symbolisation using perfs native symbol
engine (i.e., machine__find_kernel_symbol()).

    Before:
      workqueue:workqueue_execute_end(work: 0xffffffffab2f1420, function: 0xffffffffa8046b50)

    After:
      workqueue:workqueue_execute_end(work: 0xffff8ac2c420f270, function: wb_update_bandwidth_workfn)

Patch 1 adds the syscall_arg__scnprintf_ksym (SCA_KSYM) beautifier
which resolves virtual addresses via machine__find_kernel_symbol(),
formatting them as symbol_name+offset (or "NULL" with a hex fallback).

Patch 2 updates event format initialisation in
syscall_arg_fmt__init_array() to automatically assign SCA_KSYM to
tracepoint fields named function, fn, work, action, or callsite,
as well as fields typed as function pointers.

Patch 3 extends BTF pretty-printing in trace__btf_scnprintf() with
btf_is_func_ptr() to automatically detect BTF function prototypes and
symbolise kernel function pointers without requiring manual field table
configuration.

Aaron Tomlin (3):
  perf trace: Introduce kernel symbol beautifier for virtual addresses
  perf trace: Auto-assign kernel symbol beautifier to function pointer
    fields
  perf trace: Enhance BTF type formatting to symbolise kernel function
    pointers

 tools/perf/builtin-trace.c       | 76 ++++++++++++++++++++++++++++----
 tools/perf/trace/beauty/beauty.h |  3 ++
 2 files changed, 71 insertions(+), 8 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH perf-tools-next 1/3] perf trace: Introduce kernel symbol beautifier for virtual addresses
  2026-08-15 23:36 [PATCH perf-tools-next 0/3] perf trace: Symbolise kernel virtual addresses and function pointers Aaron Tomlin
@ 2026-08-15 23:36 ` Aaron Tomlin
  2026-08-15 23:44   ` sashiko-bot
  2026-08-15 23:36 ` [PATCH perf-tools-next 2/3] perf trace: Auto-assign kernel symbol beautifier to function pointer fields Aaron Tomlin
  2026-08-15 23:36 ` [PATCH perf-tools-next 3/3] perf trace: Enhance BTF type formatting to symbolise kernel function pointers Aaron Tomlin
  2 siblings, 1 reply; 8+ messages in thread
From: Aaron Tomlin @ 2026-08-15 23:36 UTC (permalink / raw)
  To: peterz, mingo, acme, namhyung
  Cc: mark.rutland, alexander.shishkin, jolsa, irogers, adrian.hunter,
	james.clark, howardchu95, atomlin, neelx, chjohnst, sean, steve,
	rishil1999, linux-perf-users, linux-kernel

Currently, when perf trace formats tracepoint payloads or system call
arguments containing raw kernel virtual addresses (e.g., a work item
function pointer work_func_t in workqueue:workqueue_execute_start),
it prints them as raw hexadecimal values (e.g., 0xffffffff81234567).
This impairs readability when tracing kernel execution flows.

Introduce a dedicated kernel symbol beautifier,
syscall_arg__scnprintf_ksym (i.e., SCA_KSYM), to resolve kernel
virtual addresses to human-readable symbol names and offsets
(e.g., flush_to_ldisc).

The beautifier looks up the virtual address in the machine kernel maps via
machine__find_kernel_symbol(). If a valid kernel symbol is found, the
symbol name and offset are printed without requiring --libtraceevent; if
the address is zero, "NULL" is rendered; otherwise, it gracefully falls
back to hexadecimal formatting.

Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 tools/perf/builtin-trace.c       | 24 ++++++++++++++++++++++++
 tools/perf/trace/beauty/beauty.h |  3 +++
 2 files changed, 27 insertions(+)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index c3c7f1f85c53..102221b301a5 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -750,6 +750,30 @@ size_t syscall_arg__scnprintf_ptr(char *bf, size_t size, struct syscall_arg *arg
 	return syscall_arg__scnprintf_hex(bf, size, arg);
 }
 
+size_t syscall_arg__scnprintf_ksym(char *bf, size_t size, struct syscall_arg *arg)
+{
+	if (arg->val == 0)
+		return scnprintf(bf, size, "NULL");
+
+	if (arg->trace && arg->trace->host) {
+		struct map *map;
+		struct symbol *sym = machine__find_kernel_symbol(arg->trace->host,
+								 arg->val, &map);
+
+		if (sym) {
+			u64 start = map__unmap_ip(map, sym->start);
+			u64 offset = arg->val - start;
+
+			if (offset == 0)
+				return scnprintf(bf, size, "%s", sym->name);
+			return scnprintf(bf, size, "%s+0x%" PRIx64,
+					 sym->name, offset);
+		}
+	}
+
+	return syscall_arg__scnprintf_hex(bf, size, arg);
+}
+
 size_t syscall_arg__scnprintf_int(char *bf, size_t size, struct syscall_arg *arg)
 {
 	return scnprintf(bf, size, "%d", arg->val);
diff --git a/tools/perf/trace/beauty/beauty.h b/tools/perf/trace/beauty/beauty.h
index 58a3206481ae..0f4801c61a5b 100644
--- a/tools/perf/trace/beauty/beauty.h
+++ b/tools/perf/trace/beauty/beauty.h
@@ -160,6 +160,9 @@ size_t syscall_arg__scnprintf_hex(char *bf, size_t size, struct syscall_arg *arg
 size_t syscall_arg__scnprintf_ptr(char *bf, size_t size, struct syscall_arg *arg);
 #define SCA_PTR syscall_arg__scnprintf_ptr
 
+size_t syscall_arg__scnprintf_ksym(char *bf, size_t size, struct syscall_arg *arg);
+#define SCA_KSYM syscall_arg__scnprintf_ksym
+
 size_t syscall_arg__scnprintf_int(char *bf, size_t size, struct syscall_arg *arg);
 #define SCA_INT syscall_arg__scnprintf_int
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH perf-tools-next 2/3] perf trace: Auto-assign kernel symbol beautifier to function pointer fields
  2026-08-15 23:36 [PATCH perf-tools-next 0/3] perf trace: Symbolise kernel virtual addresses and function pointers Aaron Tomlin
  2026-08-15 23:36 ` [PATCH perf-tools-next 1/3] perf trace: Introduce kernel symbol beautifier for virtual addresses Aaron Tomlin
@ 2026-08-15 23:36 ` Aaron Tomlin
  2026-08-15 23:47   ` sashiko-bot
  2026-08-15 23:36 ` [PATCH perf-tools-next 3/3] perf trace: Enhance BTF type formatting to symbolise kernel function pointers Aaron Tomlin
  2 siblings, 1 reply; 8+ messages in thread
From: Aaron Tomlin @ 2026-08-15 23:36 UTC (permalink / raw)
  To: peterz, mingo, acme, namhyung
  Cc: mark.rutland, alexander.shishkin, jolsa, irogers, adrian.hunter,
	james.clark, howardchu95, atomlin, neelx, chjohnst, sean, steve,
	rishil1999, linux-perf-users, linux-kernel

Tracepoint fields that convey kernel function pointers, such as function,
fn, work, action, and callsite are currently formatted as generic
hexadecimal pointers by default.

Enhance syscall_arg_fmt__init_array() to automatically detect these
fields by name and type signature (e.g., typedefs ending with _func_t
or _fn, or C function pointer types containing (*)) and assign
SCA_KSYM as their default beautifier.

Additionally, register common function pointer field names within the
sorted syscall_arg_fmts__by_name lookup table. This ensures tracepoint
arguments such as workqueue:workqueue_execute_start.function are
symbolised automatically without requiring explicit per-event
configuration. For example:

    ❯ sudo tools/perf/perf trace --event workqueue:workqueue_execute_end --max-events 2 --show-cpu
         0.000 [000] kworker/u32:15/236682 workqueue:workqueue_execute_end(work: 0xffffffffab2f1420, function: toggle_allocation_gate)
         0.132 [000] kworker/u32:15/236682 workqueue:workqueue_execute_end(work: 0xffff8ac2c1adc010, function: flush_to_ldisc)

Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 tools/perf/builtin-trace.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 102221b301a5..825b16581581 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2091,8 +2091,13 @@ static int syscall__alloc_arg_fmts(struct syscall *sc, int nr_args)
 }
 
 static const struct syscall_arg_fmt syscall_arg_fmts__by_name[] = {
+	{ .name = "action",	.scnprintf = SCA_KSYM, },
+	{ .name = "callsite",	.scnprintf = SCA_KSYM, },
+	{ .name = "fn",		.scnprintf = SCA_KSYM, },
+	{ .name = "function",	.scnprintf = SCA_KSYM, },
 	{ .name = "msr",	.scnprintf = SCA_X86_MSR,	  .strtoul = STUL_X86_MSR,	   },
 	{ .name = "vector",	.scnprintf = SCA_X86_IRQ_VECTORS, .strtoul = STUL_X86_IRQ_VECTORS, },
+	{ .name = "work",	.scnprintf = SCA_KSYM, },
 };
 
 static int syscall_arg_fmt__cmp(const void *name, const void *fmtp)
@@ -2193,6 +2198,14 @@ syscall_arg_fmt__init_array(struct syscall_arg_fmt *arg, struct tep_format_field
 		    ((len >= 4 && strcmp(field->name + len - 4, "name") == 0) ||
 		     strstr(field->name, "path") != NULL)) {
 			arg->scnprintf = SCA_FILENAME;
+		} else if (strcmp(field->name, "fn") == 0 ||
+			   strcmp(field->name, "function") == 0 ||
+			   strcmp(field->name, "callsite") == 0 ||
+			   strcmp(field->name, "action") == 0 ||
+			   (field->type && (strstr(field->type, "(*)") != NULL ||
+					    strstr(field->type, "_func_t") != NULL ||
+					    strstr(field->type, "_fn") != NULL))) {
+			arg->scnprintf = SCA_KSYM;
 		} else if ((field->flags & TEP_FIELD_IS_POINTER) || strstr(field->name, "addr") ||
 			   field_has_hex_fmt(field, len))
 			arg->scnprintf = SCA_PTR;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH perf-tools-next 3/3] perf trace: Enhance BTF type formatting to symbolise kernel function pointers
  2026-08-15 23:36 [PATCH perf-tools-next 0/3] perf trace: Symbolise kernel virtual addresses and function pointers Aaron Tomlin
  2026-08-15 23:36 ` [PATCH perf-tools-next 1/3] perf trace: Introduce kernel symbol beautifier for virtual addresses Aaron Tomlin
  2026-08-15 23:36 ` [PATCH perf-tools-next 2/3] perf trace: Auto-assign kernel symbol beautifier to function pointer fields Aaron Tomlin
@ 2026-08-15 23:36 ` Aaron Tomlin
  2026-08-15 23:50   ` sashiko-bot
  2 siblings, 1 reply; 8+ messages in thread
From: Aaron Tomlin @ 2026-08-15 23:36 UTC (permalink / raw)
  To: peterz, mingo, acme, namhyung
  Cc: mark.rutland, alexander.shishkin, jolsa, irogers, adrian.hunter,
	james.clark, howardchu95, atomlin, neelx, chjohnst, sean, steve,
	rishil1999, linux-perf-users, linux-kernel

When BTF (BPF Type Format) metadata is loaded from vmlinux, perf trace can
inspect the precise C types of tracepoint and system call parameters.
However, function pointer arguments are currently not recognised during BTF
pretty-printing and default to hexadecimal output.

Introduce btf_is_func_ptr() to inspect BTF type hierarchies
(i.e., traversing pointers, typedefs, and type modifiers) to determine
whether a parameter resolves to a function prototype (BTF_KIND_FUNC_PROTO).

Generalise BTF type caching via syscall_arg_fmt__cache_btf_type() to handle
structs, unions, enums, and function pointers alike. When a field is
identified as a kernel function pointer, trace__btf_scnprintf() routes its
value to syscall_arg__scnprintf_ksym(), enabling automatic zero-config
symbolisation of kernel function pointers whenever BTF is available.

Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 tools/perf/builtin-trace.c | 39 ++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 825b16581581..94dc720a2b3a 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -974,21 +974,43 @@ static size_t syscall_arg__scnprintf_getrandom_flags(char *bf, size_t size,
 #define SCA_GETRANDOM_FLAGS syscall_arg__scnprintf_getrandom_flags
 
 #ifdef HAVE_LIBBPF_SUPPORT
-static void syscall_arg_fmt__cache_btf_enum(struct syscall_arg_fmt *arg_fmt, struct btf *btf, char *type)
+static bool btf_is_func_ptr(const struct btf *btf, const struct btf_type *type)
+{
+	while (type) {
+		if (btf_is_ptr(type)) {
+			type = btf__type_by_id(btf, type->type);
+			return type && btf_is_func_proto(type);
+		}
+		if (btf_is_typedef(type) || btf_is_mod(type)) {
+			type = btf__type_by_id(btf, type->type);
+		} else {
+			break;
+		}
+	}
+	return false;
+}
+
+static void syscall_arg_fmt__cache_btf_type(struct syscall_arg_fmt *arg_fmt,
+					    struct btf *btf, char *type)
 {
 	int id;
 
-	type = strstr(type, "enum ");
 	if (type == NULL)
 		return;
 
-	type += 5; // skip "enum " to get the enumeration name
+	if (strstarts(type, "enum "))
+		type += 5;
+	else if (strstarts(type, "struct "))
+		type += 7;
+	else if (strstarts(type, "union "))
+		type += 6;
 
 	id = btf__find_by_name(btf, type);
 	if (id < 0)
 		return;
 
 	arg_fmt->type = btf__type_by_id(btf, id);
+	arg_fmt->type_id = id;
 }
 
 static bool syscall_arg__strtoul_btf_enum(char *bf, size_t size, struct syscall_arg *arg, u64 *val)
@@ -1023,8 +1045,7 @@ static bool syscall_arg__strtoul_btf_type(char *bf, size_t size, struct syscall_
 		return false;
 
 	if (arg->fmt->type == NULL) {
-		// See if this is an enum
-		syscall_arg_fmt__cache_btf_enum(arg->fmt, btf, type);
+		syscall_arg_fmt__cache_btf_type(arg->fmt, btf, type);
 	}
 
 	// Now let's see if we have a BTF type resolved
@@ -1111,8 +1132,7 @@ static size_t trace__btf_scnprintf(struct trace *trace, struct syscall_arg *arg,
 		return 0;
 
 	if (arg_fmt->type == NULL) {
-		// Check if this is an enum and if we have the BTF type for it.
-		syscall_arg_fmt__cache_btf_enum(arg_fmt, trace->btf, type);
+		syscall_arg_fmt__cache_btf_type(arg_fmt, trace->btf, type);
 	}
 
 	// Did we manage to find a BTF type for the syscall/tracepoint argument?
@@ -1123,6 +1143,8 @@ static size_t trace__btf_scnprintf(struct trace *trace, struct syscall_arg *arg,
 		return btf_enum_scnprintf(arg_fmt->type, trace->btf, bf, size, val);
 	else if (btf_is_struct(arg_fmt->type) || btf_is_union(arg_fmt->type))
 		return btf_struct_scnprintf(arg_fmt->type, trace->btf, bf, size, arg);
+	else if (btf_is_func_ptr(trace->btf, arg_fmt->type))
+		return syscall_arg__scnprintf_ksym(bf, size, arg);
 
 	return 0;
 }
@@ -2566,7 +2588,8 @@ static size_t syscall__scnprintf_args(struct syscall *sc, char *bf, size_t size,
 
 			default_scnprintf = sc->arg_fmt[arg.idx].scnprintf;
 
-			if (trace->force_btf || default_scnprintf == NULL || default_scnprintf == SCA_PTR) {
+			if (trace->force_btf || default_scnprintf == NULL ||
+			    default_scnprintf == SCA_PTR || default_scnprintf == SCA_KSYM) {
 				btf_printed = trace__btf_scnprintf(trace, &arg, bf + printed,
 								   size - printed, val, field->type);
 				if (btf_printed) {
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH perf-tools-next 1/3] perf trace: Introduce kernel symbol beautifier for virtual addresses
  2026-08-15 23:36 ` [PATCH perf-tools-next 1/3] perf trace: Introduce kernel symbol beautifier for virtual addresses Aaron Tomlin
@ 2026-08-15 23:44   ` sashiko-bot
  2026-08-16  1:50     ` Aaron Tomlin
  0 siblings, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-08-15 23:44 UTC (permalink / raw)
  To: Aaron Tomlin; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Memory leak of reference-counted `struct map` handle in `syscall_arg__scnprintf_ksym`.
--

--- Patch [1]: [PATCH perf-tools-next 1/3] perf trace: Introduce kernel symbol beautifier for virtual addresses ---
commit 0ff39d14d9508d5e3c586057a78b07cbcda7f7fb
Author: Aaron Tomlin <atomlin@atomlin.com>

perf trace: Introduce kernel symbol beautifier for virtual addresses

This commit adds a kernel symbol beautifier for perf trace to translate
raw kernel virtual addresses into human-readable symbol names and offsets.
It uses machine__find_kernel_symbol() to look up the symbols dynamically.

> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index c3c7f1f85c530..102221b301a54 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c

[ ... ]

> @@ -750,6 +750,30 @@ size_t syscall_arg__scnprintf_ptr(char *bf, size_t size, struct syscall_arg *arg
>  	return syscall_arg__scnprintf_hex(bf, size, arg);
>  }
>  
> +size_t syscall_arg__scnprintf_ksym(char *bf, size_t size, struct syscall_arg *arg)
> +{
> +	if (arg->val == 0)
> +		return scnprintf(bf, size, "NULL");
> +
> +	if (arg->trace && arg->trace->host) {
> +		struct map *map;
> +		struct symbol *sym = machine__find_kernel_symbol(arg->trace->host,
> +								 arg->val, &map);
> +
> +		if (sym) {
> +			u64 start = map__unmap_ip(map, sym->start);
> +			u64 offset = arg->val - start;
> +
> +			if (offset == 0)
> +				return scnprintf(bf, size, "%s", sym->name);
> +			return scnprintf(bf, size, "%s+0x%" PRIx64,
> +					 sym->name, offset);

[Severity: High]
Does this code leak the struct map reference?

When machine__find_kernel_symbol() populates the map pointer, it acquires a
reference that the caller takes ownership of.

However, this function returns directly on both success paths and the fallback
path without calling map__put(map). This will cause a memory leak for every
formatted event that resolves to a valid map, which can quickly lead to memory
exhaustion or LSAN crashes when running perf trace.

> +		}
> +	}
> +
> +	return syscall_arg__scnprintf_hex(bf, size, arg);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815233651.527936-1-atomlin@atomlin.com?part=1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH perf-tools-next 2/3] perf trace: Auto-assign kernel symbol beautifier to function pointer fields
  2026-08-15 23:36 ` [PATCH perf-tools-next 2/3] perf trace: Auto-assign kernel symbol beautifier to function pointer fields Aaron Tomlin
@ 2026-08-15 23:47   ` sashiko-bot
  0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-08-15 23:47 UTC (permalink / raw)
  To: Aaron Tomlin; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Overly broad name-matching heuristics in `syscall_arg_fmt__init_array()` unconditionally intercept non-function pointer fields, corrupting trace output.
- [Medium] Handling of the `work` field fails to symbolize pointers as intended while corrupting unrelated non-pointer integer fields.
- [Low] Redundant entries added to the `syscall_arg_fmts__by_name` lookup table are completely unreachable dead code.
--

--- Patch [2]: [PATCH perf-tools-next 2/3] perf trace: Auto-assign kernel symbol beautifier to function pointer fields ---
commit e41ca802a20df2b74611c38302bea036b4cc9f1f
Author: Aaron Tomlin <atomlin@atomlin.com>

perf trace: Auto-assign kernel symbol beautifier to function pointer fields

This patch enhances syscall_arg_fmt__init_array() to detect kernel function
pointers and assign SCA_KSYM as their default beautifier. It also registers
common function pointer field names in the syscall_arg_fmts__by_name lookup
table to automatically symbolize them.

> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 102221b301a54..825b16581581b 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -2091,8 +2091,13 @@ static int syscall__alloc_arg_fmts(struct syscall *sc, int nr_args)
>  }
>  
>  static const struct syscall_arg_fmt syscall_arg_fmts__by_name[] = {
> +	{ .name = "action",	.scnprintf = SCA_KSYM, },
> +	{ .name = "callsite",	.scnprintf = SCA_KSYM, },
> +	{ .name = "fn",		.scnprintf = SCA_KSYM, },
> +	{ .name = "function",	.scnprintf = SCA_KSYM, },

[Severity: Low]
Are these lookup table entries reachable? 

In syscall_arg_fmt__init_array(), there is an else-if block added by this
patch that explicitly intercepts these exact field names. It appears these
lookup table entries would never be evaluated for these strings.

>  	{ .name = "msr",	.scnprintf = SCA_X86_MSR,	  .strtoul = STUL_X86_MSR,	   },
>  	{ .name = "vector",	.scnprintf = SCA_X86_IRQ_VECTORS, .strtoul = STUL_X86_IRQ_VECTORS, },
> +	{ .name = "work",	.scnprintf = SCA_KSYM, },

[Severity: Medium]
Does this correctly symbolize work pointers, and could it inadvertently
corrupt unrelated non-pointer integer fields?

For workqueue tracepoints where the work field is a pointer, it appears it
will be intercepted earlier in syscall_arg_fmt__init_array() by the
(field->flags & TEP_FIELD_IS_POINTER) check. That check assigns SCA_PTR 
instead of allowing the field to reach this lookup table.

Conversely, for tracepoints like napi_poll that have a non-pointer integer
field named "work", the pointer check is skipped, allowing it to hit this
lookup table. Wouldn't this assign SCA_KSYM to the integer, causing it to
erroneously print as a hex pointer or NULL instead of a decimal number?

[ ... ]

> @@ -2193,6 +2198,14 @@ syscall_arg_fmt__init_array(struct syscall_arg_fmt *arg, struct tep_format_field
>  		    ((len >= 4 && strcmp(field->name + len - 4, "name") == 0) ||
>  		     strstr(field->name, "path") != NULL)) {
>  			arg->scnprintf = SCA_FILENAME;
> +		} else if (strcmp(field->name, "fn") == 0 ||
> +			   strcmp(field->name, "function") == 0 ||
> +			   strcmp(field->name, "callsite") == 0 ||
> +			   strcmp(field->name, "action") == 0 ||
> +			   (field->type && (strstr(field->type, "(*)") != NULL ||
> +					    strstr(field->type, "_func_t") != NULL ||
> +					    strstr(field->type, "_fn") != NULL))) {
> +			arg->scnprintf = SCA_KSYM;

[Severity: Medium]
Does this unconditional name matching affect non-pointer integer fields
that happen to share these generic names? 

If a tracepoint has an integer field named "action" or "function" (which
can occur in subsystems like btrfs, power, or xfs), wouldn't this assign
SCA_KSYM to it regardless of its actual type? 

This might cause perf trace to treat simple integers as kernel addresses,
failing to resolve a symbol and formatting them incorrectly.

>  		} else if ((field->flags & TEP_FIELD_IS_POINTER) || strstr(field->name, "addr") ||
>  			   field_has_hex_fmt(field, len))
>  			arg->scnprintf = SCA_PTR;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815233651.527936-1-atomlin@atomlin.com?part=2

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH perf-tools-next 3/3] perf trace: Enhance BTF type formatting to symbolise kernel function pointers
  2026-08-15 23:36 ` [PATCH perf-tools-next 3/3] perf trace: Enhance BTF type formatting to symbolise kernel function pointers Aaron Tomlin
@ 2026-08-15 23:50   ` sashiko-bot
  0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-08-15 23:50 UTC (permalink / raw)
  To: Aaron Tomlin; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] `trace__btf_scnprintf()` calls `syscall_arg__scnprintf_ksym()` without `arg->val` being updated, causing kernel function pointers to unconditionally print as "NULL" or display incorrect symbols.
- [Medium] `syscall_arg_fmt__cache_btf_type()` attempts to look up BTF types using raw string matching, which systematically fails for pointer declarations (e.g., `struct task_struct *`).
- [Medium] Replacing `strstr()` with `strstarts()` in `syscall_arg_fmt__cache_btf_type()` breaks BTF type resolution for type strings containing modifiers (e.g., `"const enum"`).
- [Medium] `btf_is_func_ptr()` improperly handles pointers targeting typedefs or modifiers.
--

--- Patch [3]: [PATCH perf-tools-next 3/3] perf trace: Enhance BTF type formatting to symbolise kernel function pointers ---
commit 53acf94abe39f745266994a52d3b39a3f7fa20c2
Author: Aaron Tomlin <atomlin@atomlin.com>

perf trace: Enhance BTF type formatting to symbolise kernel function pointers
    
When BTF metadata is loaded from vmlinux, this commit introduces
btf_is_func_ptr() to identify function prototypes. It generalizes BTF type
caching to handle structs, unions, enums, and function pointers. It also
adds routing to syscall_arg__scnprintf_ksym() to symbolize kernel function
pointers.

> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 825b16581581b..94dc720a2b3ac 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -974,21 +974,43 @@ static size_t syscall_arg__scnprintf_getrandom_flags(char *bf, size_t size,
>  #define SCA_GETRANDOM_FLAGS syscall_arg__scnprintf_getrandom_flags
>  
>  #ifdef HAVE_LIBBPF_SUPPORT
> -static void syscall_arg_fmt__cache_btf_enum(struct syscall_arg_fmt *arg_fmt, struct btf *btf, char *type)
> +static bool btf_is_func_ptr(const struct btf *btf, const struct btf_type *type)
> +{
> +	while (type) {
> +		if (btf_is_ptr(type)) {
> +			type = btf__type_by_id(btf, type->type);
> +			return type && btf_is_func_proto(type);

[Severity: Medium]
Will this return prematurely without unwrapping typedefs or modifiers on the
pointer's target type? 

If a function pointer is defined via a typedef (for example, typedef void
vq_callback_t(struct virtqueue *); vq_callback_t *cb;), does this unwrap
the pointer kind and immediately return false because it doesn't evaluate
subsequent modifiers on the function prototype?

> +		}
> +		if (btf_is_typedef(type) || btf_is_mod(type)) {
> +			type = btf__type_by_id(btf, type->type);
> +		} else {
> +			break;
> +		}
> +	}
> +	return false;
> +}
> +
> +static void syscall_arg_fmt__cache_btf_type(struct syscall_arg_fmt *arg_fmt,
> +					    struct btf *btf, char *type)
>  {
>  	int id;
>  
> -	type = strstr(type, "enum ");
>  	if (type == NULL)
>  		return;
>  
> -	type += 5; // skip "enum " to get the enumeration name
> +	if (strstarts(type, "enum "))
> +		type += 5;

[Severity: Medium]
Does replacing strstr() with strstarts() break BTF type resolution for type
strings containing modifiers? 

If a tracepoint argument is defined as const enum, strstarts() evaluates
from the beginning and returns false, leaving the full string intact and
causing the subsequent lookup to fail.

> +	else if (strstarts(type, "struct "))
> +		type += 7;
> +	else if (strstarts(type, "union "))
> +		type += 6;
>  
>  	id = btf__find_by_name(btf, type);

[Severity: Medium]
Will this raw string matching systematically fail for pointer declarations
like struct task_struct *? 

The code above strips the prefix but leaves the trailing asterisk intact.
Since pointers are anonymous in BTF and structs are named without asterisks,
does this guarantee btf__find_by_name() will fail to find the type?

>  	if (id < 0)
>  		return;
>  
>  	arg_fmt->type = btf__type_by_id(btf, id);
> +	arg_fmt->type_id = id;
>  }

[ ... ]

> @@ -1123,6 +1143,8 @@ static size_t trace__btf_scnprintf(struct trace *trace, struct syscall_arg *arg,
>  		return btf_enum_scnprintf(arg_fmt->type, trace->btf, bf, size, val);
>  	else if (btf_is_struct(arg_fmt->type) || btf_is_union(arg_fmt->type))
>  		return btf_struct_scnprintf(arg_fmt->type, trace->btf, bf, size, arg);
> +	else if (btf_is_func_ptr(trace->btf, arg_fmt->type))
> +		return syscall_arg__scnprintf_ksym(bf, size, arg);
>  
>  	return 0;
>  }

[ ... ]

> @@ -2566,7 +2588,8 @@ static size_t syscall__scnprintf_args(struct syscall *sc, char *bf, size_t size,
>  
>  			default_scnprintf = sc->arg_fmt[arg.idx].scnprintf;
>  
> -			if (trace->force_btf || default_scnprintf == NULL || default_scnprintf == SCA_PTR) {
> +			if (trace->force_btf || default_scnprintf == NULL ||
> +			    default_scnprintf == SCA_PTR || default_scnprintf == SCA_KSYM) {
>  				btf_printed = trace__btf_scnprintf(trace, &arg, bf + printed,
>  								   size - printed, val, field->type);

[Severity: Medium]
When calling trace__btf_scnprintf() here, the syscall argument val is passed
as a separate parameter, but arg.val is not updated to match val.

In trace__btf_scnprintf(), the code delegates to syscall_arg__scnprintf_ksym()
which expects arg->val to contain the valid address. Will this result in an
uninitialized or stale value being used from arg->val, causing the kernel
function pointer to print as NULL or display incorrect symbols?

>  				if (btf_printed) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260815233651.527936-1-atomlin@atomlin.com?part=3

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH perf-tools-next 1/3] perf trace: Introduce kernel symbol beautifier for virtual addresses
  2026-08-15 23:44   ` sashiko-bot
@ 2026-08-16  1:50     ` Aaron Tomlin
  0 siblings, 0 replies; 8+ messages in thread
From: Aaron Tomlin @ 2026-08-16  1:50 UTC (permalink / raw)
  To: namhyung, acme, irogers; +Cc: linux-perf-users

On Sat, Aug 15, 2026 at 11:44:46PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Memory leak of reference-counted `struct map` handle in `syscall_arg__scnprintf_ksym`.
> --
> 
> --- Patch [1]: [PATCH perf-tools-next 1/3] perf trace: Introduce kernel symbol beautifier for virtual addresses ---
> commit 0ff39d14d9508d5e3c586057a78b07cbcda7f7fb
> Author: Aaron Tomlin <atomlin@atomlin.com>
> 
> perf trace: Introduce kernel symbol beautifier for virtual addresses
> 
> This commit adds a kernel symbol beautifier for perf trace to translate
> raw kernel virtual addresses into human-readable symbol names and offsets.
> It uses machine__find_kernel_symbol() to look up the symbols dynamically.
> 
> > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > index c3c7f1f85c530..102221b301a54 100644
> > --- a/tools/perf/builtin-trace.c
> > +++ b/tools/perf/builtin-trace.c
> 
> [ ... ]
> 
> > @@ -750,6 +750,30 @@ size_t syscall_arg__scnprintf_ptr(char *bf, size_t size, struct syscall_arg *arg
> >  	return syscall_arg__scnprintf_hex(bf, size, arg);
> >  }
> >  
> > +size_t syscall_arg__scnprintf_ksym(char *bf, size_t size, struct syscall_arg *arg)
> > +{
> > +	if (arg->val == 0)
> > +		return scnprintf(bf, size, "NULL");
> > +
> > +	if (arg->trace && arg->trace->host) {
> > +		struct map *map;
> > +		struct symbol *sym = machine__find_kernel_symbol(arg->trace->host,
> > +								 arg->val, &map);
> > +
> > +		if (sym) {
> > +			u64 start = map__unmap_ip(map, sym->start);
> > +			u64 offset = arg->val - start;
> > +
> > +			if (offset == 0)
> > +				return scnprintf(bf, size, "%s", sym->name);
> > +			return scnprintf(bf, size, "%s+0x%" PRIx64,
> > +					 sym->name, offset);
> 
> [Severity: High]
> Does this code leak the struct map reference?
> 
> When machine__find_kernel_symbol() populates the map pointer, it acquires a
> reference that the caller takes ownership of.
> 
> However, this function returns directly on both success paths and the fallback
> path without calling map__put(map). This will cause a memory leak for every
> formatted event that resolves to a valid map, which can quickly lead to memory
> exhaustion or LSAN crashes when running perf trace.

Yes, the code feedback is completely accurate. The implementation leaks a
reference to struct map.

-- 
Aaron Tomlin

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-08-16  1:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-15 23:36 [PATCH perf-tools-next 0/3] perf trace: Symbolise kernel virtual addresses and function pointers Aaron Tomlin
2026-08-15 23:36 ` [PATCH perf-tools-next 1/3] perf trace: Introduce kernel symbol beautifier for virtual addresses Aaron Tomlin
2026-08-15 23:44   ` sashiko-bot
2026-08-16  1:50     ` Aaron Tomlin
2026-08-15 23:36 ` [PATCH perf-tools-next 2/3] perf trace: Auto-assign kernel symbol beautifier to function pointer fields Aaron Tomlin
2026-08-15 23:47   ` sashiko-bot
2026-08-15 23:36 ` [PATCH perf-tools-next 3/3] perf trace: Enhance BTF type formatting to symbolise kernel function pointers Aaron Tomlin
2026-08-15 23:50   ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.