[PATCH v2 0/3] perf annotate: Data type profiling support for C++ classes and virtual calls

[PATCH v2 0/3] perf annotate: Data type profiling support for C++ classes and virtual calls

[Yanbo Zhao]

Hello,

Data type profiling currently only understands C struct/union types.
For C++ workloads, member accesses through base class subobjects
cannot be resolved, and virtual function calls (indirect calls through
the vtable) lose the return type of the callee: the register holding
the returned value becomes unknown, which matters when it's used
directly to access memory like 'p->next()->val' where no DWARF
variable describes the temporary.

This series extends data type profiling to C++:

Patch 1 introduces die_is_compound_type() covering DW_TAG_class_type
as well and accepts DW_TAG_inheritance in the offset-based member
lookup so that it descends into base class subobjects.  It handles
empty base classes, members placed in the tail padding of a base and
virtual base classes.

Patch 2 adds the DWARF helpers for virtual calls: the vtable slot index
of a virtual function, the virtual function at a slot of a class
(following the primary base class chain), and the class of the vtable
pointer at an offset of a type.

Patch 3 tracks the vtable pointer and the virtual function pointer
loaded from it in the x86 instruction tracking, and resolves the
return type of 'call *N(%reg)' and 'call *%reg' through them.

Tested on x86-64 with GCC 15 (-O2 -g) using small programs covering
single/multiple inheritance, empty base optimization, tail padding
reuse, virtual inheritance, direct calls through the vtable and the
speculatively devirtualized form.  In all cases the access to the
returned pointer after a virtual call is annotated with the right type
and member, e.g.:

  movq    (%rbp), %rax           # data-type: struct Node +0 (_vptr.Node)
  movq    (%rax), %rax
  cmpq    %r14, %rax
  je      0x1240
  movq    %rbp, %rdi
  callq   *%rax
  addq    8(%rax), %r12          # data-type: struct Node +0x8 (val)

The existing results for C code are unchanged and the added cost on
the common path (a pointer dereference) is a tag check on the resolved
member type.

Changes in v2:

Patch 1:
- Explain DW_TAG_inheritance with an example DWARF in the commit
  message (Namhyung).
- Skip virtual base classes whose location is a runtime expression
  instead of falling back to offset 0 (Sashiko).
- Match a base class in the offset lookup only if it actually has a
  member at the offset, to handle empty base optimization and tail
  padding reuse where a member of the derived class shares the offset
  with the base (Sashiko).
- Keep looking at the next sibling in fill_member_name() when an
  anonymous child (base class) has nothing at the offset.

Patch 2:
- Drop the non-existent DW_AT_vtable_elem_index and the DW_LANG_*
  fallback macros (Namhyung).
- Drop cu_get_language(), cu_is_cplusplus(), die_get_base_class(),
  die_get_parent() and die_find_member_by_offset() which are not
  needed anymore (Namhyung).
- Document that die_get_vtable_index() returns the vtable slot index
  and that GCC and Clang both emit the index as DW_OP_constu
  (Namhyung, Sashiko).
- Fix die_find_virtual_func() to return the function DIE instead of
  the DW_TAG_inheritance DIE when found in a base class (Sashiko).
- Follow only the primary base class chain in die_find_virtual_func()
  since non-primary bases have their own secondary vtables, and skip
  an empty base at offset 0 which is not the primary base.
- Add die_get_vptr_class() to find the class of the vtable pointer
  through base class subobjects, and die_is_vtbl_ptr_type() to
  identify the vtable pointer by its type ('__vtbl_ptr_type').

Patch 3:
- Remove the receiver ('this' pointer) register update after the call
  which was dead code and not needed, and the arg0_reg field (Sashiko,
  Namhyung).  The 'this' pointer lives in a callee-saved register or
  on the stack across the call and DWARF location lists cover it.
- Handle 'call *%reg' by tracking the function pointer loaded from the
  vtable as TSR_KIND_VFUNC_PTR with its return type (Sashiko).  This
  form is common due to speculative devirtualization by GCC.
- Strip the leading '*' of an indirect call operand in call__parse()
  instead of extract_reg_offset() so that both forms are parsed.
- Ignore void virtual functions instead of aborting (Namhyung).
- Keep the existing pointer dereference branch and its fall-through
  intact; the vtable pointer is detected from the resolved member
  type there instead of a separate lookup before it.
- Treat the new register kinds as pointers when saved to the stack.

v1: https://lore.kernel.org/r/20260821050207.4517-1-yzhao62@ncsu.edu

Thanks,
Yanbo

Yanbo Zhao (3):
  perf dwarf-aux: Add die_is_compound_type() to handle C++ class types
  perf dwarf-aux: Add C++ vtable helpers
  perf annotate: Resolve C++ virtual function calls in x86 insn tracking

 tools/perf/util/annotate-arch/annotate-x86.c |  76 +++++-
 tools/perf/util/annotate-data.c              |  61 +++--
 tools/perf/util/annotate-data.h              |   4 +
 tools/perf/util/disasm.c                     |   5 +
 tools/perf/util/dwarf-aux.c                  | 265 ++++++++++++++++++-
 tools/perf/util/dwarf-aux.h                  |  21 ++
 6 files changed, 406 insertions(+), 26 deletions(-)

base-commit: 45d15e89a783a0a279b7a54f9018c230127380d7
-- 
2.53.0

[PATCH v2 1/3] perf dwarf-aux: Add die_is_compound_type() to handle C++ class types

[Yanbo Zhao]

Introduce the die_is_compound_type() helper which checks for
DW_TAG_structure_type, DW_TAG_union_type, and DW_TAG_class_type, and
convert the existing open-coded struct/union tag checks to use it:
- die_get_member_type() in dwarf-aux.c.
- is_compound_type() and set_stack_state() in annotate-data.c.

The switch in __add_member_cb() also handles unions and the nesting
limit, so DW_TAG_class_type is just added there as another case.

Also accept DW_TAG_inheritance in the member lookup callbacks
(__die_find_member_offset_cb() and __add_member_cb()) so that member
lookup by offset descends into C++ base class subobjects.

In DWARF, a base class subobject is described by a DW_TAG_inheritance
child of the derived class DIE.  It sits next to the DW_TAG_member
children and carries the same DW_AT_type and DW_AT_data_member_location
attributes, so it can be treated like an unnamed member whose type is
the base class.  Members inherited from the base are not repeated in
the derived class DIE; they can only be reached through the
DW_TAG_inheritance entry.

For example, with the following classes (GCC 15, -O2 -g):

  struct Base  { long a; virtual long get(long x); };
  struct Other { long c; virtual long hi(); };
  struct Multi : Base, Other { long d; ... };

the DWARF looks like this (subprogram DIEs omitted):

  <1><2f>: Abbrev Number: 15 (DW_TAG_structure_type)
     <30>   DW_AT_name        : Multi
     <34>   DW_AT_byte_size   : 40
  <2><3e>: Abbrev Number: 20 (DW_TAG_inheritance)
     <3f>   DW_AT_type        : <0x10b>
     <43>   DW_AT_data_member_location: 0
  <2><44>: Abbrev Number: 20 (DW_TAG_inheritance)
     <45>   DW_AT_type        : <0x1d0>
     <49>   DW_AT_data_member_location: 16
  <2><99>: Abbrev Number: 16 (DW_TAG_member)
     <9a>   DW_AT_name        : d
     <a1>   DW_AT_data_member_location: 32
  ...
  <1><10b>: Abbrev Number: 15 (DW_TAG_structure_type)
     <10c>   DW_AT_name        : Base
     <110>   DW_AT_byte_size   : 16
  <2><14d>: Abbrev Number: 25 (DW_TAG_member)
     <14e>   DW_AT_name        : _vptr.Base
     <156>   DW_AT_data_member_location: 0
  <2><156>: Abbrev Number: 16 (DW_TAG_member)
     <157>   DW_AT_name        : a
     <15e>   DW_AT_data_member_location: 8
  ...
  <1><1d0>: Abbrev Number: 15 (DW_TAG_structure_type)
     <1d1>   DW_AT_name        : Other
     <1d5>   DW_AT_byte_size   : 16
  <2><245>: Abbrev Number: 25 (DW_TAG_member)
     <246>   DW_AT_name        : _vptr.Other
     <24e>   DW_AT_data_member_location: 0
  <2><24e>: Abbrev Number: 16 (DW_TAG_member)
     <24f>   DW_AT_name        : c
     <256>   DW_AT_data_member_location: 8

An access to 'struct Multi' at offset 0x8 is Base::a, and one at
offset 0x18 is Other::c.  Neither of them is a DW_TAG_member of Multi.
With DW_TAG_inheritance accepted in __die_find_member_offset_cb(),
die_get_member_type() finds the DW_TAG_inheritance at offset 0 (size
16, covering 0x8), follows its DW_AT_type to Base, and then resolves
'a' at offset 0x8 within Base.  Likewise offset 0x18 goes through the
second DW_TAG_inheritance at 16 and resolves to 'c' at offset 0x8 in
Other.  The same applies to __add_member_cb() which builds the member
tree used to print 'Data Type Offset' names.

Note that this is only about the layout of data members: a base class
subobject is looked up in the same way regardless of how many base
classes the class has.  Resolving virtual function calls through the
vtable of a class with multiple base classes is a separate matter and
is described in the following patches.

Note that GCC emits DW_TAG_structure_type for a C++ 'struct' even when
it has virtual functions, and DW_TAG_class_type for 'class', so both
tags need to be treated as compound types.

A base class subobject differs from a member in three ways that need
care:

1. It can take less space than the size of the base class type.  An
   empty base occupies no bytes (empty base optimization) although the
   DW_AT_byte_size of the base type is 1, and a member of the derived
   class can be placed in the tail padding of the base:

     struct Empty {};
     struct EboD : Empty { long x; };         // Empty at 0, x at 0

     struct B { long a; int b; B(); };        // sizeof(B) == 16
     struct TailD : B { int c; };             // B at 0, c at 12

   The DW_TAG_inheritance comes before the DW_TAG_member in DWARF, so
   an offset-based lookup would match the base first and then fail to
   find the member in it.  To handle this, __die_find_member_offset_cb()
   only matches a DW_TAG_inheritance if the base class actually has a
   member at the offset, and fill_member_name() continues with the next
   sibling when an anonymous child (a base class or an anonymous
   struct/union) has nothing at the offset.

2. A virtual base class has a DW_AT_data_member_location that is a
   location expression evaluated at runtime with the vtable:

     struct V { long v; };
     struct VirtD : virtual V { long d; };

     <2><5d>: Abbrev Number: 13 (DW_TAG_inheritance)
        <5e>   DW_AT_type        : <0x2f>
        <62>   DW_AT_data_member_location: 6 byte block: 12 6 48 1c 6 22
                 (DW_OP_dup; DW_OP_deref; DW_OP_lit24; DW_OP_minus;
                  DW_OP_deref; DW_OP_plus)
        <69>   DW_AT_virtuality  : 1        (virtual)

   Its offset cannot be resolved statically, so such an entry is
   skipped in both __die_find_member_offset_cb() and __add_member_cb()
   instead of falling back to offset 0 which would shadow the vtable
   pointer of the derived class.

3. DW_TAG_inheritance has no DW_AT_name.  It's added to the member
   tree as an anonymous entry with the base class as its type name,
   so 'Data Type Offset' shows the member name without the base class,
   like 'struct Multi +0x8 (a)'.

With this, accesses through pointers to the types above are resolved
as below, where the first two were reported as '(no field)' before:

  struct EboD  +0    (x)
  struct TailD +0xc  (c)
  struct VirtD +0x8  (d)

This extends the existing member type resolution and data type
profiling state handling to C++ classes with inheritance without
changing behavior for C struct/union types.

Signed-off-by: Yanbo Zhao <yzhao62@ncsu.edu>
---
Changes in v2:
- Add an example DWARF of a class with base classes to the commit
  message and explain why DW_TAG_inheritance is handled like
  DW_TAG_member (Namhyung).
- Skip virtual base classes whose DW_AT_data_member_location is a
  runtime expression instead of falling back to offset 0, in both
  __die_find_member_offset_cb() and __add_member_cb() (Sashiko).
