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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf dwarf-aux: unbounded recursion in __die_find_member_offset_cb()
--

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

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

This commit resolves C++ virtual function calls during x86 instruction
state tracking by tracking vtable pointer loads and virtual function
pointer calls using DWARF vtable helpers.

> diff --git a/tools/perf/util/annotate-arch/annotate-x86.c b/tools/perf/util/annotate-arch/annotate-x86.c
> index 3a6ea7e978e0a..e38a4e7a54d55 100644
> --- a/tools/perf/util/annotate-arch/annotate-x86.c
> +++ b/tools/perf/util/annotate-arch/annotate-x86.c
> @@ -207,6 +207,29 @@ static int x86__cpuid_parse(struct arch *arch, const char *cpuid)
>  }
>  
>  #ifdef HAVE_LIBDW_SUPPORT
> +/*
> + * 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;
> +}

[ ... ]

> @@ -615,13 +662,39 @@ static void update_insn_state_x86(struct type_state *state,

[ ... ]

>  		/* 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 (!src->multi_regs && die_is_vtbl_ptr_type(&type_die) &&
> +			    die_deref_vptr_class(&state->regs[sreg].type,
> +						 src->offset + state->regs[sreg].offset,
> +						 &type_die))

[Severity: Medium]
Can malformed DWARF debug info with cyclic DW_TAG_inheritance entries trigger
unbounded recursion and overflow the stack when resolving C++ virtual calls
here?

In tools/perf/util/annotate-arch/annotate-x86.c, update_insn_state_x86() and
vtable_get_rettype() wire in the first calls to die_deref_vptr_class() and
die_find_virtual_func(), both of which reach die_get_vptr_class() in
tools/perf/util/dwarf-aux.c:

  update_insn_state_x86()
    -> die_deref_vptr_class()
      -> die_get_vptr_class()

  vtable_get_rettype()
    -> die_find_virtual_func()
      -> __die_find_primary_base_cb()
        -> die_get_vptr_class()

Even if die_deref_ptr_type() succeeds first because a class's _vptr
DW_TAG_member precedes an offset-0 DW_TAG_inheritance entry,
vtable_get_rettype() -> die_find_virtual_func() subsequently walks all
offset-0 DW_TAG_inheritance children via __die_find_primary_base_cb() and
calls die_get_vptr_class(&base_die, 0, &vptr_die).

Although die_find_virtual_func() and die_get_vptr_class() check
++chase > MAX_TYPE_CHASE in their outer loops to guard against cyclic
inheritance, the first iteration of die_get_vptr_class() invokes
__die_find_member_offset_cb():

  tools/perf/util/dwarf-aux.c:die_get_vptr_class() {
      ...
		if (++chase > MAX_TYPE_CHASE) {
			pr_debug("DWARF: base class chase limit reached at DIE 0x%lx\n",
				 (unsigned long)dwarf_dieoffset(type_die));
			return NULL;
		}

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

Inside __die_find_member_offset_cb(), any DW_TAG_inheritance child with
offset >= loc recursively invokes die_find_child() with
__die_find_member_offset_cb() again without a recursion depth bound:

  tools/perf/util/dwarf-aux.c:__die_find_member_offset_cb() {
      ...
	if (tag == DW_TAG_inheritance) {
		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;
      ...
  }

Does this recursion in __die_find_member_offset_cb() bypass the
MAX_TYPE_CHASE check in die_get_vptr_class() on cyclic DW_TAG_inheritance
graphs?

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