public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ian Rogers <irogers@google.com>
To: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	 Arnaldo Carvalho de Melo <acme@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>, Jiri Olsa <jolsa@kernel.org>,
	 Adrian Hunter <adrian.hunter@intel.com>,
	James Clark <james.clark@linaro.org>,
	 Zecheng Li <zli94@ncsu.edu>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	 linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Ian Rogers <irogers@google.com>
Subject: [PATCH v4 3/6] perf libdw: Fix libdw API contract violations
Date: Sun,  3 May 2026 10:10:29 -0700	[thread overview]
Message-ID: <20260503171032.1559338-4-irogers@google.com> (raw)
In-Reply-To: <20260503171032.1559338-1-irogers@google.com>

Check return values of `dwfl_report_end` and `dwfl_module_addrdie`.
Check `die_get_call_lineno` for errors.

Additionally:
 - Introduce `inline_node__clear_frames()` to clean up partial
   allocations.
 - Ensure `*file` is freed and inline frames are cleared on error in
   `libdw__addr2line()` to prevent memory leaks and duplicated
   callchains when falling back to other unwinders.
 - Allow DWARF line 0 in `libdw_a2l_cb()`, as it is a valid
   reference for compiler-generated code.
 - Fix the parent srcline lookup in `libdw_a2l_cb()` to target the
   correct parent node depending on the callchain order
   (ORDER_CALLER/ORDER_CALLEE).

Fixes: b7a2b011e962 ("perf powerpc: Unify the skip-callchain-idx libdw with that for addr2line")
Fixes: 88c51002d06f ("perf addr2line: Add a libdw implementation")
Assisted-by: Gemini-CLI:Google Gemini 3
Signed-off-by: Ian Rogers <irogers@google.com>
---
v4:
 - Fix memory leaks and duplicated callchains on unwinding errors.
 - Support DWARF line 0 in inline list.
 - Fix callchain parent update in ORDER_CALLER mode.
---
 tools/perf/util/libdw.c   | 84 +++++++++++++++++++++++++++++++--------
 tools/perf/util/srcline.c |  9 ++++-
 tools/perf/util/srcline.h |  1 +
 3 files changed, 77 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/libdw.c b/tools/perf/util/libdw.c
index 216977884103..e69ff4cea856 100644
--- a/tools/perf/util/libdw.c
+++ b/tools/perf/util/libdw.c
@@ -4,6 +4,7 @@
 #include "srcline.h"
 #include "symbol.h"
 #include "dwarf-aux.h"
+#include "callchain.h"
 #include <fcntl.h>
 #include <unistd.h>
 #include <elfutils/libdwfl.h>
@@ -60,7 +61,11 @@ struct Dwfl *dso__libdw_dwfl(struct dso *dso)
 		return NULL;
 	}
 
-	dwfl_report_end(dwfl, /*removed=*/NULL, /*arg=*/NULL);
+	if (dwfl_report_end(dwfl, NULL, NULL) != 0) {
+		dwfl_end(dwfl);
+		return NULL;
+	}
+
 	dso__set_libdw(dso, dwfl);
 
 	return dwfl;