- Match a DW_TAG_inheritance in the offset lookup only if the base
  class actually has a member at the offset, so that a member of the
  derived class placed at the same offset as an empty base (EBO) or in
  the tail padding of the base is found (Sashiko).
- Continue with the next sibling in fill_member_name() when an
  anonymous child (base class) has nothing at the offset, so the member
  name is shown instead of '(no field)' in the cases above.

 tools/perf/util/annotate-data.c | 46 +++++++++++++++++++------------
 tools/perf/util/dwarf-aux.c     | 49 +++++++++++++++++++++++++++++----
 tools/perf/util/dwarf-aux.h     |  3 ++
 3 files changed, 76 insertions(+), 22 deletions(-)

diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 19a6ecd67f28..8a9d3f2eec4d 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -237,9 +237,17 @@ static int __add_member_cb(Dwarf_Die *die, void *arg)
 	Dwarf_Word size, loc, bit_size = 0;
 	Dwarf_Attribute attr;
 	struct strbuf sb;
-	int tag;
+	int tag = dwarf_tag(die);
+
+	if (tag != DW_TAG_member && tag != DW_TAG_inheritance)
+		return DIE_FIND_CB_SIBLING;
 
-	if (dwarf_tag(die) != DW_TAG_member)
+	/*
+	 * A virtual base class (C++) has a location expression evaluated
+	 * using the vtable at runtime, so its offset is not known here.
+	 */
+	if (tag == DW_TAG_inheritance &&
+	    die_get_data_member_location(die, &loc) < 0)
 		return DIE_FIND_CB_SIBLING;
 
 	if (die_get_real_type(die, &die_mem) == NULL)
@@ -321,6 +329,7 @@ static int __add_member_cb(Dwarf_Die *die, void *arg)
 		member->is_union = true;
 		/* fall through */
 	case DW_TAG_structure_type:
