[PATCH v4 0/6] perf annotate-data: Fix hangs on broken debug info, AMD mem record

[PATCH v4 0/6] perf annotate-data: Fix hangs on broken debug info, AMD mem record

[Arnaldo Carvalho de Melo]

Hi,

This originated in another patch series, 'perf tools: Annotate fixes,
stdio progress indication, debuginfo-client in more places', and is
being split off so that these fixes can be reviewed right away; the
debuginfod download feature work from it will come later, separately,
based on this series.

  - 'perf report -s type' spins forever on the dwz compressed debug info of
    zlib-ng (libz.so.1): die_collect_vars() saves a type DIE offset that is
    relative to the file the DIE lives in, the dwz alt file for types shared
    by several CUs, and resolving it in the main file parses whatever is at
    that offset, here a typedef whose DW_AT_type refers to itself, making the
    typedef/qualifier chase spin (patch 3), with the chases bounded so that
    other kinds of broken debug info don't hang perf either (patches 1, 2
    and 4);

  - 'perf mem record' requests PERF_SAMPLE_CPU (patch 5) and uses the IBS
    swfilt filter when the kernel exposes it (patch 6), with the tables
    carrying the term kept in the arch/x86 code, where the knowledge that
    IBS needs it stays, as Ravi Bangoria suggested reviewing Namhyung
    Kim's v6 review remark on this patch
    (<4349c387-5b8a-4e8d-932a-5175a7598e1e@amd.com>, replying to
    <aqsRVABj23jaaiSq@google.com>).

About PATCH 5, answering Namhyung Kim's v6 review question
(<aqsPKGVUkWk6wvYA@google.com>) about the --sample-cpu default: the
data source field is about the memory hierarchy level of the access,
it has no record of which CPU issued it, and while the TID is in
every sample and in the CTF stream, a thread time-sliced on one CPU
or moved between SMT siblings is not told apart by it from cross-core
contention, so it is the CPU id that keys it, and it is what the
false-sharing detector in pahole needs.  The TID is recorded as well.

Requires elfutils 0.160 for dwarf_cu_getdwarf(), so the libdw feature test
probes for it and Makefile.config says 0.160: older versions now disable
dwarf support with that message instead of failing to link.

Best regards,

- Arnaldo

What changed from v3:

- Dropped patch 5/7, "perf annotate-data: Show the sample count in the
  data-type browser": it is already in perf-tools-next as commit
  9db4e9d6cc7c, so the series is now 6 patches and the subject no longer
  mentions it.
- Rebased onto current perf-tools-next (base-commit below; v3 was based on
  29f320d221c1), patch 4 adapted to the upstream __add_member_cb() cleanup
  that dropped the member_type local, no behavior change.

What changed from v2:

Only aggregate (struct/union) members are marked as truncated when the
MAX_MEMBER_DEPTH limit is reached in the member nesting recursion (patch
4), addressing a sashiko [Medium] review finding
(<20260921165704.38DBD1F000FF@smtp.kernel.org>): the v2 check was made
before looking at the member's type, so a primitive field that merely
landed on the limit, e.g. an int in a struct nested 31 deep, was marked
as truncated and logged even though it has no children to expand.  The
limit still bounds the recursion, as only aggregates are expanded.

What changed from v1:

Bump MAX_MEMBER_DEPTH from 8 to 32, addressing a review comment from Namhyung.

 tools/build/feature/test-libdw.c           |  10 +-
 tools/perf/Documentation/perf-mem.txt      |   4 +
 tools/perf/Makefile.config                 |   2 +-
 tools/perf/arch/x86/util/mem-events.c      |  19 +++
 tools/perf/arch/x86/util/mem-events.h      |   2 +
 tools/perf/arch/x86/util/pmu.c             |  10 +-
 tools/perf/builtin-mem.c                   |   7 +-
 tools/perf/tests/shell/test_data_symbol.sh |   6 +-
 tools/perf/util/annotate-data.c            |  41 ++++--
 tools/perf/util/annotate-data.h            |   3 +
 tools/perf/util/dwarf-aux.c                | 140 +++++++++++++++++----
 tools/perf/util/dwarf-aux.h                |  13 ++
 tools/perf/util/mem-events.c               |  11 +-
 13 files changed, 226 insertions(+), 42 deletions(-)

v3-head: e2504280f5c4086e9851a758ed1bae8df7ef269e
base-commit: edd8a9fe2eca009599e013a29c421c7a6b5ad1b9
--

[PATCH v4 1/6] perf dwarf-aux: Bound the type chases for broken debug info

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

A DIE parsed at an offset that is not the start of a DIE can have a
DW_AT_type that refers back to itself, making the typedef/qualifier
chases in die_get_real_type(), die_get_pointer_type() and
die_get_typename_from_type() spin forever; 'perf report -s type' did
that on the dwz compressed debug info of zlib-ng (libz.so.1).

No sane typedef/qualifier chain is 32 DIEs long, so give up on the type
with a pr_debug instead of hanging.

Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dwarf-aux.c | 84 ++++++++++++++++++++++++++++---------
 1 file changed, 64 insertions(+), 20 deletions(-)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 1560e721479a1ac2..2c135ebb1551fca4 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -267,16 +267,27 @@ Dwarf_Die *die_get_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)
 		return NULL;
 }
 