@@ -72,41 +77,70 @@ struct libdw_a2l_cb_args {
 	struct inline_node *node;
 	char *leaf_srcline;
 	bool leaf_srcline_used;
+	int err;
 };
 
 static int libdw_a2l_cb(Dwarf_Die *die, void *_args)
 {
 	struct libdw_a2l_cb_args *args  = _args;
-	struct symbol *inline_sym = new_inline_sym(args->dso, args->sym, dwarf_diename(die));
+	const char *name = dwarf_diename(die);
+	struct symbol *inline_sym = new_inline_sym(args->dso, args->sym, name ?: "unknown");
 	const char *call_fname = die_get_call_file(die);
+	int call_lineno = die_get_call_lineno(die);
 	char *call_srcline = srcline__unknown;
-	struct inline_list *ilist;
 
-	if (!inline_sym)
-		return -ENOMEM;
+	if (!inline_sym) {
+		args->err = -ENOMEM;
+		return DWARF_CB_ABORT;
+	}
 
 	/* Assign caller information to the parent. */
-	if (call_fname)
-		call_srcline = srcline_from_fileline(call_fname, die_get_call_lineno(die));
+	if (call_fname && call_lineno >= 0)
+		call_srcline = srcline_from_fileline(call_fname, call_lineno);
 
-	list_for_each_entry(ilist, &args->node->val, list) {
-		if (args->leaf_srcline == ilist->srcline)
+	if (!list_empty(&args->node->val)) {
+		struct inline_list *parent;
+
+		if (callchain_param.order == ORDER_CALLEE)
+			parent = list_first_entry(&args->node->val, struct inline_list, list);
+		else
+			parent = list_last_entry(&args->node->val, struct inline_list, list);
+
+		if (args->leaf_srcline == parent->srcline)
 			args->leaf_srcline_used = false;
-		else if (ilist->srcline != srcline__unknown)
-			free(ilist->srcline);
-		ilist->srcline =  call_srcline;
+		else if (parent->srcline != srcline__unknown)
+			free(parent->srcline);
+		parent->srcline = call_srcline;
 		call_srcline = NULL;
-		break;
 	}
 	if (call_srcline && call_srcline != srcline__unknown)
 		free(call_srcline);
 
 	/* Add this symbol to the chain as the leaf. */
 	if (!args->leaf_srcline_used) {
-		inline_list__append_tail(inline_sym, args->leaf_srcline, args->node);
+		if (inline_list__append_tail(inline_sym, args->leaf_srcline, args->node) != 0) {
+			args->err = -ENOMEM;
+			if (inline_sym->inlined)
+				symbol__delete(inline_sym);
+			return DWARF_CB_ABORT;
+		}
 		args->leaf_srcline_used = true;
 	} else {
-		inline_list__append_tail(inline_sym, strdup(args->leaf_srcline), args->node);
+		char *srcline = strdup(args->leaf_srcline);
+
+		if (!srcline) {
+			args->err = -ENOMEM;
+			if (inline_sym->inlined)
+				symbol__delete(inline_sym);
+			return DWARF_CB_ABORT;
+		}
+		if (inline_list__append_tail(inline_sym, srcline, args->node) != 0) {
+			free(srcline);
+			args->err = -ENOMEM;
+			if (inline_sym->inlined)
+				symbol__delete(inline_sym);
+			return DWARF_CB_ABORT;
+		}
 	}
 	return 0;
 }
@@ -162,11 +196,29 @@ int libdw__addr2line(u64 addr, char **file, unsigned int *line_nr,
 			.leaf_srcline = srcline_from_fileline(src ?: "<unknown>", lineno),
 		};
 
+		if (!args.leaf_srcline) {
+			if (file && *file) {
+				free(*file);
+				*file = NULL;
+			}
+			return 0;
+		}
+
 		/* Walk from the parent down to the leaf. */
-		cu_walk_functions_at(cudie, addr, libdw_a2l_cb, &args);
+		if (cudie)
+			cu_walk_functions_at(cudie, addr, libdw_a2l_cb, &args);
 
 		if (!args.leaf_srcline_used)
 			free(args.leaf_srcline);
+
+		if (args.err) {
+			if (file && *file) {
+				free(*file);
+				*file = NULL;
+			}
+			inline_node__clear_frames(node);
+			return 0;
+		}
 	}
 	return 1;
 }
diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c
index db164d258163..62884428fb5a 100644
--- a/tools/perf/util/srcline.c
+++ b/tools/perf/util/srcline.c
@@ -429,10 +429,13 @@ struct inline_node *dso__parse_addr_inlines(struct dso *dso, u64 addr,
 	return addr2inlines(dso_name, addr, dso, sym);
 }
 
-void inline_node__delete(struct inline_node *node)
+void inline_node__clear_frames(struct inline_node *node)
 {
 	struct inline_list *ilist, *tmp;
 
+	if (node == NULL)
+		return;
+
 	list_for_each_entry_safe(ilist, tmp, &node->val, list) {
 		list_del_init(&ilist->list);
 		zfree_srcline(&ilist->srcline);
@@ -441,7 +444,11 @@ void inline_node__delete(struct inline_node *node)
 			symbol__delete(ilist->symbol);
 		free(ilist);
 	}
+}
 
+void inline_node__delete(struct inline_node *node)
+{
+	inline_node__clear_frames(node);
 	free(node);
 }
 