+	case DW_TAG_class_type:
 		/* Only aggregates have children to expand, so only they get truncated. */
 		if (member->depth >= MAX_MEMBER_DEPTH) {
 			/* Consumed by the JSON exporter added in a later series. */
@@ -405,8 +414,21 @@ static int fill_member_name(char *buf, size_t sz, struct annotated_member *m,
 		if (offset < child->offset || offset >= child->offset + child->size)
 			continue;
 
-		found = true;
-		break;
+		if (child->var_name) {
+			found = true;
+			break;
+		}
+
+		/*
+		 * An anonymous child is a C++ base class or an anonymous
+		 * struct/union.  A base class subobject can share the offset
+		 * with a member of the derived class (empty base or tail
+		 * padding reuse), so keep looking if nothing is found in it.
+		 */
+		len = fill_member_name(buf, sz, child, offset, first,
+				       has_flex_array);
+		if (len)
+			return len;
 	}
 
 	if (!found && has_flex_array) {
@@ -565,9 +587,7 @@ static const char *match_result_str(enum type_match_result tmr)
 
 static bool is_compound_type(Dwarf_Die *type_die)
 {
-	int tag = dwarf_tag(type_die);
-
-	return tag == DW_TAG_structure_type || tag == DW_TAG_union_type;
+	return die_is_compound_type(type_die);
 }
 
 /* returns if Type B has better information than Type A */
@@ -694,7 +714,6 @@ struct type_state_stack *find_stack_state(struct type_state *state,
 void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
 			    Dwarf_Die *type_die, int ptr_offset)
 {
-	int tag;
 	Dwarf_Word size;
 
 	if (kind == TSR_KIND_POINTER) {
@@ -715,17 +734,10 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
 		return;
 	}
 
-	tag = dwarf_tag(type_die);
-
-	switch (tag) {
-	case DW_TAG_structure_type:
-	case DW_TAG_union_type:
+	if (die_is_compound_type(type_die))
 		stack->compound = (kind != TSR_KIND_PERCPU_POINTER);
-		break;
-	default:
+	else
 		stack->compound = false;
-		break;
-	}
 }
 
 struct type_state_stack *findnew_stack_state(struct type_state *state,
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 54f8b5ec74a2..eb4b8f3475df 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -61,6 +61,14 @@ const char *cu_get_comp_dir(Dwarf_Die *cu_die)
 	return dwarf_formstring(&attr);
 }
 
+bool die_is_compound_type(Dwarf_Die *type_die)
+{
+	int tag = dwarf_tag(type_die);
+
+	return tag == DW_TAG_structure_type || tag == DW_TAG_union_type ||
+	       tag == DW_TAG_class_type;
+}
+
 /* Unlike dwarf_getsrc_die(), cu_getsrc_die() only returns statement line */
 static Dwarf_Line *cu_getsrc_die(Dwarf_Die *cu_die, Dwarf_Addr addr)
 {
@@ -2150,13 +2158,21 @@ static int __die_find_member_offset_cb(Dwarf_Die *die_mem, void *arg)
 	Dwarf_Word offset = (long)arg;
 	int tag = dwarf_tag(die_mem);
 
-	if (tag != DW_TAG_member)
+	if (tag != DW_TAG_member && tag != DW_TAG_inheritance)
 		return DIE_FIND_CB_SIBLING;
 
 	/* Unions might not have location */
 	if (die_get_data_member_location(die_mem, &loc) < 0) {
 		Dwarf_Attribute attr;
 
+		/*
+		 * A virtual base class (C++) has a location expression that
+		 * needs the vtable at runtime.  Skip it as it cannot be
+		 * resolved statically.
+		 */
+		if (tag == DW_TAG_inheritance)
+			return DIE_FIND_CB_SIBLING;
+
 		if (dwarf_attr_integrate(die_mem, DW_AT_data_bit_offset, &attr) &&
 		    dwarf_formudata(&attr, &loc) == 0)
 			loc /= 8;
@@ -2164,6 +2180,30 @@ static int __die_find_member_offset_cb(Dwarf_Die *die_mem, void *arg)
 			loc = 0;
 	}
 
+	if (tag == DW_TAG_inheritance) {
+		Dwarf_Die base_die, member_die;
+
+		/*
+		 * A base class subobject can be smaller than the size of the
+		 * class type: an empty base takes no space (EBO) and a member
+		 * of the derived class can be placed in the tail padding of
+		 * the base.  In both cases a member of the derived class is
+		 * at the same offset as the base, so only match the base if
+		 * it actually has a member at the offset.
+		 */
+		if (offset < loc)
+			return DIE_FIND_CB_SIBLING;
+
+		if (die_get_real_type(die_mem, &base_die) == NULL)
+			return DIE_FIND_CB_SIBLING;
+
+		if (die_find_child(&base_die, __die_find_member_offset_cb,
+				   (void *)(long)(offset - loc), &member_die))
+			return DIE_FIND_CB_END;
+
+		return DIE_FIND_CB_SIBLING;
+	}
+
 	if (offset == loc)
 		return DIE_FIND_CB_END;
 
@@ -2201,7 +2241,7 @@ Dwarf_Die *die_get_member_type(Dwarf_Die *type_die, int offset,
 
 	tag = dwarf_tag(type_die);
 	/* If it's not a compound type, return the type directly */
-	if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type) {
+	if (!die_is_compound_type(type_die)) {
 		Dwarf_Word size;
 
 		if (dwarf_aggregate_size(type_die, &size) < 0)
@@ -2216,7 +2256,7 @@ Dwarf_Die *die_get_member_type(Dwarf_Die *type_die, int offset,
 
 	mb_type = *type_die;
 	/* TODO: Handle union types better? */
-	while (tag == DW_TAG_structure_type || tag == DW_TAG_union_type) {
+	while (die_is_compound_type(&mb_type)) {
 		member = die_find_child(&mb_type, __die_find_member_offset_cb,
 					(void *)(long)offset, die_mem);
 		if (member == NULL)
@@ -2227,8 +2267,7 @@ Dwarf_Die *die_get_member_type(Dwarf_Die *type_die, int offset,
 
 		tag = dwarf_tag(&mb_type);
 
-		if (tag == DW_TAG_structure_type || tag == DW_TAG_union_type ||
-		    tag == DW_TAG_array_type) {
+		if (die_is_compound_type(&mb_type) || tag == DW_TAG_array_type) {
 			Dwarf_Word loc;
 
 			/* Update offset for the start of the member struct */
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index 6f9145510adc..7c893e385824 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -23,6 +23,9 @@ const char *cu_find_realpath(Dwarf_Die *cu_die, const char *fname);
 /* Get DW_AT_comp_dir (should be NULL with older gcc) */
 const char *cu_get_comp_dir(Dwarf_Die *cu_die);
 
+/* Check if DIE is a compound type (structure, union, or class) */
+bool die_is_compound_type(Dwarf_Die *type_die);
+
 /* Get a line number and file name for given address */
 int cu_find_lineinfo(Dwarf_Die *cudie, Dwarf_Addr addr,
 		     const char **fname, int *lineno);
-- 
2.53.0

[PATCH v2 2/3] perf dwarf-aux: Add C++ vtable helpers

[Yanbo Zhao]

Add DWARF helper functions needed to resolve C++ virtual function calls
statically in the data type profiling:

- die_get_vtable_index() returns the vtable slot index of a virtual
  function DIE from DW_AT_vtable_elem_location.  DWARF defines the
  attribute as a location description of the slot but GCC and Clang
  both emit the slot index as a single DW_OP_constu, which is also how
  debuggers interpret it.

- die_find_virtual_func() finds the virtual function DIE at a given
  vtable slot index of a class.  The (primary) vtable of a class starts
  with the vtable of its primary base class, i.e. the first non-virtual
  base class at offset 0 which has a vtable, and the slots for virtual
  functions introduced by the class come after that.  So it looks at
  the class first and then follows the primary base class chain.
  Non-primary base classes have their own secondary vtables with a
  different slot numbering, so they are not searched.  Note that an
  empty base class can be placed at offset 0 too (empty base
  optimization) and it comes before the primary base in DWARF if it's
  declared first, like 'struct Derived : Empty, Base'.  So the base
  class needs to be checked if it has the vtable pointer at offset 0.

- die_is_vtbl_ptr_type() checks if a type is the vtable pointer.
  Compilers add the vtable pointer as an artificial member named
  '_vptr.<class>' (GCC) or '_vptr$<class>' (Clang) to the class that
  first introduces virtual functions, and both give it a type of
  pointer to a pointer type named '__vtbl_ptr_type'.  Checking the
  type is cheap once a member type is resolved, so the instruction
  tracking can use it on every pointer access without an extra DWARF
  lookup.

- die_get_vptr_class() checks if the member at an offset of a class is
  the vtable pointer, following DW_TAG_inheritance into base class
  subobjects like die_get_member_type(), and returns the class whose
  vtable it points to.  Since
  the primary vtable of a derived class extends the one of its base,
  the vtable pointer at offset 0 is returned as the vtable of the
  (derived) class itself, and only when a non-primary base subobject at
  a non-zero offset is entered it's the vtable of that base class.
  die_deref_vptr_class() is a variant for an access through a pointer
  to the class like die_deref_ptr_type().

For example, with the following classes (GCC 15, -O2 -g):

  struct Base {
      long a;
      virtual long get(long x);
      virtual void set(long v);
  };
  struct Other {
      long c;
      virtual long hi();
  };
  struct Multi : Base, Other {
      long d;
      long get(long x) override;
      long hi() override;
  };

the vtable pointers and slot indexes are described like below:

  <1><10b>: Abbrev Number: 15 (DW_TAG_structure_type)
     <10c>   DW_AT_name        : Base
  <2><14d>: Abbrev Number: 25 (DW_TAG_member)
     <14e>   DW_AT_name        : _vptr.Base
     <152>   DW_AT_type        : <0x2bf>
     <156>   DW_AT_data_member_location: 0
  <2><15f>: Abbrev Number: 17 (DW_TAG_subprogram)
     <160>   DW_AT_name        : get
     <16e>   DW_AT_virtuality  : 1        (virtual)
     <16e>  DW_AT_vtable_elem_location: (DW_OP_constu: 0)
  <2><188>: Abbrev Number: 38 (DW_TAG_subprogram)
     <189>   DW_AT_name        : set
     <194>   DW_AT_virtuality  : 1        (virtual)
     <195>  DW_AT_vtable_elem_location: (DW_OP_constu: 1)
  ...
  <1><2f>: Abbrev Number: 15 (DW_TAG_structure_type)
     <30>   DW_AT_name        : Multi
  <2><3e>: Abbrev Number: 20 (DW_TAG_inheritance)
     <3f>   DW_AT_type        : <0x10b>                        # Base
     <43>   DW_AT_data_member_location: 0
  <2><44>: Abbrev Number: 20 (DW_TAG_inheritance)
     <45>   DW_AT_type        : <0x1d0>                        # Other
     <49>   DW_AT_data_member_location: 16
  <2><a2>: Abbrev Number: 17 (DW_TAG_subprogram)
     <a3>   DW_AT_name        : get
     <b1>   DW_AT_vtable_elem_location: (DW_OP_constu: 0)
  <2><cb>: Abbrev Number: 17 (DW_TAG_subprogram)
     <cc>   DW_AT_name        : hi
     <d9>   DW_AT_vtable_elem_location: (DW_OP_constu: 4)

Multi::hi() is at slot 4 of the primary vtable of Multi (after get,
set and the two destructor entries), while Other::hi() is at slot 0
of the vtable of Other.  Thus for a 'Multi *' the vtable pointer at
offset 0 is looked up with Multi: slot 4 finds Multi::hi() and slot 1
finds Base::set() through the primary base.  The vtable pointer at
offset 16 belongs to the Other subobject, so it's looked up with Other
where slot 0 finds Other::hi().

Signed-off-by: Yanbo Zhao <yzhao62@ncsu.edu>
---
Changes in v2:
- Drop the DW_AT_vtable_elem_index fallback which does not exist in
  DWARF (0x8c is DW_AT_loclists_base) (Namhyung).
- Drop the DW_LANG_C_plus_plus_* fallback macros; they are enum
  constants and the language check is not needed anymore (Namhyung).
- Drop cu_get_language(), cu_is_cplusplus(), die_get_base_class(),
  die_get_parent() and die_find_member_by_offset() which are unused
  or duplicate die_get_member_type() (Namhyung, Sashiko).
- Document that die_get_vtable_index() returns the vtable slot index
  of the function.  GCC and Clang both emit DW_AT_vtable_elem_location
  as a single DW_OP_constu with the slot index, not a byte offset
  (Namhyung, Sashiko).
- Fix die_find_virtual_func() to return the DW_TAG_subprogram DIE
  instead of the DW_TAG_inheritance DIE when the function is found in
  a base class (Sashiko).
- Follow only the primary base class chain in die_find_virtual_func()
  since non-primary base classes have their own secondary vtables with
  a different slot numbering, and skip an empty base class at offset 0
  which comes before the primary base in DWARF.
- Add die_is_vtbl_ptr_type() to identify the vtable pointer by its
  type ('__vtbl_ptr_type') and die_get_vptr_class() /
  die_deref_vptr_class() to get the class of the vtable pointer
  through base class subobjects, replacing die_find_member_by_offset()
  and die_is_vptr_member().

 tools/perf/util/dwarf-aux.c | 216 ++++++++++++++++++++++++++++++++++++
 tools/perf/util/dwarf-aux.h |  18 +++
 2 files changed, 234 insertions(+)

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index eb4b8f3475df..b1f3dca057ab 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -2293,6 +2293,222 @@ Dwarf_Die *die_get_member_type(Dwarf_Die *type_die, int offset,
 	return die_mem;
 }
 
+/**
+ * die_get_vtable_index - Get the vtable slot index of a virtual function
+ * @func_die: a DW_TAG_subprogram DIE
+ *
+ * Returns the index of the slot of @func_die in the vtable of its class or
+ * -1 if it's not a virtual function.  DWARF defines the
+ * DW_AT_vtable_elem_location as a location description of the slot, but
+ * both GCC and Clang emit the slot index as a single DW_OP_constu (or
+ * DW_OP_litN) and it's what debuggers expect too.
+ */
+int die_get_vtable_index(Dwarf_Die *func_die)
+{
+	Dwarf_Attribute attr;
+	Dwarf_Op *expr;
+	size_t nexpr;
+
+	if (dwarf_attr_integrate(func_die, DW_AT_vtable_elem_location, &attr) == NULL)
+		return -1;
+
+	if (dwarf_getlocation(&attr, &expr, &nexpr) < 0 || nexpr != 1)
+		return -1;
+
+	if (expr[0].atom == DW_OP_constu)
+		return expr[0].number;
+
+	if (expr[0].atom >= DW_OP_lit0 && expr[0].atom <= DW_OP_lit31)
+		return expr[0].atom - DW_OP_lit0;
+
+	pr_debug("Unexpected vtable_elem_location OP %x\n", expr[0].atom);
+	return -1;
+}
+
+static int __die_find_virtual_func_cb(Dwarf_Die *die_mem, void *arg)
+{
+	int index = (long)arg;
+
+	if (dwarf_tag(die_mem) != DW_TAG_subprogram)
+		return DIE_FIND_CB_SIBLING;
+
+	if (die_get_vtable_index(die_mem) == index)
+		return DIE_FIND_CB_END;
+
+	return DIE_FIND_CB_SIBLING;
+}
+
+/*
+ * Find the primary base class: the first non-virtual base class at offset 0
+ * which has a vtable.  Note that an empty base class can also be placed at
+ * offset 0 (empty base optimization) before the primary base.
+ */
+static int __die_find_primary_base_cb(Dwarf_Die *die_mem, void *arg __maybe_unused)
+{
+	Dwarf_Attribute attr;
+	Dwarf_Die base_die, vptr_die;
+	Dwarf_Word loc;
+
+	if (dwarf_tag(die_mem) != DW_TAG_inheritance)
+		return DIE_FIND_CB_SIBLING;
+
+	if (dwarf_attr_integrate(die_mem, DW_AT_virtuality, &attr))
+		return DIE_FIND_CB_SIBLING;
+
+	if (die_get_data_member_location(die_mem, &loc) < 0 || loc != 0)
+		return DIE_FIND_CB_SIBLING;
+
+	if (die_get_real_type(die_mem, &base_die) == NULL)
+		return DIE_FIND_CB_SIBLING;
+
+	/* It should have the vtable pointer at offset 0 */
+	if (die_get_vptr_class(&base_die, 0, &vptr_die) == NULL)
+		return DIE_FIND_CB_SIBLING;
+
+	return DIE_FIND_CB_END;
+}
+
+/**
+ * die_find_virtual_func - Find a virtual function by its vtable slot index
+ * @class_die: a class type DIE
+ * @index: index of the slot in the vtable of @class_die
+ * @die_mem: a buffer for result DIE
+ *
+ * Search a DW_TAG_subprogram DIE at the slot @index of the vtable of
+ * @class_die and store the DIE to @die_mem and returns it if found.  The
+ * (primary) vtable of a class starts with the vtable of its primary base
+ * class, that is the first non-virtual base class at offset 0 which has a
+ * vtable, and virtual functions introduced by the class come after that.
+ * So if @class_die doesn't have a virtual function with the @index, it
+ * continues to the primary base.  Other (non-primary) base classes have
+ * their own secondary vtables where the index is different, so they are
+ * not searched.
+ *
+ * Returns NULL if not found.
+ */
+Dwarf_Die *die_find_virtual_func(Dwarf_Die *class_die, int index,
+				 Dwarf_Die *die_mem)
+{
+	Dwarf_Die cur_die = *class_die;
+	Dwarf_Die base_die;
+
+	while (die_is_compound_type(&cur_die)) {
+		if (die_find_child(&cur_die, __die_find_virtual_func_cb,
+				   (void *)(long)index, die_mem))
+			return die_mem;
+
+		if (die_find_child(&cur_die, __die_find_primary_base_cb,
+				   NULL, &base_die) == NULL)
+			break;
+
+		if (die_get_real_type(&base_die, &cur_die) == NULL)
+			break;
+	}
+	return NULL;
+}
+
+/**
+ * die_is_vtbl_ptr_type - Check if the type is a C++ vtable pointer
+ * @type_die: a type DIE
+ *
+ * Compilers add the vtable pointer as an artificial member to the class
+ * that introduces virtual functions first.  Both GCC and Clang give it a
+ * type of pointer to a pointer type named "__vtbl_ptr_type", so it can
+ * be identified by the type without looking at the member name.
+ */
+bool die_is_vtbl_ptr_type(Dwarf_Die *type_die)
+{
+	Dwarf_Die ptr_die;
+	const char *name;
+
+	if (dwarf_tag(type_die) != DW_TAG_pointer_type)
+		return false;
+
+	if (die_get_real_type(type_die, &ptr_die) == NULL)
+		return false;
+
+	name = dwarf_diename(&ptr_die);
+	return name != NULL && !strcmp(name, "__vtbl_ptr_type");
+}
+
+/**
+ * die_get_vptr_class - Get the class of the vtable pointer at the offset
+ * @type_die: a class type DIE
+ * @offset: offset of the accessed member in @type_die
+ * @die_mem: a buffer for result DIE
+ *
+ * Check if the member at @offset in @type_die is a vtable pointer and
+ * return the class DIE whose vtable it points to.  The vtable pointer is
+ * a member of the class that introduces virtual functions first, so it
+ * follows DW_TAG_inheritance to look into the base class subobjects like
+ * die_get_member_type().
+ *
+ * Note that the vtable pointer at offset 0 belongs to the (primary) vtable
+ * of @type_die itself as it extends the one of the primary base class.
+ * Only when a non-primary base class subobject (at non-zero offset) is
+ * entered, it points to a separate vtable of the base class.
+ *
+ * Returns NULL if the member at @offset is not a vtable pointer.
+ */
+Dwarf_Die *die_get_vptr_class(Dwarf_Die *type_die, int offset,
+			      Dwarf_Die *die_mem)
+{
+	Dwarf_Die class_die = *type_die;
+	Dwarf_Die vptr_class = *type_die;
+	Dwarf_Die member_die, mb_type;
+	Dwarf_Word loc;
+
+	while (die_is_compound_type(&class_die)) {
+		if (die_find_child(&class_die, __die_find_member_offset_cb,
+				   (void *)(long)offset, &member_die) == NULL)
+			return NULL;
+
+		if (dwarf_tag(&member_die) == DW_TAG_member) {
+			if (die_get_real_type(&member_die, &mb_type) == NULL ||
+			    !die_is_vtbl_ptr_type(&mb_type))
+				return NULL;
+
+			*die_mem = vptr_class;
+			return die_mem;
+		}
+
+		/* DW_TAG_inheritance: go into the base class subobject */
+		if (die_get_data_member_location(&member_die, &loc) < 0)
+			return NULL;
+
+		if (die_get_real_type(&member_die, &class_die) == NULL)
+			return NULL;
+
+		offset -= loc;
+		if (loc != 0)
+			vptr_class = class_die;
+	}
+	return NULL;
+}
+
+/**
+ * die_deref_vptr_class - Get the class of the vtable pointer via pointer access
+ * @ptr_die: a pointer type DIE
+ * @offset: offset of the accessed member from the pointer
+ * @die_mem: a buffer for result DIE
+ *
+ * Same as die_get_vptr_class() but for an access through a pointer to the
+ * class, like die_deref_ptr_type().
+ */
+Dwarf_Die *die_deref_vptr_class(Dwarf_Die *ptr_die, int offset,
+				Dwarf_Die *die_mem)
+{
+	Dwarf_Die type_die;
+
+	if (dwarf_tag(ptr_die) != DW_TAG_pointer_type)
+		return NULL;
+
+	if (die_get_real_type(ptr_die, &type_die) == NULL)
+		return NULL;
+
+	return die_get_vptr_class(&type_die, offset, die_mem);
+}
+
 /**
  * die_deref_ptr_type - Return type info for pointer access
  * @ptr_die: a pointer type DIE
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index 7c893e385824..803182cdff8a 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -26,6 +26,24 @@ const char *cu_get_comp_dir(Dwarf_Die *cu_die);
 /* Check if DIE is a compound type (structure, union, or class) */
 bool die_is_compound_type(Dwarf_Die *type_die);
 
+/* Get the vtable slot index of a virtual function (-1 if not virtual) */
+int die_get_vtable_index(Dwarf_Die *func_die);
+
+/* Find the virtual function at the vtable slot index of a class */
+Dwarf_Die *die_find_virtual_func(Dwarf_Die *class_die, int index,
+				 Dwarf_Die *die_mem);
+
+/* Check if the type is a C++ vtable pointer (pointer to __vtbl_ptr_type) */
+bool die_is_vtbl_ptr_type(Dwarf_Die *type_die);
+
+/* Get the class of the vtable pointer at the offset of a type (or NULL) */
+Dwarf_Die *die_get_vptr_class(Dwarf_Die *type_die, int offset,
+			      Dwarf_Die *die_mem);
+
+/* Get the class of the vtable pointer at the offset from a pointer (or NULL) */
+Dwarf_Die *die_deref_vptr_class(Dwarf_Die *ptr_die, int offset,
+				Dwarf_Die *die_mem);
+
 /* Get a line number and file name for given address */
 int cu_find_lineinfo(Dwarf_Die *cudie, Dwarf_Addr addr,
 		     const char **fname, int *lineno);
-- 
2.53.0

[PATCH v2 3/3] perf annotate: Resolve C++ virtual function calls in x86 insn tracking

[Yanbo Zhao]

A C++ virtual function call is an indirect call through the vtable.
The instruction tracking of the data type profiling cannot resolve the
callee name for it, so the return type is lost and the type of the
register holding the returned value is unknown after the call.  It
matters when the returned value is used directly to access memory, like
in 'p->next()->val', where no DWARF variable describes the temporary.

The vtable pointer and the slot index are known statically from the
type of the object pointer, so the callee can be found by DWARF.  Track
the two steps of a virtual call in update_insn_state_x86():

1. Loading the vtable pointer from the object.  When a load from a
   register with a pointer type hits the '_vptr' member of the class
   (or one of its base classes), the destination register is marked as
   TSR_KIND_VTABLE_PTR with the class DIE returned by
   die_get_vptr_class().

2. Calling through the vtable.  GCC emits the call in two forms:

     mov    (%rbx),%rax          # vtable pointer
     call   *0x8(%rax)           # slot 1

   or, with speculative devirtualization when it can guess the target,
   the function pointer is loaded first and compared with the guess:

     mov    (%rbx),%rax          # vtable pointer
     mov    0x8(%rax),%rax       # function pointer at slot 1
     cmp    %r14,%rax
     je     <inlined copy>
     call   *%rax

   For the first form, the call handler resolves the slot from the
   offset of the target operand when the base register is a vtable
   pointer.  For the second form, the load from a vtable pointer
   register marks the destination as TSR_KIND_VFUNC_PTR holding the
   return type of the function at the slot, and the call handler uses
   it when the target operand is such a register.  In both cases the
   function DIE is found by die_find_virtual_func() and its return
   type is set to the return value register like a direct call.  Void
   functions have no return type and are simply ignored.

The resolution happens before the caller-saved registers are
invalidated since the register used for the call is one of them.

To let the call handler see the target operand, call__parse() saves
the operand of an indirect call (without the leading '*') to
ops->target.raw so that annotate_get_insn_location() can extract the
register and offset for both '*0x8(%rax)' and '*%rax'.

The new kinds are pointer-sized and not compound when saved to the
stack, and pr_debug_type_name() knows how to print them.

The slot index is calculated with the host pointer size like the
other places for now.  Only the primary vtable is handled: a virtual
call through a pointer to a non-primary base class subobject (multiple
inheritance) gets the class of that base from die_get_vptr_class() so
the slot index is looked up in the right vtable, but the receiver
adjustment (this pointer thunks) is not tracked.

For example, with the following code built with -O2 -g:

  struct Node {
      long val;
      Node *nxt;
      virtual Node *next() { return nxt; }
      ...
  };

  long run(Node *p, long n)
  {
      long s = 0;
      for (long i = 0; i < n; i++)
          s += p->next()->val;
      return s;
  }

'perf annotate --code-with-type' now shows the type for the access to
the returned pointer after the call which was unknown before:

  1251:  movq  (%rbp), %rax      # data-type: struct Node +0 (_vptr.Node)
  1255:  movq  (%rax), %rax
  1258:  cmpq  %r14, %rax
  125b:  je    0x1240
  125d:  movq  %rbp, %rdi
  1260:  addq  $1, %rbx
  1264:  callq *%rax
  1266:  addq  8(%rax), %r12     # data-type: struct Node +0x8 (val)

Signed-off-by: Yanbo Zhao <yzhao62@ncsu.edu>
---
Changes in v2:
- Remove the receiver ('this' pointer) register update after the call
  and the type_state::arg0_reg field.  The code was unreachable since
  the register is caller-saved and already invalidated, and it's not
  needed: the 'this' pointer lives in a callee-saved register or on
  the stack across the call and the DWARF location lists cover it
  (Sashiko, Namhyung).
- Handle 'call *%reg' in addition to 'call *N(%reg)'.  GCC emits it
  with speculative devirtualization: the function pointer is loaded
  from the vtable, compared with the guessed target and then called.
  The load is tracked as TSR_KIND_VFUNC_PTR holding the return type
  of the function at the slot (Sashiko).
- Strip the leading '*' of an indirect call operand in call__parse()
  when saving ops->target.raw, instead of in extract_reg_offset()
  which is not reached for '*%reg' (Sashiko).
- Ignore virtual functions returning void (no return type) instead of
  treating it as a failure (Namhyung).
- Keep the existing pointer dereference branch and its fall-through
  to the multi-register and per-cpu paths intact.  The vtable pointer
  is detected from the member type resolved there, so the common
  path only pays a tag check.
- Treat the new register kinds as pointer-sized and non-compound when
  saved to the stack.
- Drop the unnecessary include of dwarf-aux.h and hist.h.

 tools/perf/util/annotate-arch/annotate-x86.c | 76 +++++++++++++++++++-
 tools/perf/util/annotate-data.c              | 15 +++-
 tools/perf/util/annotate-data.h              |  4 ++
 tools/perf/util/disasm.c                     |  5 ++
 4 files changed, 96 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/annotate-arch/annotate-x86.c b/tools/perf/util/annotate-arch/annotate-x86.c
index 1acf31a2c759..d21192b74b6b 100644
--- a/tools/perf/util/annotate-arch/annotate-x86.c
+++ b/tools/perf/util/annotate-arch/annotate-x86.c
@@ -216,6 +216,29 @@ static void invalidate_reg_state(struct type_state_reg *reg)
 	reg->copied_from = -1;
 }
 
+/*
+ * Get the return type of the C++ virtual function at the @offset in the
+ * vtable of @class_die.  Returns false if it's not found or the function
+ * returns void.
+ */
+static bool vtable_get_rettype(Dwarf_Die *class_die, int offset,
+			       Dwarf_Die *type_die)
+{
+	Dwarf_Die func_die;
+	int index;
+
+	if (offset < 0)
+		return false;
+
+	/* TODO: arch-dependent pointer size */
+	index = offset / sizeof(void *);
+
+	if (die_find_virtual_func(class_die, index, &func_die) == NULL)
+		return false;
+
+	return die_get_real_type(&func_die, type_die) != NULL;
+}
+
 static void update_insn_state_x86(struct type_state *state,
 				  struct data_loc_info *dloc, Dwarf_Die *cu_die,
 				  struct disasm_line *dl)
@@ -236,6 +259,7 @@ static void update_insn_state_x86(struct type_state *state,
 		struct symbol *func = dl->ops.target.sym;
 		const char *call_name;
 		u64 call_addr;
+		bool has_rettype = false;
 
 		/* Try to resolve the call target name */
 		if (func)
@@ -252,6 +276,26 @@ static void update_insn_state_x86(struct type_state *state,
 		else
 			pr_debug_dtp("call [%x] <unknown>\n", insn_offset);
 
+		/*
+		 * Resolve the return type of a C++ virtual function call
+		 * before invalidating the caller-saved register used for the
+		 * indirect call.  It's either through the vtable pointer
+		 * directly like 'call *0x8(%rax)' or through the function
+		 * pointer loaded from the vtable like 'call *%rax'.
+		 */
+		if (has_reg_type(state, dst->reg1) && state->regs[dst->reg1].ok) {
+			tsr = &state->regs[dst->reg1];
+
+			if (dst->mem_ref && !dst->multi_regs &&
+			    tsr->kind == TSR_KIND_VTABLE_PTR &&
+			    vtable_get_rettype(&tsr->type, dst->offset, &type_die)) {
+				has_rettype = true;
+			} else if (!dst->mem_ref && tsr->kind == TSR_KIND_VFUNC_PTR) {
+				type_die = tsr->type;
+				has_rettype = true;
+			}
+		}
+
 		/* Invalidate caller-saved registers after call */
 		call_addr = map__rip_2objdump(dloc->ms->map,
 					      dloc->ms->sym->start + dl->al.offset);
@@ -267,7 +311,10 @@ static void update_insn_state_x86(struct type_state *state,
 		}
 
 		/* Update register with the return type (if any) */
-		if (call_name && die_find_func_rettype(cu_die, call_name, &type_die)) {
+		if (call_name && die_find_func_rettype(cu_die, call_name, &type_die))
+			has_rettype = true;
+
+		if (has_rettype) {
 			tsr = &state->regs[state->ret_reg];
 			tsr->type = type_die;
 			tsr->kind = TSR_KIND_TYPE;
@@ -622,13 +669,38 @@ static void update_insn_state_x86(struct type_state *state,
 			}
 			pr_debug_type_name(&tsr->type, tsr->kind);
 		}
+		/* Load a function pointer from the vtable (for 'call *%reg') */
+		else if (has_reg_type(state, sreg) && state->regs[sreg].ok &&
+			 state->regs[sreg].kind == TSR_KIND_VTABLE_PTR &&
+			 vtable_get_rettype(&state->regs[sreg].type, src->offset,
+					    &type_die)) {
+			tsr->type = type_die;
+			tsr->kind = TSR_KIND_VFUNC_PTR;
+			tsr->offset = 0;
+			tsr->ok = true;
+
+			pr_debug_dtp("mov [%x] %#x(reg%d) -> reg%d",
+				     insn_offset, src->offset, sreg, dst->reg1);
+			pr_debug_type_name(&tsr->type, tsr->kind);
+		}
 		/* And then dereference the pointer if it has one */
 		else if (has_reg_type(state, sreg) && state->regs[sreg].ok &&
 			 state->regs[sreg].kind == TSR_KIND_TYPE &&
 			 die_deref_ptr_type(&state->regs[sreg].type,
 					    src->offset + state->regs[sreg].offset, &type_die)) {
+			/*
+			 * The vtable pointer of a C++ object needs the class
+			 * to resolve virtual calls through it.
+			 */
+			if (die_is_vtbl_ptr_type(&type_die) &&
+			    die_deref_vptr_class(&state->regs[sreg].type,
+						 src->offset + state->regs[sreg].offset,
+						 &type_die))
+				tsr->kind = TSR_KIND_VTABLE_PTR;
+			else
+				tsr->kind = TSR_KIND_TYPE;
+
 			tsr->type = type_die;
-			tsr->kind = TSR_KIND_TYPE;
 			tsr->offset = 0;
 			tsr->ok = true;
 
diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 8a9d3f2eec4d..e9bf76cc2863 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -67,6 +67,14 @@ void pr_debug_type_name(Dwarf_Die *die, enum type_state_kind kind)
 		pr_info(" pointer");
 		/* it also prints the type info */
 		break;
+	case TSR_KIND_VTABLE_PTR:
+		pr_info(" vtable pointer");
+		/* it also prints the type info */
+		break;
+	case TSR_KIND_VFUNC_PTR:
+		pr_info(" virtual function pointer returning");
+		/* it also prints the type info */
+		break;
 	case TSR_KIND_CANARY:
 		pr_info(" stack canary\n");
 		return;
@@ -715,8 +723,11 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
 			    Dwarf_Die *type_die, int ptr_offset)
 {
 	Dwarf_Word size;
+	bool is_pointer = (kind == TSR_KIND_POINTER ||
+			   kind == TSR_KIND_VTABLE_PTR ||
+			   kind == TSR_KIND_VFUNC_PTR);
 
-	if (kind == TSR_KIND_POINTER) {
+	if (is_pointer) {
 		/* TODO: arch-dependent pointer size */
 		size = sizeof(void *);
 	}
@@ -729,7 +740,7 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
 	stack->ptr_offset = ptr_offset;
 	stack->kind = kind;
 
-	if (kind == TSR_KIND_POINTER) {
+	if (is_pointer) {
 		stack->compound = false;
 		return;
 	}
diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
index bbf4fd35c1d7..14dab1beae2c 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -36,6 +36,10 @@ enum type_state_kind {
 	TSR_KIND_CONST,
 	TSR_KIND_PERCPU_POINTER,
 	TSR_KIND_POINTER,
+	/* C++ vtable pointer of the class in 'type' */
+	TSR_KIND_VTABLE_PTR,
+	/* C++ virtual function pointer with the return type in 'type' */
+	TSR_KIND_VFUNC_PTR,
 	TSR_KIND_CANARY,
 };
 
diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
index 5478c134e7e3..5d96ee4c866b 100644
--- a/tools/perf/util/disasm.c
+++ b/tools/perf/util/disasm.c
@@ -300,6 +300,11 @@ static int call__parse(const struct arch *arch, struct ins_operands *ops, struct
 	if (tok != NULL) {
 		endptr++;
 
+		/* Save the operand (without '*') to extract register and offset */
+		ops->target.raw = strdup(tok + 1);
+		if (ops->target.raw == NULL)
+			return -1;
+
 		/* Indirect call can use a non-rip register and offset: callq  *0x8(%rbx).
 		 * Do not parse such instruction.  */
 		if (strstr(endptr, "(%r") == NULL)
-- 
2.53.0

Re: [PATCH v2 1/3] perf dwarf-aux: Add die_is_compound_type() to handle C++ class types

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] perf/dwarf-aux: stack overflow in __die_find_member_offset_cb via cyclic inheritance
- [Medium] perf/dwarf-aux: flex array detection fails for C++ classes
--

commit 3f3b9b56b2e28f1713af3b40ab8946d5bbc8d245
Author: Yanbo Zhao <yzhao62@ncsu.edu>

perf dwarf-aux: Add die_is_compound_type() to handle C++ class types

This commit introduces die_is_compound_type() to support C++ classes alongside
structs and unions. It also updates offset lookups to descend into base class
subobjects using DW_TAG_inheritance entries.

> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 54f8b5ec74a2f..eb4b8f3475df1 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -61,6 +61,14 @@ const char *cu_get_comp_dir(Dwarf_Die *cu_die)
>  	return dwarf_formstring(&attr);
>  }
>  
> +bool die_is_compound_type(Dwarf_Die *type_die)
> +{
> +	int tag = dwarf_tag(type_die);
> +
> +	return tag == DW_TAG_structure_type || tag == DW_TAG_union_type ||
> +	       tag == DW_TAG_class_type;
> +}

[Severity: Medium]
Does die_has_flex_array_recurse() also need to be updated to use this new
helper?

If we look at die_has_flex_array_recurse() in tools/perf/util/dwarf-aux.c,
it currently forces flex array detection to return false for C++ classes
because it explicitly ignores DW_TAG_class_type:

    if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type)
        return false;
    ...
    if (tag == DW_TAG_structure_type && found) {
        if (is_flex_array_member(&last_mb))
            return true;

Could this cause accesses to flex array members inside C++ classes to be
wrongly rejected as out-of-bounds?

[ ... ]

> @@ -2164,6 +2180,30 @@ static int __die_find_member_offset_cb(Dwarf_Die *die_mem, void *arg)
>  			loc = 0;
>  	}
>  
> +	if (tag == DW_TAG_inheritance) {
> +		Dwarf_Die base_die, member_die;
> +
> +		/*
> +		 * A base class subobject can be smaller than the size of the
> +		 * class type: an empty base takes no space (EBO) and a member
> +		 * of the derived class can be placed in the tail padding of
> +		 * the base.  In both cases a member of the derived class is
> +		 * at the same offset as the base, so only match the base if
> +		 * it actually has a member at the offset.
> +		 */
> +		if (offset < loc)
> +			return DIE_FIND_CB_SIBLING;
> +
> +		if (die_get_real_type(die_mem, &base_die) == NULL)
> +			return DIE_FIND_CB_SIBLING;
> +
> +		if (die_find_child(&base_die, __die_find_member_offset_cb,
> +				   (void *)(long)(offset - loc), &member_die))
> +			return DIE_FIND_CB_END;

[Severity: High]
Does this recursive call into the DWARF child tree lack a depth limit?

Unlike __add_member_cb(), there is no recursion depth limit tracked or
enforced here. If a maliciously crafted or corrupted DWARF file contains
cyclic inheritance (e.g., A inherits from B, B inherits from A), could
this trigger unbounded mutual recursion between die_find_child() and
__die_find_member_offset_cb(), rapidly exhausting the stack?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930210038.196928-1-yzhao62@ncsu.edu?part=1

Re: [PATCH v2 2/3] perf dwarf-aux: Add C++ vtable helpers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] perf dwarf-aux: DW_AT_virtuality presence check incorrectly skips non-virtual bases
- [Medium] perf dwarf-aux: infinite loop in C++ inheritance traversal on malformed DWARF
--

commit e1f1b87424c8d58d923a8c3fbc1408fa0ae0c3b9
Author: Yanbo Zhao <yzhao62@ncsu.edu>

perf dwarf-aux: Add C++ vtable helpers
    
Add DWARF helper functions needed to resolve C++ virtual function calls
statically in the data type profiling.

> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index eb4b8f3475df1..b1f3dca057abb 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -2293,6 +2293,222 @@ Dwarf_Die *die_get_member_type(Dwarf_Die *type_die, int offset,
> [ ... ]
> +static int __die_find_primary_base_cb(Dwarf_Die *die_mem, void *arg __maybe_unused)
> +{
> +	Dwarf_Attribute attr;
> +	Dwarf_Die base_die, vptr_die;
> +	Dwarf_Word loc;
> +
> +	if (dwarf_tag(die_mem) != DW_TAG_inheritance)
> +		return DIE_FIND_CB_SIBLING;
> +
> +	if (dwarf_attr_integrate(die_mem, DW_AT_virtuality, &attr))
> +		return DIE_FIND_CB_SIBLING;

[Severity: Medium]
Does this code incorrectly skip valid primary base classes when parsing DWARF 5?

DWARF 5 allows DW_AT_virtuality to be present with the default value
DW_VIRTUALITY_none (0x00).

If a compiler explicitly emits this default value, will the presence check
evaluate to true and falsely identify the non-virtual base as virtual,
skipping it?

> +
> +	if (die_get_data_member_location(die_mem, &loc) < 0 || loc != 0)
> [ ... ]
> +Dwarf_Die *die_find_virtual_func(Dwarf_Die *class_die, int index,
> +				 Dwarf_Die *die_mem)
> +{
> +	Dwarf_Die cur_die = *class_die;
> +	Dwarf_Die base_die;
> +
> +	while (die_is_compound_type(&cur_die)) {

[Severity: Medium]
Can this loop hang indefinitely when processing malformed DWARF?

If a maliciously crafted or malformed DWARF file contains cyclic inheritance,
die_get_real_type() resolves the type back to the current class.

Since the loop does not maintain a depth counter or cycle detection, does it
spin infinitely as it re-evaluates the same class type?

> +		if (die_find_child(&cur_die, __die_find_virtual_func_cb,
> +				   (void *)(long)index, die_mem))
> +			return die_mem;
> [ ... ]
> +Dwarf_Die *die_get_vptr_class(Dwarf_Die *type_die, int offset,
> +			      Dwarf_Die *die_mem)
> +{
> +	Dwarf_Die class_die = *type_die;
> +	Dwarf_Die vptr_class = *type_die;
> +	Dwarf_Die member_die, mb_type;
> +	Dwarf_Word loc;
> +
> +	while (die_is_compound_type(&class_die)) {

[Severity: Medium]
Similarly, does this loop also lack a depth limit or cycle detection when
traversing base class subobjects dynamically, risking infinite cycles on
malformed DWARF?

> +		if (die_find_child(&class_die, __die_find_member_offset_cb,
> +				   (void *)(long)offset, &member_die) == NULL)
> +			return NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930210038.196928-1-yzhao62@ncsu.edu?part=2

Re: [PATCH v2 3/3] perf annotate: Resolve C++ virtual function calls in x86 insn tracking

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] perf: cross-platform analysis broken by hardcoded host pointer size
- [Medium] perf: vtable pointer load ignores multi-register index
--

commit a71181b28893b295ef8ae1558faf7b1c66e3593b
Author: Yanbo Zhao <yzhao62@ncsu.edu>

perf annotate: Resolve C++ virtual function calls in x86 insn tracking

This commit introduces tracking of C++ virtual function calls by resolving
vtable pointer loads and virtual function calls through the vtable in x86
instruction state tracking. It enhances update_insn_state_x86() to handle
TSR_KIND_VTABLE_PTR and TSR_KIND_VFUNC_PTR register states.

> diff --git a/tools/perf/util/annotate-arch/annotate-x86.c b/tools/perf/util/annotate-arch/annotate-x86.c
> index 1acf31a2c7597..d21192b74b6bf 100644
> --- a/tools/perf/util/annotate-arch/annotate-x86.c
> +++ b/tools/perf/util/annotate-arch/annotate-x86.c

[ ... ]

> @@ -216,6 +216,29 @@ static void invalidate_reg_state(struct type_state_reg *reg)
>  	reg->copied_from = -1;
>  }
>  
> +/*
> + * Get the return type of the C++ virtual function at the @offset in the
> + * vtable of @class_die.  Returns false if it's not found or the function
> + * returns void.
> + */
> +static bool vtable_get_rettype(Dwarf_Die *class_die, int offset,
> +			       Dwarf_Die *type_die)
> +{
> +	Dwarf_Die func_die;
> +	int index;
> +
> +	if (offset < 0)
> +		return false;
> +
> +	/* TODO: arch-dependent pointer size */
> +	index = offset / sizeof(void *);

[Severity: Medium]
Will dividing the target's vtable byte offset by the host's sizeof(void *)
compute an incorrect slot index when analyzing data from an architecture
with a different pointer size?

Relying on host execution details like sizeof(void *) breaks
cross-platform analysis (e.g., analyzing 32-bit x86 perf.data on a
64-bit host).

> +
> +	if (die_find_virtual_func(class_die, index, &func_die) == NULL)
> +		return false;
> +
> +	return die_get_real_type(&func_die, type_die) != NULL;
> +}
> +
>  static void update_insn_state_x86(struct type_state *state,

[ ... ]

> @@ -622,13 +669,38 @@ static void update_insn_state_x86(struct type_state *state,
>  			}
>  			pr_debug_type_name(&tsr->type, tsr->kind);
>  		}
> +		/* Load a function pointer from the vtable (for 'call *%reg') */
> +		else if (has_reg_type(state, sreg) && state->regs[sreg].ok &&
> +			 state->regs[sreg].kind == TSR_KIND_VTABLE_PTR &&
> +			 vtable_get_rettype(&state->regs[sreg].type, src->offset,
> +					    &type_die)) {

[Severity: Medium]
Does this query the wrong vtable slot by ignoring the index register when
src->multi_regs is true?

If the operand is an indexed memory reference using multiple registers
(e.g., mov 0x8(%rbx, %rcx, 8), %rax), src->offset contains only the
static displacement part, not the dynamic index. Should this verify
!src->multi_regs before querying the DWARF vtable with src->offset?

> +			tsr->type = type_die;
> +			tsr->kind = TSR_KIND_VFUNC_PTR;
> +			tsr->offset = 0;
> +			tsr->ok = true;
> +
> +			pr_debug_dtp("mov [%x] %#x(reg%d) -> reg%d",
> +				     insn_offset, src->offset, sreg, dst->reg1);
> +			pr_debug_type_name(&tsr->type, tsr->kind);
> +		}
>  		/* And then dereference the pointer if it has one */

[ ... ]

> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 8a9d3f2eec4dc..e9bf76cc28633 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c

[ ... ]

> @@ -715,8 +723,11 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
>  			    Dwarf_Die *type_die, int ptr_offset)
>  {
>  	Dwarf_Word size;
> +	bool is_pointer = (kind == TSR_KIND_POINTER ||
> +			   kind == TSR_KIND_VTABLE_PTR ||
> +			   kind == TSR_KIND_VFUNC_PTR);
>  
> -	if (kind == TSR_KIND_POINTER) {
> +	if (is_pointer) {
>  		/* TODO: arch-dependent pointer size */
>  		size = sizeof(void *);
>  	}

[Severity: Medium]
Like the vtable_get_rettype() issue above, will hardcoding the host's
pointer size here corrupt the stack state tracking for the target
architecture?

[ ... ]

> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 5478c134e7e3b..5d96ee4c866b6 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c

[ ... ]

> @@ -300,6 +300,11 @@ static int call__parse(const struct arch *arch, struct ins_operands *ops, struct
>  	if (tok != NULL) {
>  		endptr++;
>  
> +		/* Save the operand (without '*') to extract register and offset */
> +		ops->target.raw = strdup(tok + 1);

[Severity: Low]
This is a pre-existing issue, but this file calls strdup() without
explicitly including <string.h>.

Can we add the explicit <string.h> include? The perf subsystem guide states
that POSIX standard headers should be explicitly included to prevent musl
compilation failures from missing implicit inclusions.

> +		if (ops->target.raw == NULL)
> +			return -1;
> +
>  		/* Indirect call can use a non-rip register and offset: callq  *0x8(%rbx).
>  		 * Do not parse such instruction.  */
>  		if (strstr(endptr, "(%r") == NULL)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930210038.196928-1-yzhao62@ncsu.edu?part=3

Re: [PATCH v2 1/3] perf dwarf-aux: Add die_is_compound_type() to handle C++ class types

[Namhyung Kim]

On Wed, Sep 30, 2026 at 09:10:51PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] perf/dwarf-aux: stack overflow in __die_find_member_offset_cb via cyclic inheritance
> - [Medium] perf/dwarf-aux: flex array detection fails for C++ classes
> --
> 
> commit 3f3b9b56b2e28f1713af3b40ab8946d5bbc8d245
> Author: Yanbo Zhao <yzhao62@ncsu.edu>
> 
> perf dwarf-aux: Add die_is_compound_type() to handle C++ class types
> 
> This commit introduces die_is_compound_type() to support C++ classes alongside
> structs and unions. It also updates offset lookups to descend into base class
> subobjects using DW_TAG_inheritance entries.
> 
> > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> > index 54f8b5ec74a2f..eb4b8f3475df1 100644
> > --- a/tools/perf/util/dwarf-aux.c
> > +++ b/tools/perf/util/dwarf-aux.c
> > @@ -61,6 +61,14 @@ const char *cu_get_comp_dir(Dwarf_Die *cu_die)
> >  	return dwarf_formstring(&attr);
> >  }
> >  
> > +bool die_is_compound_type(Dwarf_Die *type_die)
> > +{
> > +	int tag = dwarf_tag(type_die);
> > +
> > +	return tag == DW_TAG_structure_type || tag == DW_TAG_union_type ||
> > +	       tag == DW_TAG_class_type;
> > +}
> 
> [Severity: Medium]
> Does die_has_flex_array_recurse() also need to be updated to use this new
> helper?
> 
> If we look at die_has_flex_array_recurse() in tools/perf/util/dwarf-aux.c,
> it currently forces flex array detection to return false for C++ classes
> because it explicitly ignores DW_TAG_class_type:
> 
>     if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type)
>         return false;
>     ...
>     if (tag == DW_TAG_structure_type && found) {
>         if (is_flex_array_member(&last_mb))
>             return true;
> 
> Could this cause accesses to flex array members inside C++ classes to be
> wrongly rejected as out-of-bounds?

IIUC C++ officially doesn't support flexible arrays and there are
better alternatives so I guess it's rare to see them in C++ classes.
Maybe it's ok to ignore them until we find it's really needed.

> 
> [ ... ]
> 
> > @@ -2164,6 +2180,30 @@ static int __die_find_member_offset_cb(Dwarf_Die *die_mem, void *arg)
> >  			loc = 0;
> >  	}
> >  
> > +	if (tag == DW_TAG_inheritance) {
> > +		Dwarf_Die base_die, member_die;
> > +
> > +		/*
> > +		 * A base class subobject can be smaller than the size of the
> > +		 * class type: an empty base takes no space (EBO) and a member
> > +		 * of the derived class can be placed in the tail padding of
> > +		 * the base.  In both cases a member of the derived class is
> > +		 * at the same offset as the base, so only match the base if
> > +		 * it actually has a member at the offset.
> > +		 */
> > +		if (offset < loc)
> > +			return DIE_FIND_CB_SIBLING;
> > +
> > +		if (die_get_real_type(die_mem, &base_die) == NULL)
> > +			return DIE_FIND_CB_SIBLING;
> > +
> > +		if (die_find_child(&base_die, __die_find_member_offset_cb,
> > +				   (void *)(long)(offset - loc), &member_die))
> > +			return DIE_FIND_CB_END;
> 
> [Severity: High]
> Does this recursive call into the DWARF child tree lack a depth limit?
> 
> Unlike __add_member_cb(), there is no recursion depth limit tracked or
> enforced here. If a maliciously crafted or corrupted DWARF file contains
> cyclic inheritance (e.g., A inherits from B, B inherits from A), could
> this trigger unbounded mutual recursion between die_find_child() and
> __die_find_member_offset_cb(), rapidly exhausting the stack?

It looks like a corner case which requires some refactoring.  I feel
like we can add it in a separate patch.

Thanks,
Namhyung

Re: [PATCH v2 1/3] perf dwarf-aux: Add die_is_compound_type() to handle C++ class types

[Arnaldo Carvalho de Melo]

On Thu, Oct 01, 2026 at 11:14:21AM -0700, Namhyung Kim wrote:
> On Wed, Sep 30, 2026 at 09:10:51PM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > - [High] perf/dwarf-aux: stack overflow in __die_find_member_offset_cb via cyclic inheritance
> > - [Medium] perf/dwarf-aux: flex array detection fails for C++ classes
> > --
> > 
> > commit 3f3b9b56b2e28f1713af3b40ab8946d5bbc8d245
> > Author: Yanbo Zhao <yzhao62@ncsu.edu>
> > 
> > perf dwarf-aux: Add die_is_compound_type() to handle C++ class types
> > 
> > This commit introduces die_is_compound_type() to support C++ classes alongside
> > structs and unions. It also updates offset lookups to descend into base class
> > subobjects using DW_TAG_inheritance entries.
> > 
> > > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> > > index 54f8b5ec74a2f..eb4b8f3475df1 100644
> > > --- a/tools/perf/util/dwarf-aux.c
> > > +++ b/tools/perf/util/dwarf-aux.c
> > > @@ -61,6 +61,14 @@ const char *cu_get_comp_dir(Dwarf_Die *cu_die)
> > >  	return dwarf_formstring(&attr);
> > >  }
> > >  
> > > +bool die_is_compound_type(Dwarf_Die *type_die)
> > > +{
> > > +	int tag = dwarf_tag(type_die);
> > > +
> > > +	return tag == DW_TAG_structure_type || tag == DW_TAG_union_type ||
> > > +	       tag == DW_TAG_class_type;
> > > +}
> > 
> > [Severity: Medium]
> > Does die_has_flex_array_recurse() also need to be updated to use this new
> > helper?
> > 
> > If we look at die_has_flex_array_recurse() in tools/perf/util/dwarf-aux.c,
> > it currently forces flex array detection to return false for C++ classes
> > because it explicitly ignores DW_TAG_class_type:
> > 
> >     if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type)
> >         return false;
> >     ...
> >     if (tag == DW_TAG_structure_type && found) {
> >         if (is_flex_array_member(&last_mb))
> >             return true;
> > 
> > Could this cause accesses to flex array members inside C++ classes to be
> > wrongly rejected as out-of-bounds?
 
> IIUC C++ officially doesn't support flexible arrays and there are
> better alternatives so I guess it's rare to see them in C++ classes.
> Maybe it's ok to ignore them until we find it's really needed.

ok on both counts, there is another patch with a Sashiko report, can you
guys reply to that one as well?

Thanks,

- Arnaldo

Re: [PATCH v2 2/3] perf dwarf-aux: Add C++ vtable helpers

[Namhyung Kim]

On Wed, Sep 30, 2026 at 09:10:56PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] perf dwarf-aux: DW_AT_virtuality presence check incorrectly skips non-virtual bases
> - [Medium] perf dwarf-aux: infinite loop in C++ inheritance traversal on malformed DWARF
> --
> 
> commit e1f1b87424c8d58d923a8c3fbc1408fa0ae0c3b9
> Author: Yanbo Zhao <yzhao62@ncsu.edu>
> 
> perf dwarf-aux: Add C++ vtable helpers
>     
> Add DWARF helper functions needed to resolve C++ virtual function calls
> statically in the data type profiling.
> 
> > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> > index eb4b8f3475df1..b1f3dca057abb 100644
> > --- a/tools/perf/util/dwarf-aux.c
> > +++ b/tools/perf/util/dwarf-aux.c
> > @@ -2293,6 +2293,222 @@ Dwarf_Die *die_get_member_type(Dwarf_Die *type_die, int offset,
> > [ ... ]
> > +static int __die_find_primary_base_cb(Dwarf_Die *die_mem, void *arg __maybe_unused)
> > +{
> > +	Dwarf_Attribute attr;
> > +	Dwarf_Die base_die, vptr_die;
> > +	Dwarf_Word loc;
> > +
> > +	if (dwarf_tag(die_mem) != DW_TAG_inheritance)
> > +		return DIE_FIND_CB_SIBLING;
> > +
> > +	if (dwarf_attr_integrate(die_mem, DW_AT_virtuality, &attr))
> > +		return DIE_FIND_CB_SIBLING;
> 
> [Severity: Medium]
> Does this code incorrectly skip valid primary base classes when parsing DWARF 5?
> 
> DWARF 5 allows DW_AT_virtuality to be present with the default value
> DW_VIRTUALITY_none (0x00).
> 
> If a compiler explicitly emits this default value, will the presence check
> evaluate to true and falsely identify the non-virtual base as virtual,
> skipping it?

Right, it'd be better to check the value of the attribute as well.

> 
> > +
> > +	if (die_get_data_member_location(die_mem, &loc) < 0 || loc != 0)
> > [ ... ]
> > +Dwarf_Die *die_find_virtual_func(Dwarf_Die *class_die, int index,
> > +				 Dwarf_Die *die_mem)
> > +{
> > +	Dwarf_Die cur_die = *class_die;
> > +	Dwarf_Die base_die;
> > +
> > +	while (die_is_compound_type(&cur_die)) {
> 
> [Severity: Medium]
> Can this loop hang indefinitely when processing malformed DWARF?
> 
> If a maliciously crafted or malformed DWARF file contains cyclic inheritance,
> die_get_real_type() resolves the type back to the current class.
> 
> Since the loop does not maintain a depth counter or cycle detection, does it
> spin infinitely as it re-evaluates the same class type?

It seems sashiko is picky about possible recursions on invalid dwarf.
As we recently added recursion depth limits, can you follow the
convention and bail out if it detects one?

> 
> > +		if (die_find_child(&cur_die, __die_find_virtual_func_cb,
> > +				   (void *)(long)index, die_mem))
> > +			return die_mem;
> > [ ... ]
> > +Dwarf_Die *die_get_vptr_class(Dwarf_Die *type_die, int offset,
> > +			      Dwarf_Die *die_mem)
> > +{
> > +	Dwarf_Die class_die = *type_die;
> > +	Dwarf_Die vptr_class = *type_die;
> > +	Dwarf_Die member_die, mb_type;
> > +	Dwarf_Word loc;
> > +
> > +	while (die_is_compound_type(&class_die)) {
> 
> [Severity: Medium]
> Similarly, does this loop also lack a depth limit or cycle detection when
> traversing base class subobjects dynamically, risking infinite cycles on
> malformed DWARF?

Ditto.

Thanks,
Namhyung

> 
> > +		if (die_find_child(&class_die, __die_find_member_offset_cb,
> > +				   (void *)(long)offset, &member_die) == NULL)
> > +			return NULL;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260930210038.196928-1-yzhao62@ncsu.edu?part=2

Re: [PATCH v2 3/3] perf annotate: Resolve C++ virtual function calls in x86 insn tracking

[Namhyung Kim]

On Wed, Sep 30, 2026 at 09:11:12PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] perf: cross-platform analysis broken by hardcoded host pointer size
> - [Medium] perf: vtable pointer load ignores multi-register index
> --
> 
> commit a71181b28893b295ef8ae1558faf7b1c66e3593b
> Author: Yanbo Zhao <yzhao62@ncsu.edu>
> 
> perf annotate: Resolve C++ virtual function calls in x86 insn tracking
> 
> This commit introduces tracking of C++ virtual function calls by resolving
> vtable pointer loads and virtual function calls through the vtable in x86
> instruction state tracking. It enhances update_insn_state_x86() to handle
> TSR_KIND_VTABLE_PTR and TSR_KIND_VFUNC_PTR register states.
> 
> > diff --git a/tools/perf/util/annotate-arch/annotate-x86.c b/tools/perf/util/annotate-arch/annotate-x86.c
> > index 1acf31a2c7597..d21192b74b6bf 100644
> > --- a/tools/perf/util/annotate-arch/annotate-x86.c
> > +++ b/tools/perf/util/annotate-arch/annotate-x86.c
> 
> [ ... ]
> 
> > @@ -216,6 +216,29 @@ static void invalidate_reg_state(struct type_state_reg *reg)
> >  	reg->copied_from = -1;
> >  }
> >  
> > +/*
> > + * Get the return type of the C++ virtual function at the @offset in the
> > + * vtable of @class_die.  Returns false if it's not found or the function
> > + * returns void.
> > + */
> > +static bool vtable_get_rettype(Dwarf_Die *class_die, int offset,
> > +			       Dwarf_Die *type_die)
> > +{
> > +	Dwarf_Die func_die;
> > +	int index;
> > +
> > +	if (offset < 0)
> > +		return false;
> > +
> > +	/* TODO: arch-dependent pointer size */
> > +	index = offset / sizeof(void *);
> 
> [Severity: Medium]
> Will dividing the target's vtable byte offset by the host's sizeof(void *)
> compute an incorrect slot index when analyzing data from an architecture
> with a different pointer size?
> 
> Relying on host execution details like sizeof(void *) breaks
> cross-platform analysis (e.g., analyzing 32-bit x86 perf.data on a
> 64-bit host).

I think this is fine for now as we also have other places assuming the
host pointer size.  We'll fix it later.

> 
> > +
> > +	if (die_find_virtual_func(class_die, index, &func_die) == NULL)
> > +		return false;
> > +
> > +	return die_get_real_type(&func_die, type_die) != NULL;
> > +}
> > +
> >  static void update_insn_state_x86(struct type_state *state,
> 
> [ ... ]
> 
> > @@ -622,13 +669,38 @@ static void update_insn_state_x86(struct type_state *state,
> >  			}
> >  			pr_debug_type_name(&tsr->type, tsr->kind);
> >  		}
> > +		/* Load a function pointer from the vtable (for 'call *%reg') */
> > +		else if (has_reg_type(state, sreg) && state->regs[sreg].ok &&
> > +			 state->regs[sreg].kind == TSR_KIND_VTABLE_PTR &&
> > +			 vtable_get_rettype(&state->regs[sreg].type, src->offset,
> > +					    &type_die)) {
> 
> [Severity: Medium]
> Does this query the wrong vtable slot by ignoring the index register when
> src->multi_regs is true?
> 
> If the operand is an indexed memory reference using multiple registers
> (e.g., mov 0x8(%rbx, %rcx, 8), %rax), src->offset contains only the
> static displacement part, not the dynamic index. Should this verify
> !src->multi_regs before querying the DWARF vtable with src->offset?

Multiple register is not handled well in the current code.  Dealing with
dynamic index would require more careful instruction tracking.

Anyway, it'd be safer to check !src->multi_regs here.

Thanks,
Namhyung

> 
> > +			tsr->type = type_die;
> > +			tsr->kind = TSR_KIND_VFUNC_PTR;
> > +			tsr->offset = 0;
> > +			tsr->ok = true;
> > +
> > +			pr_debug_dtp("mov [%x] %#x(reg%d) -> reg%d",
> > +				     insn_offset, src->offset, sreg, dst->reg1);
> > +			pr_debug_type_name(&tsr->type, tsr->kind);
> > +		}
> >  		/* And then dereference the pointer if it has one */
> 
> [ ... ]
> 
> > diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> > index 8a9d3f2eec4dc..e9bf76cc28633 100644
> > --- a/tools/perf/util/annotate-data.c
> > +++ b/tools/perf/util/annotate-data.c
> 
> [ ... ]
> 
> > @@ -715,8 +723,11 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
> >  			    Dwarf_Die *type_die, int ptr_offset)
> >  {
> >  	Dwarf_Word size;
> > +	bool is_pointer = (kind == TSR_KIND_POINTER ||
> > +			   kind == TSR_KIND_VTABLE_PTR ||
> > +			   kind == TSR_KIND_VFUNC_PTR);
> >  
> > -	if (kind == TSR_KIND_POINTER) {
> > +	if (is_pointer) {
> >  		/* TODO: arch-dependent pointer size */
> >  		size = sizeof(void *);
> >  	}
> 
> [Severity: Medium]
> Like the vtable_get_rettype() issue above, will hardcoding the host's
> pointer size here corrupt the stack state tracking for the target
> architecture?
> 
> [ ... ]
> 
> > diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> > index 5478c134e7e3b..5d96ee4c866b6 100644
> > --- a/tools/perf/util/disasm.c
> > +++ b/tools/perf/util/disasm.c
> 
> [ ... ]
> 
> > @@ -300,6 +300,11 @@ static int call__parse(const struct arch *arch, struct ins_operands *ops, struct
> >  	if (tok != NULL) {
> >  		endptr++;
> >  
> > +		/* Save the operand (without '*') to extract register and offset */
> > +		ops->target.raw = strdup(tok + 1);
> 
> [Severity: Low]
> This is a pre-existing issue, but this file calls strdup() without
> explicitly including <string.h>.
> 
> Can we add the explicit <string.h> include? The perf subsystem guide states
> that POSIX standard headers should be explicitly included to prevent musl
> compilation failures from missing implicit inclusions.
> 
> > +		if (ops->target.raw == NULL)
> > +			return -1;
> > +
> >  		/* Indirect call can use a non-rip register and offset: callq  *0x8(%rbx).
> >  		 * Do not parse such instruction.  */
> >  		if (strstr(endptr, "(%r") == NULL)
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260930210038.196928-1-yzhao62@ncsu.edu?part=3

Re: [PATCH v2 1/3] perf dwarf-aux: Add die_is_compound_type() to handle C++ class types

[Yanbo Zhao]

Hello Arnaldo and Namhyung,

On Fri, Oct 02, 2026 at 06:34:33PM +0200, Arnaldo Carvalho de Melo wrote:
> On Thu, Oct 01, 2026 at 11:14:21AM -0700, Namhyung Kim wrote:
> > IIUC C++ officially doesn't support flexible arrays and there are
> > better alternatives so I guess it's rare to see them in C++ classes.
> > Maybe it's ok to ignore them until we find it's really needed.
>
> ok on both counts, there is another patch with a Sashiko report, can you
> guys reply to that one as well?

Thanks for the review.  I agree with both, so I'll leave this patch as
is.

For the recursion in __die_find_member_offset_cb(), the callback needs
to carry the depth in its argument.  I'll send it as a separate patch
on top of this series.

I will reply to the reports for the patch 2 and 3 as well and send v3
with the changes in those two patches, and there will be no change in
this one.

Thanks,
Yanbo

On Fri, Oct 2, 2026 at 12:34 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Thu, Oct 01, 2026 at 11:14:21AM -0700, Namhyung Kim wrote:
> > On Wed, Sep 30, 2026 at 09:10:51PM +0000, sashiko-bot@kernel.org wrote:
> > > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > > - [High] perf/dwarf-aux: stack overflow in __die_find_member_offset_cb via cyclic inheritance
> > > - [Medium] perf/dwarf-aux: flex array detection fails for C++ classes
> > > --
> > >
> > > commit 3f3b9b56b2e28f1713af3b40ab8946d5bbc8d245
> > > Author: Yanbo Zhao <yzhao62@ncsu.edu>
> > >
> > > perf dwarf-aux: Add die_is_compound_type() to handle C++ class types
> > >
> > > This commit introduces die_is_compound_type() to support C++ classes alongside
> > > structs and unions. It also updates offset lookups to descend into base class
> > > subobjects using DW_TAG_inheritance entries.
> > >
> > > > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> > > > index 54f8b5ec74a2f..eb4b8f3475df1 100644
> > > > --- a/tools/perf/util/dwarf-aux.c
> > > > +++ b/tools/perf/util/dwarf-aux.c
> > > > @@ -61,6 +61,14 @@ const char *cu_get_comp_dir(Dwarf_Die *cu_die)
> > > >   return dwarf_formstring(&attr);
> > > >  }
> > > >
> > > > +bool die_is_compound_type(Dwarf_Die *type_die)
> > > > +{
> > > > + int tag = dwarf_tag(type_die);
> > > > +
> > > > + return tag == DW_TAG_structure_type || tag == DW_TAG_union_type ||
> > > > +        tag == DW_TAG_class_type;
> > > > +}
> > >
> > > [Severity: Medium]
> > > Does die_has_flex_array_recurse() also need to be updated to use this new
> > > helper?
> > >
> > > If we look at die_has_flex_array_recurse() in tools/perf/util/dwarf-aux.c,
> > > it currently forces flex array detection to return false for C++ classes
> > > because it explicitly ignores DW_TAG_class_type:
> > >
> > >     if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type)
> > >         return false;
> > >     ...
> > >     if (tag == DW_TAG_structure_type && found) {
> > >         if (is_flex_array_member(&last_mb))
> > >             return true;
> > >
> > > Could this cause accesses to flex array members inside C++ classes to be
> > > wrongly rejected as out-of-bounds?
>
> > IIUC C++ officially doesn't support flexible arrays and there are
> > better alternatives so I guess it's rare to see them in C++ classes.
> > Maybe it's ok to ignore them until we find it's really needed.
>
> ok on both counts, there is another patch with a Sashiko report, can you
> guys reply to that one as well?
>
> Thanks,
>
> - Arnaldo

Re: [PATCH v2 2/3] perf dwarf-aux: Add C++ vtable helpers

[Yanbo Zhao]

Hello Namhyung,

On Fri, Oct 02, 2026 at 03:52:43PM -0700, Namhyung Kim wrote:
> On Wed, Sep 30, 2026 at 09:10:56PM +0000, sashiko-bot@kernel.org wrote:
> > [Severity: Medium]
> > Does this code incorrectly skip valid primary base classes when parsing DWARF 5?
> >
> > DWARF 5 allows DW_AT_virtuality to be present with the default value
> > DW_VIRTUALITY_none (0x00).
>
> Right, it'd be better to check the value of the attribute as well.

Will do.  I'll skip the base class only if the value is not
DW_VIRTUALITY_none.

> > [Severity: Medium]
> > Can this loop hang indefinitely when processing malformed DWARF?
>
> It seems sashiko is picky about possible recursions on invalid dwarf.
> As we recently added recursion depth limits, can you follow the
> convention and bail out if it detects one?

Sure.  I'll use MAX_TYPE_CHASE to bound the loops in
die_find_virtual_func() and die_get_vptr_class(), and bail out with a
debug message like the other places in v3.

Thanks,
Yanbo

On Fri, Oct 2, 2026 at 6:52 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Wed, Sep 30, 2026 at 09:10:56PM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > - [Medium] perf dwarf-aux: DW_AT_virtuality presence check incorrectly skips non-virtual bases
> > - [Medium] perf dwarf-aux: infinite loop in C++ inheritance traversal on malformed DWARF
> > --
> >
> > commit e1f1b87424c8d58d923a8c3fbc1408fa0ae0c3b9
> > Author: Yanbo Zhao <yzhao62@ncsu.edu>
> >
> > perf dwarf-aux: Add C++ vtable helpers
> >
> > Add DWARF helper functions needed to resolve C++ virtual function calls
> > statically in the data type profiling.
> >
> > > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> > > index eb4b8f3475df1..b1f3dca057abb 100644
> > > --- a/tools/perf/util/dwarf-aux.c
> > > +++ b/tools/perf/util/dwarf-aux.c
> > > @@ -2293,6 +2293,222 @@ Dwarf_Die *die_get_member_type(Dwarf_Die *type_die, int offset,
> > > [ ... ]
> > > +static int __die_find_primary_base_cb(Dwarf_Die *die_mem, void *arg __maybe_unused)
> > > +{
> > > +   Dwarf_Attribute attr;
> > > +   Dwarf_Die base_die, vptr_die;
> > > +   Dwarf_Word loc;
> > > +
> > > +   if (dwarf_tag(die_mem) != DW_TAG_inheritance)
> > > +           return DIE_FIND_CB_SIBLING;
> > > +
> > > +   if (dwarf_attr_integrate(die_mem, DW_AT_virtuality, &attr))
> > > +           return DIE_FIND_CB_SIBLING;
> >
> > [Severity: Medium]
> > Does this code incorrectly skip valid primary base classes when parsing DWARF 5?
> >
> > DWARF 5 allows DW_AT_virtuality to be present with the default value
> > DW_VIRTUALITY_none (0x00).
> >
> > If a compiler explicitly emits this default value, will the presence check
> > evaluate to true and falsely identify the non-virtual base as virtual,
> > skipping it?
>
> Right, it'd be better to check the value of the attribute as well.
>
> >
> > > +
> > > +   if (die_get_data_member_location(die_mem, &loc) < 0 || loc != 0)
> > > [ ... ]
> > > +Dwarf_Die *die_find_virtual_func(Dwarf_Die *class_die, int index,
> > > +                            Dwarf_Die *die_mem)
> > > +{
> > > +   Dwarf_Die cur_die = *class_die;
> > > +   Dwarf_Die base_die;
> > > +
> > > +   while (die_is_compound_type(&cur_die)) {
> >
> > [Severity: Medium]
> > Can this loop hang indefinitely when processing malformed DWARF?
> >
> > If a maliciously crafted or malformed DWARF file contains cyclic inheritance,
> > die_get_real_type() resolves the type back to the current class.
> >
> > Since the loop does not maintain a depth counter or cycle detection, does it
> > spin infinitely as it re-evaluates the same class type?
>
> It seems sashiko is picky about possible recursions on invalid dwarf.
> As we recently added recursion depth limits, can you follow the
> convention and bail out if it detects one?
>
> >
> > > +           if (die_find_child(&cur_die, __die_find_virtual_func_cb,
> > > +                              (void *)(long)index, die_mem))
> > > +                   return die_mem;
> > > [ ... ]
> > > +Dwarf_Die *die_get_vptr_class(Dwarf_Die *type_die, int offset,
> > > +                         Dwarf_Die *die_mem)
> > > +{
> > > +   Dwarf_Die class_die = *type_die;
> > > +   Dwarf_Die vptr_class = *type_die;
> > > +   Dwarf_Die member_die, mb_type;
> > > +   Dwarf_Word loc;
> > > +
> > > +   while (die_is_compound_type(&class_die)) {
> >
> > [Severity: Medium]
> > Similarly, does this loop also lack a depth limit or cycle detection when
> > traversing base class subobjects dynamically, risking infinite cycles on
> > malformed DWARF?
>
> Ditto.
>
> Thanks,
> Namhyung
>
> >
> > > +           if (die_find_child(&class_die, __die_find_member_offset_cb,
> > > +                              (void *)(long)offset, &member_die) == NULL)
> > > +                   return NULL;
> >
> > --
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260930210038.196928-1-yzhao62@ncsu.edu?part=2

Re: [PATCH v2 3/3] perf annotate: Resolve C++ virtual function calls in x86 insn tracking

[Yanbo Zhao]

Hello Namhyung,

On Fri, Oct 02, 2026 at 03:56:46PM -0700, Namhyung Kim wrote:
> On Wed, Sep 30, 2026 at 09:11:12PM +0000, sashiko-bot@kernel.org wrote:
> > [Severity: Medium]
> > Will dividing the target's vtable byte offset by the host's sizeof(void *)
> > compute an incorrect slot index when analyzing data from an architecture
> > with a different pointer size?
>
> I think this is fine for now as we also have other places assuming the
> host pointer size.  We'll fix it later.

Ok, I'll keep it with the TODO comment.

> > [Severity: Medium]
> > Does this query the wrong vtable slot by ignoring the index register when
> > src->multi_regs is true?
>
> Multiple register is not handled well in the current code.  Dealing with
> dynamic index would require more careful instruction tracking.
>
> Anyway, it'd be safer to check !src->multi_regs here.

Will add.  I'll also add the same check where it marks a register as
the vtable pointer, since the offset doesn't tell the member with an
index register there either.

I'll send v3 with these changes.

Thanks,
Yanbo

On Fri, Oct 2, 2026 at 6:56 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Wed, Sep 30, 2026 at 09:11:12PM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > - [Medium] perf: cross-platform analysis broken by hardcoded host pointer size
> > - [Medium] perf: vtable pointer load ignores multi-register index
> > --
> >
> > commit a71181b28893b295ef8ae1558faf7b1c66e3593b
> > Author: Yanbo Zhao <yzhao62@ncsu.edu>
> >
> > perf annotate: Resolve C++ virtual function calls in x86 insn tracking
> >
> > This commit introduces tracking of C++ virtual function calls by resolving
> > vtable pointer loads and virtual function calls through the vtable in x86
> > instruction state tracking. It enhances update_insn_state_x86() to handle
> > TSR_KIND_VTABLE_PTR and TSR_KIND_VFUNC_PTR register states.
> >
> > > diff --git a/tools/perf/util/annotate-arch/annotate-x86.c b/tools/perf/util/annotate-arch/annotate-x86.c
> > > index 1acf31a2c7597..d21192b74b6bf 100644
> > > --- a/tools/perf/util/annotate-arch/annotate-x86.c
> > > +++ b/tools/perf/util/annotate-arch/annotate-x86.c
> >
> > [ ... ]
> >
> > > @@ -216,6 +216,29 @@ static void invalidate_reg_state(struct type_state_reg *reg)
> > >     reg->copied_from = -1;
> > >  }
> > >
> > > +/*
> > > + * Get the return type of the C++ virtual function at the @offset in the
> > > + * vtable of @class_die.  Returns false if it's not found or the function
> > > + * returns void.
> > > + */
> > > +static bool vtable_get_rettype(Dwarf_Die *class_die, int offset,
> > > +                          Dwarf_Die *type_die)
> > > +{
> > > +   Dwarf_Die func_die;
> > > +   int index;
> > > +
> > > +   if (offset < 0)
> > > +           return false;
> > > +
> > > +   /* TODO: arch-dependent pointer size */
> > > +   index = offset / sizeof(void *);
> >
> > [Severity: Medium]
> > Will dividing the target's vtable byte offset by the host's sizeof(void *)
> > compute an incorrect slot index when analyzing data from an architecture
> > with a different pointer size?
> >
> > Relying on host execution details like sizeof(void *) breaks
> > cross-platform analysis (e.g., analyzing 32-bit x86 perf.data on a
> > 64-bit host).
>
> I think this is fine for now as we also have other places assuming the
> host pointer size.  We'll fix it later.
>
> >
> > > +
> > > +   if (die_find_virtual_func(class_die, index, &func_die) == NULL)
> > > +           return false;
> > > +
> > > +   return die_get_real_type(&func_die, type_die) != NULL;
> > > +}
> > > +
> > >  static void update_insn_state_x86(struct type_state *state,
> >
> > [ ... ]
> >
> > > @@ -622,13 +669,38 @@ static void update_insn_state_x86(struct type_state *state,
> > >                     }
> > >                     pr_debug_type_name(&tsr->type, tsr->kind);
> > >             }
> > > +           /* Load a function pointer from the vtable (for 'call *%reg') */
> > > +           else if (has_reg_type(state, sreg) && state->regs[sreg].ok &&
> > > +                    state->regs[sreg].kind == TSR_KIND_VTABLE_PTR &&
> > > +                    vtable_get_rettype(&state->regs[sreg].type, src->offset,
> > > +                                       &type_die)) {
> >
> > [Severity: Medium]
> > Does this query the wrong vtable slot by ignoring the index register when
> > src->multi_regs is true?
> >
> > If the operand is an indexed memory reference using multiple registers
> > (e.g., mov 0x8(%rbx, %rcx, 8), %rax), src->offset contains only the
> > static displacement part, not the dynamic index. Should this verify
> > !src->multi_regs before querying the DWARF vtable with src->offset?
>
> Multiple register is not handled well in the current code.  Dealing with
> dynamic index would require more careful instruction tracking.
>
> Anyway, it'd be safer to check !src->multi_regs here.
>
> Thanks,
> Namhyung
>
> >
> > > +                   tsr->type = type_die;
> > > +                   tsr->kind = TSR_KIND_VFUNC_PTR;
> > > +                   tsr->offset = 0;
> > > +                   tsr->ok = true;
> > > +
> > > +                   pr_debug_dtp("mov [%x] %#x(reg%d) -> reg%d",
> > > +                                insn_offset, src->offset, sreg, dst->reg1);
> > > +                   pr_debug_type_name(&tsr->type, tsr->kind);
> > > +           }
> > >             /* And then dereference the pointer if it has one */
> >
> > [ ... ]
> >
> > > diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> > > index 8a9d3f2eec4dc..e9bf76cc28633 100644
> > > --- a/tools/perf/util/annotate-data.c
> > > +++ b/tools/perf/util/annotate-data.c
> >
> > [ ... ]
> >
> > > @@ -715,8 +723,11 @@ void set_stack_state(struct type_state_stack *stack, int offset, u8 kind,
> > >                         Dwarf_Die *type_die, int ptr_offset)
> > >  {
> > >     Dwarf_Word size;
> > > +   bool is_pointer = (kind == TSR_KIND_POINTER ||
> > > +                      kind == TSR_KIND_VTABLE_PTR ||
> > > +                      kind == TSR_KIND_VFUNC_PTR);
> > >
> > > -   if (kind == TSR_KIND_POINTER) {
> > > +   if (is_pointer) {
> > >             /* TODO: arch-dependent pointer size */
> > >             size = sizeof(void *);
> > >     }
> >
> > [Severity: Medium]
> > Like the vtable_get_rettype() issue above, will hardcoding the host's
> > pointer size here corrupt the stack state tracking for the target
> > architecture?
> >
> > [ ... ]
> >
> > > diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> > > index 5478c134e7e3b..5d96ee4c866b6 100644
> > > --- a/tools/perf/util/disasm.c
> > > +++ b/tools/perf/util/disasm.c
> >
> > [ ... ]
> >
> > > @@ -300,6 +300,11 @@ static int call__parse(const struct arch *arch, struct ins_operands *ops, struct
> > >     if (tok != NULL) {
> > >             endptr++;
> > >
> > > +           /* Save the operand (without '*') to extract register and offset */
> > > +           ops->target.raw = strdup(tok + 1);
> >
> > [Severity: Low]
> > This is a pre-existing issue, but this file calls strdup() without
> > explicitly including <string.h>.
> >
> > Can we add the explicit <string.h> include? The perf subsystem guide states
> > that POSIX standard headers should be explicitly included to prevent musl
> > compilation failures from missing implicit inclusions.
> >
> > > +           if (ops->target.raw == NULL)
> > > +                   return -1;
> > > +
> > >             /* Indirect call can use a non-rip register and offset: callq  *0x8(%rbx).
> > >              * Do not parse such instruction.  */
> > >             if (strstr(endptr, "(%r") == NULL)
> >
> > --
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260930210038.196928-1-yzhao62@ncsu.edu?part=3