+/*
+ * A DIE parsed at an offset that is not the start of a DIE can refer
+ * back to itself via DW_AT_type: bound these chases.
+ */
+#define MAX_TYPE_CHASE 32
+
 /* Get a type die, but skip qualifiers */
 Dwarf_Die *__die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)
 {
-	int tag;
+	int tag, chase = 0;
 
 	do {
 		vr_die = die_get_type(vr_die, die_mem);
 		if (!vr_die)
-			break;
+			return NULL;
 		tag = dwarf_tag(vr_die);
+		if (++chase > MAX_TYPE_CHASE) {
+			pr_debug("DWARF: qualifier chase limit reached at DIE 0x%lx\n",
+				 (unsigned long)dwarf_dieoffset(vr_die));
+			return NULL;
+		}
 	} while (tag == DW_TAG_const_type ||
 		 tag == DW_TAG_restrict_type ||
 		 tag == DW_TAG_volatile_type ||
@@ -297,8 +308,15 @@ Dwarf_Die *__die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)
  */
 Dwarf_Die *die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)
 {
+	int chase = 0;
+
 	do {
 		vr_die = __die_get_real_type(vr_die, die_mem);
+		if (++chase > MAX_TYPE_CHASE) {
+			pr_debug("DWARF: typedef chase limit reached at DIE 0x%lx\n",
+				 vr_die ? (unsigned long)dwarf_dieoffset(vr_die) : 0);
+			return NULL;
+		}
 	} while (vr_die && dwarf_tag(vr_die) == DW_TAG_typedef);
 
 	return vr_die;
@@ -315,7 +333,7 @@ Dwarf_Die *die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)
  */
 Dwarf_Die *die_get_pointer_type(Dwarf_Die *type_die, Dwarf_Die *die_mem)
 {
-	int tag;
+	int tag, chase = 0;
 
 	do {
 		tag = dwarf_tag(type_die);
@@ -325,6 +343,11 @@ Dwarf_Die *die_get_pointer_type(Dwarf_Die *type_die, Dwarf_Die *die_mem)
 		    tag != DW_TAG_restrict_type && tag != DW_TAG_volatile_type &&
 		    tag != DW_TAG_shared_type)
 			return NULL;
+		if (++chase > MAX_TYPE_CHASE) {
+			pr_debug("DWARF: pointer type chase limit reached at DIE 0x%lx\n",
+				 (unsigned long)dwarf_dieoffset(type_die));
+			return NULL;
+		}
 		type_die = die_get_type(type_die, die_mem);
 	} while (type_die);
 
@@ -1119,17 +1142,25 @@ Dwarf_Die *die_find_member(Dwarf_Die *st_die, const char *name,
 			      die_mem);
 }
 
-/**
- * die_get_typename_from_type - Get the name of given type DIE
- * @type_die: a type DIE
- * @buf: a strbuf for result type name
- *
- * Get the name of @type_die and stores it to @buf. Return 0 if succeeded.
- * and Return -ENOENT if failed to find type name.
- * Note that the result will stores typedef name if possible, and stores
- * "*(function_type)" if the type is a function pointer.
+/*
+ * The name follows DW_AT_type, so a self-referring DIE makes this
+ * recurse forever: bound it like the chases above.
  */
-int die_get_typename_from_type(Dwarf_Die *type_die, struct strbuf *buf)
+static int __die_get_typename_from_type(Dwarf_Die *type_die, struct strbuf *buf,
+					int depth);
+
+static int __die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf, int depth)
+{
+	Dwarf_Die type;
+
+	if (__die_get_real_type(vr_die, &type) == NULL)
+		return -ENOENT;
+
+	return __die_get_typename_from_type(&type, buf, depth);
+}
+
+static int __die_get_typename_from_type(Dwarf_Die *type_die, struct strbuf *buf,
+					int depth)
 {
 	int tag, ret;
 	const char *tmp = "";
@@ -1156,7 +1187,12 @@ int die_get_typename_from_type(Dwarf_Die *type_die, struct strbuf *buf)
 		/* Write a base name */
 		return strbuf_addf(buf, "%s%s", tmp, name ?: "");
 	}
-	ret = die_get_typename(type_die, buf);
+	if (depth >= MAX_TYPE_CHASE) {
+		pr_debug("DWARF: type name recursion limit reached at DIE 0x%lx\n",
+			 (unsigned long)dwarf_dieoffset(type_die));
+		return -ENOENT;
+	}
+	ret = __die_get_typename(type_die, buf, depth + 1);
 	if (ret < 0) {
 		/* void pointer has no type attribute */
 		if (tag == DW_TAG_pointer_type && ret == -ENOENT)
@@ -1167,6 +1203,19 @@ int die_get_typename_from_type(Dwarf_Die *type_die, struct strbuf *buf)
 	return strbuf_addstr(buf, tmp);
 }
 