diff --git a/tools/perf/util/srcline.h b/tools/perf/util/srcline.h
index 7c37b3bf9ce7..1018cbc886d6 100644
--- a/tools/perf/util/srcline.h
+++ b/tools/perf/util/srcline.h
@@ -47,6 +47,7 @@ struct inline_node *dso__parse_addr_inlines(struct dso *dso, u64 addr,
 					    struct symbol *sym);
 /* free resources associated to the inline node list */
 void inline_node__delete(struct inline_node *node);
+void inline_node__clear_frames(struct inline_node *node);
 
 /* insert the inline node list into the DSO, which will take ownership */
 void inlines__tree_insert(struct rb_root_cached *tree,
-- 
2.54.0.545.g6539524ca2-goog


  parent reply	other threads:[~2026-05-03 17:10 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-03  0:35 [PATCH v3 0/6] perf DWARF: Fix libdw API contract violations and crashes Ian Rogers
2026-05-03  0:35 ` [PATCH v3 1/6] perf dwarf-aux: Fix libdw segmentation fault in cu_walk_functions_at Ian Rogers
2026-05-03  0:35 ` [PATCH v3 2/6] perf dwarf-aux: Fix libdw API contract violations Ian Rogers
2026-05-03  0:35 ` [PATCH v3 3/6] perf libdw: " Ian Rogers
2026-05-03  0:35 ` [PATCH v3 4/6] perf probe-finder: " Ian Rogers
2026-05-03  0:35 ` [PATCH v3 5/6] perf annotate-data: " Ian Rogers
2026-05-03  0:35 ` [PATCH v3 6/6] perf debuginfo: " Ian Rogers
2026-05-03 17:10 ` [PATCH v4 0/6] perf DWARF: Fix libdw API contract violations and crashes Ian Rogers
2026-05-03 17:10   ` [PATCH v4 1/6] perf dwarf-aux: Fix libdw segmentation fault in cu_walk_functions_at Ian Rogers
2026-05-03 23:33     ` Namhyung Kim
2026-05-03 17:10   ` [PATCH v4 2/6] perf dwarf-aux: Fix libdw API contract violations Ian Rogers
2026-05-03 23:36     ` Namhyung Kim
2026-05-03 17:10   ` Ian Rogers [this message]
2026-05-03 23:44     ` [PATCH v4 3/6] perf libdw: " Namhyung Kim
2026-05-03 17:10   ` [PATCH v4 4/6] perf probe-finder: " Ian Rogers
2026-05-03 23:49     ` Namhyung Kim
2026-05-03 17:10   ` [PATCH v4 5/6] perf annotate-data: " Ian Rogers
2026-05-03 23:53     ` Namhyung Kim
2026-05-03 17:10   ` [PATCH v4 6/6] perf debuginfo: " Ian Rogers
2026-05-03 23:54     ` Namhyung Kim
2026-05-04  8:12   ` [PATCH v5 0/9] [PATCH v5 0/9] perf DWARF: Fix libdw API contract violations and crashes Ian Rogers
2026-05-04  8:12     ` [PATCH v5 1/9] perf dwarf-aux: Fix libdw segmentation fault in cu_walk_functions_at Ian Rogers
2026-05-04  8:12     ` [PATCH v5 2/9] perf dwarf-aux: Fix libdw API contract violations Ian Rogers
2026-05-04  8:12     ` [PATCH v5 3/9] perf srcline: Introduce inline_node__clear_frames() Ian Rogers
2026-05-04  8:12     ` [PATCH v5 4/9] perf libdw: Fix callchain parent update in ORDER_CALLER mode Ian Rogers
2026-05-04  8:12     ` [PATCH v5 5/9] perf libdw: Support DWARF line 0 in inline list Ian Rogers
2026-05-04  8:12     ` [PATCH v5 6/9] perf libdw: Fix libdw API contract violations and memory leaks Ian Rogers
2026-05-04  8:12     ` [PATCH v5 7/9] perf probe-finder: Fix libdw API contract violations Ian Rogers
2026-05-04  8:12     ` [PATCH v5 8/9] perf annotate-data: " Ian Rogers
2026-05-04  8:12     ` [PATCH v5 9/9] perf debuginfo: " Ian Rogers
2026-05-04 23:50     ` [PATCH v5 0/9] [PATCH v5 0/9] perf DWARF: Fix libdw API contract violations and crashes Namhyung Kim

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=20260503171032.1559338-4-irogers@google.com \
    --to=irogers@google.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=zli94@ncsu.edu \
    /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