From e223a41c429bad47ed44907a14e8de8901eb93ce Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Thu, 17 Sep 2026 23:32:45 -0700 Subject: [PATCH] perf trace-event: Report tracepoint format errors with NULL and errno trace_event__tp_format() encoded failure with ERR_PTR() while trace_event__tp_format_id() returned a plain NULL when tep_find_event() found nothing, and tp_format() itself returned NULL when tep_parse_format() failed, as its return value was discarded. Callers test with IS_ERR(), which NULL does not satisfy, so those two failures were taken for success. In syscall__read_info() that leads straight to: if (IS_ERR(sc->tp_format)) { ... return err; } if (syscall__alloc_arg_fmts(sc, sc->tp_format->format.nr_fields - 1)) which dereferences NULL when a format file fails to parse. Mixing encoded error pointers with pointers that are compared against NULL is what allows that to happen, so drop ERR_PTR() here and report failures the way the rest of these paths already expect, by returning NULL with errno set. evsel__tp_format() no longer has to translate the error back into errno before printing it with %m, and the remaining callers become NULL tests. syscall__scnprintf_args() gains back its fallback of printing raw arguments: it asked for IS_ERR(sc->tp_format), but syscall__read_info() had already replaced the error pointer with NULL, so the branch could never be taken. Assisted-by: Antigravity:gemini-3.1-pro Signed-off-by: Ian Rogers Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-kmem.c | 3 +- tools/perf/builtin-sched.c | 5 ++-- tools/perf/builtin-trace.c | 10 +++---- tools/perf/util/evsel.c | 5 +--- tools/perf/util/trace-event.c | 52 ++++++++++++++++++++++++++--------- 5 files changed, 47 insertions(+), 28 deletions(-) diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c index e1b2f5bc1ba8..8693c6b135ca 100644 --- a/tools/perf/builtin-kmem.c +++ b/tools/perf/builtin-kmem.c @@ -1870,8 +1870,7 @@ static bool slab_legacy_tp_is_exposed(void) * means the tool is running on an old kernel, we need to * rollback to support these legacy tracepoints. */ - return IS_ERR(trace_event__tp_format("kmem", "kmalloc_node")) ? - false : true; + return trace_event__tp_format("kmem", "kmalloc_node"); } static int __cmd_record(int argc, const char **argv) diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c index dd39a4fb6c7a..5b4092ae33ee 100644 --- a/tools/perf/builtin-sched.c +++ b/tools/perf/builtin-sched.c @@ -5182,8 +5182,7 @@ static bool schedstat_events_exposed(void) * Select "sched:sched_stat_wait" event to check * whether schedstat tracepoints are exposed. */ - return IS_ERR(trace_event__tp_format("sched", "sched_stat_wait")) ? - false : true; + return trace_event__tp_format("sched", "sched_stat_wait"); } static int __cmd_record(int argc, const char **argv) @@ -5240,7 +5239,7 @@ static int __cmd_record(int argc, const char **argv) rec_argv[i++] = strdup("-e"); waking_event = trace_event__tp_format("sched", "sched_waking"); - if (!IS_ERR(waking_event)) + if (waking_event) rec_argv[i++] = strdup("sched:sched_waking"); else rec_argv[i++] = strdup("sched:sched_wakeup"); diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index f67557e7a254..7fe39455085f 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -2391,7 +2391,7 @@ static int syscall__read_info(struct syscall *sc, struct trace *trace) snprintf(tp_name, sizeof(tp_name), "sys_enter_%s", sc->name); sc->tp_format = trace_event__tp_format("syscalls", tp_name); - if (IS_ERR(sc->tp_format) && sc->fmt && sc->fmt->alias) { + if (!sc->tp_format && sc->fmt && sc->fmt->alias) { snprintf(tp_name, sizeof(tp_name), "sys_enter_%s", sc->fmt->alias); sc->tp_format = trace_event__tp_format("syscalls", tp_name); } @@ -2400,11 +2400,9 @@ static int syscall__read_info(struct syscall *sc, struct trace *trace) * Fails to read trace point format via sysfs node, so the trace point * doesn't exist. Set the 'nonexistent' flag as true. */ - if (IS_ERR(sc->tp_format)) { + if (!sc->tp_format) { sc->nonexistent = true; - err = PTR_ERR(sc->tp_format); - sc->tp_format = NULL; - return err; + return -errno; } /* @@ -2687,7 +2685,7 @@ static size_t syscall__scnprintf_args(struct syscall *sc, char *bf, size_t size, printed += syscall_arg_fmt__scnprintf_val(&sc->arg_fmt[arg.idx], bf + printed, size - printed, &arg, val); } - } else if (IS_ERR(sc->tp_format)) { + } else if (!sc->tp_format) { /* * If we managed to read the tracepoint /format file, then we * may end up not having any args, like with gettid(), so only diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index a2e9a31c1472..1c9e5a30a0cd 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -720,10 +720,7 @@ struct tep_event *evsel__tp_format(struct evsel *evsel) else tp_format = trace_event__tp_format(evsel->tp_sys, evsel->tp_name); - if (IS_ERR(tp_format)) { - int err = -PTR_ERR(tp_format); - - errno = err; + if (!tp_format) { pr_err("Error getting tracepoint format '%s': %m\n", evsel__name(evsel)); return NULL; diff --git a/tools/perf/util/trace-event.c b/tools/perf/util/trace-event.c index 000c1e1d68c1..10a7652f0305 100644 --- a/tools/perf/util/trace-event.c +++ b/tools/perf/util/trace-event.c @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include @@ -77,7 +76,7 @@ void trace_event__cleanup(struct trace_event *t) } /* - * Returns pointer with encoded error via interface. + * Returns NULL and sets errno on failure. */ static struct tep_event* tp_format(const char *sys, const char *name) @@ -90,38 +89,65 @@ tp_format(const char *sys, const char *name) char *data; int err; - if (!tp_dir) - return ERR_PTR(-errno); + if (!tp_dir) { + errno = ENOMEM; + return NULL; + } scnprintf(path, PATH_MAX, "%s/%s/format", tp_dir, name); put_events_file(tp_dir); err = filename__read_str(path, &data, &size); - if (err) - return ERR_PTR(err); + if (err) { + errno = -err; + return NULL; + } - tep_parse_format(pevent, &event, data, size, sys); + err = tep_parse_format(pevent, &event, data, size, sys); free(data); + + /* + * A parse failure leaves no event behind, report it rather than + * letting a NULL be mistaken for a successfully parsed format. + */ + if (err != TEP_ERRNO__SUCCESS || !event) { + errno = EINVAL; + return NULL; + } + return event; } /* - * Returns pointer with encoded error via interface. + * Returns NULL and sets errno on failure. */ struct tep_event* trace_event__tp_format(const char *sys, const char *name) { - if (!tevent_initialized && trace_event__init2()) - return ERR_PTR(-ENOMEM); + if (!tevent_initialized && trace_event__init2()) { + errno = ENOMEM; + return NULL; + } return tp_format(sys, name); } +/* + * Returns NULL and sets errno on failure. + */ struct tep_event *trace_event__tp_format_id(int id) { - if (!tevent_initialized && trace_event__init2()) - return ERR_PTR(-ENOMEM); + struct tep_event *event; - return tep_find_event(tevent.pevent, id); + if (!tevent_initialized && trace_event__init2()) { + errno = ENOMEM; + return NULL; + } + + event = tep_find_event(tevent.pevent, id); + if (!event) + errno = ENOENT; + + return event; }