+/**
+ * die_get_typename_from_type - Get the name of a type DIE
+ * @type_die: a type DIE
+ * @buf: a strbuf for the result type name
+ *
+ * Stores the typedef name if possible, "*(function_type)" for function
+ * pointers.  Returns 0 or -ENOENT.
+ */
+int die_get_typename_from_type(Dwarf_Die *type_die, struct strbuf *buf)
+{
+	return __die_get_typename_from_type(type_die, buf, 0);
+}
+
 /**
  * die_get_typename - Get the name of given variable DIE
  * @vr_die: a variable DIE
@@ -1179,12 +1228,7 @@ int die_get_typename_from_type(Dwarf_Die *type_die, struct strbuf *buf)
  */
 int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
 {
-	Dwarf_Die type;
-
-	if (__die_get_real_type(vr_die, &type) == NULL)
-		return -ENOENT;
-
-	return die_get_typename_from_type(&type, buf);
+	return __die_get_typename(vr_die, buf, 0);
 }
 
 /**
-- 
2.53.0

[PATCH v4 2/6] perf dwarf-aux: Add die_same_file() and die_get_type_die()

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

dwarf_dieoffset() is relative to the file the DIE is in, so an offset
saved by die_collect_vars()/die_collect_global_vars() is only
meaningful in that file: add die_same_file(), comparing the Dwarf each
DIE's CU belongs to, and die_get_type_die(), resolving the offset in it
with the saved tag as a sanity check.  Resolving the offset in the main
file instead parses whatever is at it when it came from the dwz common
file.

dwarf_cu_getdwarf() is new in elfutils 0.160, so the libdw feature test
probes for it and Makefile.config says 0.160.

Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/build/feature/test-libdw.c | 10 +++++-
 tools/perf/Makefile.config       |  2 +-
 tools/perf/util/dwarf-aux.c      | 56 ++++++++++++++++++++++++++++++--
 tools/perf/util/dwarf-aux.h      | 13 ++++++++
 4 files changed, 77 insertions(+), 4 deletions(-)

diff --git a/tools/build/feature/test-libdw.c b/tools/build/feature/test-libdw.c
index aabd63ca76b4d7e6..46f75cde59471079 100644
--- a/tools/build/feature/test-libdw.c
+++ b/tools/build/feature/test-libdw.c
@@ -49,8 +49,16 @@ int test_elfutils(void)
 	return 0;
 }
 
+/* Needs elfutils 0.160: taking its address fails the probe, not the link. */
+int test_libdw_cu_getdwarf(void)
+{
+	void *sym = (void *)dwarf_cu_getdwarf;
+
+	return sym == NULL;
+}
+
 int main(void)
 {
 	return test_libdw() + test_libdw_unwind() + test_libdw_getlocations() +
-	       test_libdw_getcfi() + test_elfutils();
+	       test_libdw_getcfi() + test_libdw_cu_getdwarf() + test_elfutils();
 }
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index dca62e155fb5570a..b288ae0191acf7ef 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -477,7 +477,7 @@ else
   else
     ifneq ($(feature-libdw), 1)
       ifndef NO_LIBDW
-        $(warning No libdw.h found or old libdw.h found or elfutils is older than 0.157, disables dwarf support. Please install new elfutils-devel/libdw-dev)
+        $(warning No libdw.h found or old libdw.h found or elfutils is older than 0.160, disables dwarf support. Please install new elfutils-devel/libdw-dev)
         NO_LIBDW := 1
       endif
     endif # Dwarf support
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 2c135ebb1551fca4..54f8b5ec74a2fe70 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1677,6 +1677,15 @@ Dwarf_Die *die_find_variable_by_addr(Dwarf_Die *sc_die, Dwarf_Addr addr,
 	return result;
 }
 
+/*
+ * Whether two DIEs live in the same debug file: dwarf_dieoffset() is
+ * relative to the file the DIE is in.
+ */
+bool die_same_file(Dwarf_Die *die_a, Dwarf_Die *die_b)
+{
+	return dwarf_cu_getdwarf(die_a->cu) == dwarf_cu_getdwarf(die_b->cu);
+}
+
 static int __die_collect_vars_cb(Dwarf_Die *die_mem, void *arg)
 {
 	struct die_var_type **var_types = arg;
@@ -1721,6 +1730,8 @@ static int __die_collect_vars_cb(Dwarf_Die *die_mem, void *arg)
 			vt->is_reg_var_addr = true;
 
 		vt->die_off = dwarf_dieoffset(&type_die);
+		vt->die_tag = dwarf_tag(&type_die);
+		vt->from_alt = !die_same_file(die_mem, &type_die);
 		vt->addr = start;
 		vt->end = end;
 		vt->has_range = (end != 0 || start != 0);
@@ -1740,7 +1751,8 @@ static int __die_collect_vars_cb(Dwarf_Die *die_mem, void *arg)
  *
  * Save all variables and parameters in the @sc_die and save them to @var_types.
  * The @var_types is a singly-linked list containing type and location info.
- * Actual type can be retrieved using dwarf_offdie() with 'die_off' later.
+ * Actual type can be retrieved using die_get_type_die() with 'die_off',
+ * 'die_tag' and 'from_alt' later.
  *
  * Callers should free @var_types.
  */
@@ -1786,6 +1798,8 @@ static int __die_collect_global_vars_cb(Dwarf_Die *die_mem, void *arg)
 		return DIE_FIND_CB_END;
 
 	vt->die_off = dwarf_dieoffset(&type_die);
+	vt->die_tag = dwarf_tag(&type_die);
+	vt->from_alt = !die_same_file(die_mem, &type_die);
 	vt->addr = ops->number;
 	vt->end = 0;
 	vt->has_range = false;
@@ -1797,6 +1811,43 @@ static int __die_collect_global_vars_cb(Dwarf_Die *die_mem, void *arg)
 	return DIE_FIND_CB_SIBLING;
 }
 
