All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Yanbo Zhao <yzhao62@ncsu.edu>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	James Clark <james.clark@linaro.org>, Zecheng Li <zli94@ncsu.edu>,
	Xu Liu <xliuprof@google.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] perf dwarf-aux: Add die_is_compound_type() to handle C++ class types
Date: Mon, 24 Aug 2026 13:35:41 -0700	[thread overview]
Message-ID: <aoyrHe2MBOf4ocqn@google.com> (raw)
In-Reply-To: <20260821050207.4517-2-yzhao62@ncsu.edu>

Hello,

On Fri, Aug 21, 2026 at 01:02:05AM -0400, Yanbo Zhao wrote:
> 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 all the existing open-coded struct/union tag checks to use it:
> - die_get_member_type() in dwarf-aux.c.
> - __add_member_cb(), is_compound_type(), and set_stack_state() in
>   annotate-data.c.

I think this part is fine.

> 
> 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.
> 
> 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.

But this part is a little unclear.  Can you please share an example
DWARF of a class that has inherited members?  It'd be helpful for others
to understand why it needs to handle the tag in the same way.

Thanks,
Namhyung

> 
> Signed-off-by: Yanbo Zhao <yzhao62@ncsu.edu>
> ---
>  tools/perf/util/annotate-data.c | 29 ++++++-----------------------
>  tools/perf/util/dwarf-aux.c     | 17 ++++++++++++-----
>  tools/perf/util/dwarf-aux.h     |  3 +++
>  3 files changed, 21 insertions(+), 28 deletions(-)
> 
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 4e4c58764082..ee6bd2d0012d 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -230,9 +230,9 @@ 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 (dwarf_tag(die) != DW_TAG_member)
> +	if (tag != DW_TAG_member && tag != DW_TAG_inheritance)
>  		return DIE_FIND_CB_SIBLING;
>  
>  	member = zalloc(sizeof(*member));
> @@ -292,15 +292,8 @@ static int __add_member_cb(Dwarf_Die *die, void *arg)
>  	INIT_LIST_HEAD(&member->children);
>  	list_add_tail(&member->node, &parent->children);
>  
> -	tag = dwarf_tag(&die_mem);
> -	switch (tag) {
> -	case DW_TAG_structure_type:
> -	case DW_TAG_union_type:
> +	if (die_is_compound_type(&die_mem))
>  		die_find_child(&die_mem, __add_member_cb, member, &die_mem);
> -		break;
> -	default:
> -		break;
> -	}
>  	return DIE_FIND_CB_SIBLING;
>  }
>  
> @@ -464,9 +457,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 */
> @@ -584,7 +575,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) {
> @@ -605,17 +595,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 d7160f87ac7d..88ba0ec23a68 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -60,6 +60,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)
>  {
> @@ -2053,7 +2061,7 @@ 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 */
> @@ -2104,7 +2112,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)
> @@ -2119,7 +2127,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)
> @@ -2130,8 +2138,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 161f0bf980b6..855c45fec5bb 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.25.1
> 

  parent reply	other threads:[~2026-08-24 20:35 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-21  5:02 [PATCH 0/3] perf annotate: Data type profiling support for C++ classes and virtual calls Yanbo Zhao
2026-08-21  5:02 ` [PATCH 1/3] perf dwarf-aux: Add die_is_compound_type() to handle C++ class types Yanbo Zhao
2026-08-21  5:13   ` sashiko-bot
2026-08-24 20:35   ` Namhyung Kim [this message]
2026-08-21  5:02 ` [PATCH 2/3] perf dwarf-aux: Add C++ class, inheritance and vtable helpers Yanbo Zhao
2026-08-21  5:12   ` sashiko-bot
2026-08-24 21:04   ` Namhyung Kim
2026-08-21  5:02 ` [PATCH 3/3] perf disasm & annotate x86: Track C++ vptr and resolve virtual function calls Yanbo Zhao
2026-08-21  5:26   ` sashiko-bot
2026-08-24 21:09     ` Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aoyrHe2MBOf4ocqn@google.com \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=xliuprof@google.com \
    --cc=yzhao62@ncsu.edu \
    --cc=zli94@ncsu.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.