+/**
+ * die_get_type_die - Get a type DIE saved by die_collect_vars()
+ * @dbg: the main debug info
+ * @die_off: offset of the type DIE, from dwarf_dieoffset()
+ * @die_tag: tag that DIE had when the offset was saved
+ * @from_alt: whether the type DIE is in the dwz alt file
+ * @die_mem: where to store the resulting DIE
+ *
+ * Resolve @die_off in the file it was recorded as belonging to, with no
+ * fallback; @die_tag is a sanity check.
+ */
+Dwarf_Die *die_get_type_die(Dwarf *dbg, u64 die_off, int die_tag, bool from_alt,
+			    Dwarf_Die *die_mem)
+{
+	Dwarf *target = dbg;
+	Dwarf_Die die;
+
+	if (from_alt) {
+		/* No fallback: an alt offset here parses whatever is at it. */
+		target = dwarf_getalt(dbg);
+		if (target == NULL) {
+			pr_debug("DWARF: no alt (dwz) debug file to resolve the type DIE at offset 0x%lx in\n",
+				 (unsigned long)die_off);
+			return NULL;
+		}
+	}
+
+	if (dwarf_offdie(target, die_off, &die) && dwarf_tag(&die) == die_tag) {
+		*die_mem = die;
+		return die_mem;
+	}
+
+	pr_debug("DWARF: no DIE with tag %d at offset 0x%lx in the %s debug file\n",
+		 die_tag, (unsigned long)die_off, from_alt ? "alt" : "main");
+	return NULL;
+}
+
 /**
  * die_collect_global_vars - Save all global variables
  * @cu_die: a CU DIE
@@ -1804,7 +1855,8 @@ static int __die_collect_global_vars_cb(Dwarf_Die *die_mem, void *arg)
  *
  * Save all global variables in the @cu_die and save them to @var_types.
  * The @var_types is a singly-linked list containing type and location info.
- * Actual type can be retrieved using dwarf_offdie() with 'die_off' later.
+ * Actual type can be retrieved using die_get_type_die() with 'die_off',
+ * 'die_tag' and 'from_alt' later.
  *
  * Callers should free @var_types.
  */
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index 9b662db710220522..6f9145510adcb841 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -152,6 +152,8 @@ int die_get_scopes(Dwarf_Die *cu_die, Dwarf_Addr pc, Dwarf_Die **scopes);
 struct die_var_type {
 	struct die_var_type *next;
 	u64 die_off;
+	int die_tag;
+	bool from_alt;	/* die_off is relative to the alt (dwz) file */
 	u64 addr;
 	u64 end;        /* end address of location range */
 	int reg;
@@ -183,6 +185,17 @@ Dwarf_Die *die_find_variable_by_addr(Dwarf_Die *sc_die, Dwarf_Addr addr,
 /* Save all variables and parameters in this scope */
 void die_collect_vars(Dwarf_Die *sc_die, struct die_var_type **var_types);
 
+/*
+ * Get the type DIE saved by die_collect_vars()/die_collect_global_vars():
+ * @from_alt says which file the offset must be resolved in, @die_tag is
+ * a sanity check.
+ */
+Dwarf_Die *die_get_type_die(Dwarf *dbg, u64 die_off, int die_tag, bool from_alt,
+			    Dwarf_Die *die_mem);
+
+/* Whether two DIEs live in the same debug file */
+bool die_same_file(Dwarf_Die *die_a, Dwarf_Die *die_b);
+
 /* Save all global variables in this CU */
 void die_collect_global_vars(Dwarf_Die *cu_die, struct die_var_type **var_types);
 
-- 
2.53.0

[PATCH v4 3/6] perf annotate-data: Resolve type DIEs in the debug file they came from

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

'perf report -s type' hangs burning CPU on the dwz compressed debug
info of libz.so.1 (zlib-ng): die_collect_vars() saves the
dwarf_dieoffset() of the type DIE, which is relative to the file it
lives in - the dwz alt file for the types shared by more than one CU -
and update_var_state() resolves that offset in the main debug file,
parsing whatever is at it, here a typedef whose DW_AT_type refers to
itself.

Record whether the type DIE came from the alt file and resolve the
offset with die_get_type_die() in that file, with no fallback.

Fixes: 06b2ce75386df04b ("perf annotate-data: Maintain variable type info")
Fixes: 55ee3d005d62279d ("perf annotate-data: Add a cache for global variable types")
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/annotate-data.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index e9b58a3d587be45b..bc9698abc6a9e055 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -739,6 +739,8 @@ struct global_var_entry {
 	u64 start;
 	u64 end;
 	u64 die_offset;
+	int die_tag;
+	bool from_alt;	/* die_offset is relative to the alt (dwz) file */
 };
 
 static int global_var_cmp(const void *_key, const struct rb_node *node)
@@ -776,7 +778,7 @@ static struct global_var_entry *global_var__find(struct data_loc_info *dloc, u64
 }
 
 static bool global_var__add(struct data_loc_info *dloc, u64 addr,
-			    const char *name, Dwarf_Die *type_die)
+			    const char *name, Dwarf_Die *type_die, bool from_alt)
 {
 	struct dso *dso = map__dso(dloc->ms->map);
 	struct global_var_entry *gvar;
@@ -798,6 +800,8 @@ static bool global_var__add(struct data_loc_info *dloc, u64 addr,
 	gvar->start = addr;
 	gvar->end = addr + size;
 	gvar->die_offset = dwarf_dieoffset(type_die);
+	gvar->die_tag = dwarf_tag(type_die);
+	gvar->from_alt = from_alt;
 
 	rb_add(&gvar->node, dso__global_vars(dso), global_var_less);
 	return true;
@@ -872,12 +876,14 @@ static void global_var__collect(struct data_loc_info *dloc)
 			if (pos->reg != -1)
 				continue;
 
-			if (!dwarf_offdie(dwarf, pos->die_off, &type_die))
+			if (!die_get_type_die(dwarf, pos->die_off, pos->die_tag,
+					      pos->from_alt, &type_die))
 				continue;
 
 			get_global_var_info(dloc, pos->addr, &var_name, &var_offset);
 
-			global_var__add(dloc, pos->addr, var_name, &type_die);
+			global_var__add(dloc, pos->addr, var_name, &type_die,
+					pos->from_alt);
 		}
 
 		delete_var_types(var_types);
@@ -902,7 +908,8 @@ bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc,
 
 	gvar = global_var__find(dloc, var_addr);
 	if (gvar) {
-		if (!dwarf_offdie(dloc->di->dbg, gvar->die_offset, type_die))
+		if (!die_get_type_die(dloc->di->dbg, gvar->die_offset,
+				      gvar->die_tag, gvar->from_alt, type_die))
 			return false;
 
 		*var_offset = var_addr - gvar->start;
@@ -932,7 +939,8 @@ bool get_global_var_type(Dwarf_Die *cu_die, struct data_loc_info *dloc,
 
 ok:
 	/* The address should point to the start of the variable */
-	global_var__add(dloc, var_addr - *var_offset, var_name, type_die);
+	global_var__add(dloc, var_addr - *var_offset, var_name, type_die,
+			!die_same_file(cu_die, type_die));
 	return true;
 }
 
@@ -987,7 +995,8 @@ static void update_var_state(struct type_state *state, struct data_loc_info *dlo
 				continue;
 		}
 		/* Get the type DIE using the offset */
-		if (!dwarf_offdie(dloc->di->dbg, var->die_off, &mem_die))
+		if (!die_get_type_die(dloc->di->dbg, var->die_off,
+				      var->die_tag, var->from_alt, &mem_die))
 			continue;
 
 		if (var->reg == DWARF_REG_FB || var->reg == fbreg || var->reg == state->stack_reg) {
-- 
2.53.0

[PATCH v4 4/6] perf annotate-data: Bound the member nesting recursion

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Members are added recursively, and a broken DIE can make a member's
type point back at one of its own ancestors, recursing until the stack
is gone; nothing usable comes out of nesting members 32 deep anyway, so
stop there, marking the member as truncated and giving up on member
types that don't resolve.

The check is where the children of an aggregate would be expanded, so
only struct/union members are marked as truncated.

Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/annotate-data.c | 20 ++++++++++++++++++--
 tools/perf/util/annotate-data.h |  3 +++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index bc9698abc6a9e055..39e0b125a6cc1eca 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -222,6 +222,12 @@ static bool data_type_less(struct rb_node *node_a, const struct rb_node *node_b)
 	return strcmp(a->self.type_name, b->self.type_name) < 0;
 }
 
+/*
+ * A broken type can point back at one of its own ancestors: bound the
+ * nesting so it doesn't recurse until the stack is gone.
+ */
+#define MAX_MEMBER_DEPTH 32
+
 /* Recursively add new members for struct/union */
 static int __add_member_cb(Dwarf_Die *die, void *arg)
 {
@@ -236,6 +242,9 @@ static int __add_member_cb(Dwarf_Die *die, void *arg)
 	if (dwarf_tag(die) != DW_TAG_member)
 		return DIE_FIND_CB_SIBLING;
 
+	if (die_get_real_type(die, &die_mem) == NULL)
+		return DIE_FIND_CB_SIBLING;
+
 	member = zalloc(sizeof(*member));
 	if (member == NULL)
 		return DIE_FIND_CB_END;
@@ -248,8 +257,6 @@ static int __add_member_cb(Dwarf_Die *die, void *arg)
 	if (die_get_typename(die, &sb) < 0)
 		strbuf_add(&sb, "(unknown type)", 14);
 
-	die_get_real_type(die, &die_mem);
-
 	if (dwarf_aggregate_size(&die_mem, &size) < 0 || size == 0) {
 		if (dwarf_tag(&die_mem) == DW_TAG_array_type) { /* flex-array? */
 			die_get_real_type(&die_mem, &die_mem);
@@ -299,6 +306,7 @@ static int __add_member_cb(Dwarf_Die *die, void *arg)
 	}
 	member->size = size;
 	member->offset = loc + parent->offset;
+	member->depth = parent->depth + 1;
 	INIT_LIST_HEAD(&member->children);
 
 	list_for_each_entry_reverse(prev, &parent->children, node) {
@@ -313,6 +321,14 @@ static int __add_member_cb(Dwarf_Die *die, void *arg)
 		member->is_union = true;
 		/* fall through */
 	case DW_TAG_structure_type:
+		/* Only aggregates have children to expand, so only they get truncated. */
+		if (member->depth >= MAX_MEMBER_DEPTH) {
+			/* Reported by the JSON exporter so consumers can tell a truncated tree. */
+			member->truncated = true;
+			pr_debug_dtp("member nesting limit reached at %s\n",
+				     member->type_name ?: "(unknown type)");
+			break;
+		}
 		die_find_child(&die_mem, __add_member_cb, member, &die_mem);
 		break;
 	default:
diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
index 14b8113521a927cf..ca0abfb2a036d229 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -61,6 +61,9 @@ struct annotated_member {
 	int size;
 	bool is_union;
 	bool is_flex_array;
+	unsigned int depth;
+	/* Children not expanded because the nesting limit was reached */
+	bool truncated;
 };
 
 /**
-- 
2.53.0

[PATCH v4 5/6] perf mem record: Request PERF_SAMPLE_CPU by default

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

The data type profiling per-sample stream keys cross-CPU contention on
sample->cpu, which without PERF_SAMPLE_CPU is the "no CPU info"
sentinel, making same-instance accesses from different cores
indistinguishable from same-CPU traffic.  'perf mem record' already
passes -d and -W to the record parser, add --sample-cpu and document it
in perf-mem(1).

Also grow rec_argv: the new entry overflows it on PMUs with separate
load and store events, as the space for the fixed arguments was not
reserved.

Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-mem.txt | 4 ++++
 tools/perf/builtin-mem.c              | 7 +++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-mem.txt b/tools/perf/Documentation/perf-mem.txt
index 4d164836d0943119..fe51c5e3333dc4a0 100644
--- a/tools/perf/Documentation/perf-mem.txt
+++ b/tools/perf/Documentation/perf-mem.txt
@@ -14,6 +14,10 @@ DESCRIPTION
 -----------
 "perf mem record" runs a command and gathers memory operation data
 from it, into perf.data. Perf record options are accepted and are passed through.
+It also requests the address (-d), the weight (-W, where supported) and the
+CPU id (--sample-cpu) of every sampled access by default; the CPU id is what
+lets per-sample analysis tell reads and writes to the same data from
+different cores apart from same-CPU traffic.
 
 "perf mem report" displays the result. It invokes perf report with the
 right set of options to display a memory access profile. By default, loads
diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index 6101a26b3a781e69..ade53688ebcd0f10 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -99,8 +99,8 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem,
 	argc = parse_options(argc, argv, options, record_usage,
 			     PARSE_OPT_KEEP_UNKNOWN);
 
-	/* Max number of arguments multiplied by number of PMUs that can support them. */
-	rec_argc = argc + 9 * (perf_pmu__mem_events_num_mem_pmus(pmu) + 1);
+	/* Max number of arguments per PMU plus the fixed ones added below. */
+	rec_argc = argc + 8 + 9 * (perf_pmu__mem_events_num_mem_pmus(pmu) + 1);
 
 	if (mem->cpu_list)
 		rec_argc += 2;
@@ -135,6 +135,9 @@ static int __cmd_record(int argc, const char **argv, struct perf_mem *mem,
 
 	rec_argv[i++] = "-d";
 
+	/* Cross-CPU contention keys on sample->cpu, -1 without PERF_SAMPLE_CPU. */
+	rec_argv[i++] = "--sample-cpu";
+
 	if (mem->phys_addr)
 		rec_argv[i++] = "--phys-data";
 
-- 
2.53.0

[PATCH v4 6/6] perf mem record: Use the IBS swfilt filter when available

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

IBS events with exclude_{user,kernel} bits, as used for per-thread
recording when kernel samples are not allowed, are rejected on hardware
without the privilege filter, so per-thread 'perf mem record' fails on
AMD:

  $ perf mem record -o /dev/null -- true
  Failure to open event 'ibs_op/ldlat=0/u' on PMU 'ibs_op' which will be removed.

Kernel v6.14 added swfilt, a software privilege filter exposed as the
'swfilt' format term, making those events usable per-thread.  Give the
ibs_op memory events extra tables with the term, selected in
perf_pmu__arch_init() when the PMU exposes it, keeping the names that
need system wide mode otherwise; the knowledge that IBS needs this
stays in the arch code.

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Suggested-by: Ravi Bangoria <ravi.bangoria@amd.com>
Assisted-by: LLM
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/x86/util/mem-events.c      | 19 +++++++++++++++++++
 tools/perf/arch/x86/util/mem-events.h      |  2 ++
 tools/perf/arch/x86/util/pmu.c             | 10 ++++++++--
 tools/perf/tests/shell/test_data_symbol.sh |  6 ++++--
 tools/perf/util/mem-events.c               | 11 +++++++----
 5 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/tools/perf/arch/x86/util/mem-events.c b/tools/perf/arch/x86/util/mem-events.c
index b38f519020ff8c6f..156974ead7d42a82 100644
--- a/tools/perf/arch/x86/util/mem-events.c
+++ b/tools/perf/arch/x86/util/mem-events.c
@@ -21,14 +21,33 @@ struct perf_mem_event perf_mem_events_intel_aux[PERF_MEM_EVENTS__MAX] = {
 	E(NULL,			NULL,				NULL,		false,	0),
 };
 
+/*
+ * IBS events with exclude_{user,kernel} bits are rejected on hardware
+ * without the privilege filter unless swfilt is used: the extra tables
+ * carry the term, selected when the PMU exposes it.  Keep it even when
+ * the event has no exclude bits yet, 'perf record' adds them on the
+ * first open failure and retries with the same name.
+ */
 struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX] = {
 	E(NULL,		NULL,		NULL,	false,	0),
 	E(NULL,		NULL,		NULL,	false,	0),
 	E("mem-ldst",	"%s//",		NULL,	false,	0),
 };
 
+struct perf_mem_event perf_mem_events_amd_swfilt[PERF_MEM_EVENTS__MAX] = {
+	E(NULL,		NULL,		NULL,	false,	0),
+	E(NULL,		NULL,		NULL,	false,	0),
+	E("mem-ldst",	"%s/swfilt=1/",	NULL,	false,	0),
+};
+
 struct perf_mem_event perf_mem_events_amd_ldlat[PERF_MEM_EVENTS__MAX] = {
 	E(NULL,		NULL,		NULL,	false,	0),
 	E(NULL,		NULL,		NULL,	false,	0),
 	E("mem-ldst",	"%s/ldlat=%u/",	NULL,	true,	0),
 };
+
+struct perf_mem_event perf_mem_events_amd_ldlat_swfilt[PERF_MEM_EVENTS__MAX] = {
+	E(NULL,		NULL,			NULL,	false,	0),
+	E(NULL,		NULL,			NULL,	false,	0),
+	E("mem-ldst",	"%s/ldlat=%u,swfilt=1/",	NULL,	true,	0),
+};
diff --git a/tools/perf/arch/x86/util/mem-events.h b/tools/perf/arch/x86/util/mem-events.h
index 11e09a256f5bb084..f707de38037017c1 100644
--- a/tools/perf/arch/x86/util/mem-events.h
+++ b/tools/perf/arch/x86/util/mem-events.h
@@ -6,6 +6,8 @@ extern struct perf_mem_event perf_mem_events_intel[PERF_MEM_EVENTS__MAX];
 extern struct perf_mem_event perf_mem_events_intel_aux[PERF_MEM_EVENTS__MAX];
 
 extern struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX];
+extern struct perf_mem_event perf_mem_events_amd_swfilt[PERF_MEM_EVENTS__MAX];
 extern struct perf_mem_event perf_mem_events_amd_ldlat[PERF_MEM_EVENTS__MAX];
+extern struct perf_mem_event perf_mem_events_amd_ldlat_swfilt[PERF_MEM_EVENTS__MAX];
 
 #endif /* _X86_MEM_EVENTS_H */
diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
index 2c24ef3140da5e9b..fd4491fda117e51a 100644
--- a/tools/perf/arch/x86/util/pmu.c
+++ b/tools/perf/arch/x86/util/pmu.c
@@ -333,6 +333,7 @@ static void uncore_cha_imc_adjust_cpumask_for_snc(struct perf_pmu *pmu, bool cha
 void perf_pmu__arch_init(struct perf_pmu *pmu)
 {
 	struct perf_pmu_caps *ldlat_cap;
+	bool swfilt_format;
 
 	if (!strcmp(pmu->name, INTEL_PT_PMU_NAME)) {
 		pmu->auxtrace = true;
@@ -348,7 +349,10 @@ void perf_pmu__arch_init(struct perf_pmu *pmu)
 		if (strcmp(pmu->name, "ibs_op"))
 			return;
 
-		pmu->mem_events = perf_mem_events_amd;
+		swfilt_format = perf_pmu__has_format(pmu, "swfilt");
+		pmu->mem_events = swfilt_format ?
+				  perf_mem_events_amd_swfilt :
+				  perf_mem_events_amd;
 
 		if (!perf_pmu__caps_parse(pmu))
 			return;
@@ -358,7 +362,9 @@ void perf_pmu__arch_init(struct perf_pmu *pmu)
 			return;
 
 		perf_mem_events__loads_ldlat = 0;
-		pmu->mem_events = perf_mem_events_amd_ldlat;
+		pmu->mem_events = swfilt_format ?
+				  perf_mem_events_amd_ldlat_swfilt :
+				  perf_mem_events_amd_ldlat;
 	} else {
 		if (pmu->is_core) {
 			if (perf_pmu__have_event(pmu, "mem-loads-aux"))
diff --git a/tools/perf/tests/shell/test_data_symbol.sh b/tools/perf/tests/shell/test_data_symbol.sh
index d61b5659a46d9a77..52c837fddb639595 100755
--- a/tools/perf/tests/shell/test_data_symbol.sh
+++ b/tools/perf/tests/shell/test_data_symbol.sh
@@ -65,15 +65,17 @@ if (($is_amd >= 1)); then
 	# --ldlat on AMD:
 	# o Zen4 and earlier uarch does not support ldlat
 	# o Even on supported platforms, it's disabled (--ldlat=0) by default.
+	# o Kernels with the swfilt term add it even when ldlat is not
+	#   supported, so only check ldlat when the term is present.
 	ldlat=${BASH_REMATCH[1]}
-	if [[ -n $ldlat ]]; then
+	if [[ $ldlat == *ldlat=* ]]; then
 		if ! [[ "$ldlat" =~ ldlat=0 ]]; then
 			echo "ERROR: ldlat not initialized to 0?"
 			exit 1
 		fi
 
 		mem_events="$(perf mem record -v --ldlat=150 -e list 2>&1)"
-		if ! [[ "$mem_events" =~ ^mem-ldst.*ibs_op/ldlat=150/.*available ]]; then
+		if ! [[ "$mem_events" =~ ^mem-ldst.*ibs_op/ldlat=150[,/].*available ]]; then
 			echo "ERROR: --ldlat not honored?"
 			exit 1
 		fi
diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
index 0b49fce251fcc184..0b07011939d7d7c3 100644
--- a/tools/perf/util/mem-events.c
+++ b/tools/perf/util/mem-events.c
@@ -82,6 +82,7 @@ static const char *perf_pmu__mem_events_name(struct perf_pmu *pmu, int i,
 					     char *buf, size_t buf_size)
 {
 	struct perf_mem_event *e;
+	const char *name;
 
 	if (i >= PERF_MEM_EVENTS__MAX || !pmu)
 		return NULL;
@@ -90,24 +91,26 @@ static const char *perf_pmu__mem_events_name(struct perf_pmu *pmu, int i,
 	if (!e || !e->name)
 		return NULL;
 
+	name = e->name;
+
 	if (i == PERF_MEM_EVENTS__LOAD || i == PERF_MEM_EVENTS__LOAD_STORE) {
 		if (e->ldlat) {
 			if (!e->aux_event) {
 				/* ARM and Most of Intel */
 				scnprintf(buf, buf_size,
-					  e->name, pmu->name,
+					  name, pmu->name,
 					  perf_mem_events__loads_ldlat);
 			} else {
 				/* Intel with mem-loads-aux event */
 				scnprintf(buf, buf_size,
-					  e->name, pmu->name, pmu->name,
+					  name, pmu->name, pmu->name,
 					  perf_mem_events__loads_ldlat);
 			}
 		} else {
 			if (!e->aux_event) {
 				/* AMD and POWER */
 				scnprintf(buf, buf_size,
-					  e->name, pmu->name);
+					  name, pmu->name);
 			} else {
 				return NULL;
 			}
@@ -117,7 +120,7 @@ static const char *perf_pmu__mem_events_name(struct perf_pmu *pmu, int i,
 
 	if (i == PERF_MEM_EVENTS__STORE) {
 		scnprintf(buf, buf_size,
-			  e->name, pmu->name);
+			  name, pmu->name);
 		return buf;
 	}
 
-- 
2.53.0

Re: [PATCH v4 6/6] perf mem record: Use the IBS swfilt filter when available

[Ravi Bangoria]

Hi Arnaldo,

> IBS events with exclude_{user,kernel} bits, as used for per-thread
> recording when kernel samples are not allowed, are rejected on hardware
> without the privilege filter, so per-thread 'perf mem record' fails on
> AMD:
> 
>   $ perf mem record -o /dev/null -- true
>   Failure to open event 'ibs_op/ldlat=0/u' on PMU 'ibs_op' which will be removed.
> 
> Kernel v6.14 added swfilt, a software privilege filter exposed as the
> 'swfilt' format term, making those events usable per-thread.  Give the
> ibs_op memory events extra tables with the term, selected in
> perf_pmu__arch_init() when the PMU exposes it, keeping the names that
> need system wide mode otherwise; the knowledge that IBS needs this
> stays in the arch code.
> 
> Suggested-by: Namhyung Kim <namhyung@kernel.org>
> Suggested-by: Ravi Bangoria <ravi.bangoria@amd.com>
> Assisted-by: LLM
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ravi Bangoria <ravi.bangoria@amd.com>

> + * ... Keep it even when
> + * the event has no exclude bits yet, 'perf record' adds them on the
> + * first open failure and retries with the same name.

Currently, 'perf record' fails rather than silently retrying with
/swfilt=1/. The reason to not transparently add /swfilt=1/ is to
make the user aware that software filtering is being used, and it
is not overhead free.

Thanks,
Ravi

Re: [PATCH v4 6/6] perf mem record: Use the IBS swfilt filter when available

[Arnaldo Carvalho de Melo]

On Fri, Sep 25, 2026 at 09:32:42AM +0530, Ravi Bangoria wrote:
> Hi Arnaldo,
> 
> > IBS events with exclude_{user,kernel} bits, as used for per-thread
> > recording when kernel samples are not allowed, are rejected on hardware
> > without the privilege filter, so per-thread 'perf mem record' fails on
> > AMD:
> > 
> >   $ perf mem record -o /dev/null -- true
> >   Failure to open event 'ibs_op/ldlat=0/u' on PMU 'ibs_op' which will be removed.
> > 
> > Kernel v6.14 added swfilt, a software privilege filter exposed as the
> > 'swfilt' format term, making those events usable per-thread.  Give the
> > ibs_op memory events extra tables with the term, selected in
> > perf_pmu__arch_init() when the PMU exposes it, keeping the names that
> > need system wide mode otherwise; the knowledge that IBS needs this
> > stays in the arch code.
> > 
> > Suggested-by: Namhyung Kim <namhyung@kernel.org>
> > Suggested-by: Ravi Bangoria <ravi.bangoria@amd.com>
> > Assisted-by: LLM
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> Reviewed-by: Ravi Bangoria <ravi.bangoria@amd.com>
> 
> > + * ... Keep it even when
> > + * the event has no exclude bits yet, 'perf record' adds them on the
> > + * first open failure and retries with the same name.
> 
> Currently, 'perf record' fails rather than silently retrying with
> /swfilt=1/. The reason to not transparently add /swfilt=1/ is to
> make the user aware that software filtering is being used, and it
> is not overhead free.

Made it more concise:

+/*
+ * IBS events with exclude_{user,kernel} bits are rejected on hardware
+ * without the privilege filter unless swfilt is used: the extra tables
+ * carry the term, selected when the PMU exposes it.
+ */
 struct perf_mem_event perf_mem_events_amd[PERF_MEM_EVENTS__MAX] = {
        E(NULL,         NULL,           NULL,   false,  0),
        E(NULL,         NULL,           NULL,   false,  0),
        E("mem-ldst",   "%s//",         NULL,   false,  0),
 };

+struct perf_mem_event perf_mem_events_amd_swfilt[PERF_MEM_EVENTS__MAX] = {
+       E(NULL,         NULL,           NULL,   false,  0),
+       E(NULL,         NULL,           NULL,   false,  0),
+       E("mem-ldst",   "%s/swfilt=1/", NULL,   false,  0),
+};
+
 struct perf_mem_event perf_mem_events_amd_ldlat[PERF_MEM_EVENTS__MAX] = {
        E(NULL,         NULL,           NULL,   false,  0),
        E(NULL,         NULL,           NULL,   false,  0),
        E("mem-ldst",   "%s/ldlat=%u/", NULL,   true,   0),
 };
+
+struct perf_mem_event perf_mem_events_amd_ldlat_swfilt[PERF_MEM_EVENTS__MAX] = {
+       E(NULL,         NULL,                   NULL,   false,  0),
+       E(NULL,         NULL,                   NULL,   false,  0),
+       E("mem-ldst",   "%s/ldlat=%u,swfilt=1/",        NULL,   true,   0),
+};o
- Arnaldo

Re: [PATCH v4 6/6] perf mem record: Use the IBS swfilt filter when available

[Ravi Bangoria]

> Made it more concise:
> 
> +/*
> + * IBS events with exclude_{user,kernel} bits are rejected on hardware
> + * without the privilege filter unless swfilt is used: the extra tables
> + * carry the term, selected when the PMU exposes it.
> + */
Looks good. Thanks.

Ravi