1. Patchew
  2. linux
  3. View series

[PATCH] perf annotate-data: Show typedef names properly

Namhyung Kim posted 1 patch 1 year, 6 months ago 
tools/perf/util/annotate-data.c | 39 +++++++++++++++++++++++++--------
tools/perf/util/dwarf-aux.c     |  2 +-
tools/perf/util/dwarf-aux.h     |  2 ++
3 files changed, 33 insertions(+), 10 deletions(-)
[PATCH] perf annotate-data: Show typedef names properly
Posted by Namhyung Kim 1 year, 6 months ago 
The die_get_typename() would resolve typedef and get to the original
type.  But sometimes the original type is a struct without name and it
makes the output confusing and hard to read.

This is a diff of perf report -s type before and after the change.
New types such as atomic{,64}_t and sigset_t appeared and the portion
of unnamed struct was reduced.  Also u32, u64 and size_t were splitted
from the base types.

  --- b   2024-08-01 17:02:34.307809952 -0700
  +++ a   2024-08-07 14:17:05.245853999 -0700
  -     2.40%  long unsigned int
  +     2.26%  long unsigned int
  -     1.56%  unsigned int
  +     1.27%  unsigned int
  -     0.98%  struct
  -     0.79%  long long unsigned int
  +     0.58%  long long unsigned int
  +     0.36%  struct
  +     0.27%  atomic64_t
  +     0.22%  u32
  +     0.21%  u64
  +     0.19%  atomic_t
  +     0.13%  size_t
  -     0.08%  struct seqcount_spinlock
  +     0.08%  seqcount_spinlock_t
  +     0.08%  sigset_t
  +     0.08%  __poll_t

Let's use the typedef name directly and the resolved to get the size of
the type.

Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/annotate-data.c | 39 +++++++++++++++++++++++++--------
 tools/perf/util/dwarf-aux.c     |  2 +-
 tools/perf/util/dwarf-aux.h     |  2 ++
 3 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index 734acdd8c4b7..092809bb92f5 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -217,8 +217,13 @@ static int __add_member_cb(Dwarf_Die *die, void *arg)
 	strbuf_init(&sb, 32);
 	die_get_typename(die, &sb);
 
-	die_get_real_type(die, &member_type);
-	if (dwarf_aggregate_size(&member_type, &size) < 0)
+	__die_get_real_type(die, &member_type);
+	if (dwarf_tag(&member_type) == DW_TAG_typedef)
+		die_get_real_type(&member_type, &die_mem);
+	else
+		die_mem = member_type;
+
+	if (dwarf_aggregate_size(&die_mem, &size) < 0)
 		size = 0;
 
 	if (!dwarf_attr_integrate(die, DW_AT_data_member_location, &attr))
@@ -235,11 +240,11 @@ 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(&member_type);
+	tag = dwarf_tag(&die_mem);
 	switch (tag) {
 	case DW_TAG_structure_type:
 	case DW_TAG_union_type:
-		die_find_child(&member_type, __add_member_cb, member, &die_mem);
+		die_find_child(&die_mem, __add_member_cb, member, &die_mem);
 		break;
 	default:
 		break;
@@ -281,6 +286,10 @@ static struct annotated_data_type *dso__findnew_data_type(struct dso *dso,
 	if (die_get_typename_from_type(type_die, &sb) < 0)
 		strbuf_add(&sb, "(unknown type)", 14);
 	type_name = strbuf_detach(&sb, NULL);
+
+	if (dwarf_tag(type_die) == DW_TAG_typedef)
+		die_get_real_type(type_die, type_die);
+
 	dwarf_aggregate_size(type_die, &size);
 
 	/* Check existing nodes in dso->data_types tree */
@@ -342,6 +351,7 @@ static int check_variable(struct data_loc_info *dloc, Dwarf_Die *var_die,
 {
 	Dwarf_Word size;
 	bool is_pointer = true;
+	Dwarf_Die sized_type;
 
 	if (reg == DWARF_REG_PC)
 		is_pointer = false;
@@ -351,7 +361,7 @@ static int check_variable(struct data_loc_info *dloc, Dwarf_Die *var_die,
 		is_pointer = false;
 
 	/* Get the type of the variable */
-	if (die_get_real_type(var_die, type_die) == NULL) {
+	if (__die_get_real_type(var_die, type_die) == NULL) {
 		pr_debug_dtp("variable has no type\n");
 		ann_data_stat.no_typeinfo++;
 		return -1;
@@ -365,15 +375,20 @@ static int check_variable(struct data_loc_info *dloc, Dwarf_Die *var_die,
 	if (is_pointer) {
 		if ((dwarf_tag(type_die) != DW_TAG_pointer_type &&
 		     dwarf_tag(type_die) != DW_TAG_array_type) ||
-		    die_get_real_type(type_die, type_die) == NULL) {
+		    __die_get_real_type(type_die, type_die) == NULL) {
 			pr_debug_dtp("no pointer or no type\n");
 			ann_data_stat.no_typeinfo++;
 			return -1;
 		}
 	}
 
+	if (dwarf_tag(type_die) == DW_TAG_typedef)
+		die_get_real_type(type_die, &sized_type);
+	else
+		sized_type = *type_die;
+
 	/* Get the size of the actual type */
-	if (dwarf_aggregate_size(type_die, &size) < 0) {
+	if (dwarf_aggregate_size(&sized_type, &size) < 0) {
 		pr_debug_dtp("type size is unknown\n");
 		ann_data_stat.invalid_size++;
 		return -1;
@@ -846,6 +861,7 @@ static int check_matching_type(struct type_state *state,
 
 	if (state->regs[reg].ok && state->regs[reg].kind == TSR_KIND_TYPE) {
 		int tag = dwarf_tag(&state->regs[reg].type);
+		Dwarf_Die sized_type;
 
 		/*
 		 * Normal registers should hold a pointer (or array) to
@@ -862,13 +878,18 @@ static int check_matching_type(struct type_state *state,
 		pr_debug_dtp("\n");
 
 		/* Remove the pointer and get the target type */
-		if (die_get_real_type(&state->regs[reg].type, type_die) == NULL)
+		if (__die_get_real_type(&state->regs[reg].type, type_die) == NULL)
 			return -1;
 
 		dloc->type_offset = dloc->op->offset;
 
+		if (dwarf_tag(type_die) == DW_TAG_typedef)
+			die_get_real_type(type_die, &sized_type);
+		else
+			sized_type = *type_die;
+
 		/* Get the size of the actual type */
-		if (dwarf_aggregate_size(type_die, &size) < 0 ||
+		if (dwarf_aggregate_size(&sized_type, &size) < 0 ||
 		    (unsigned)dloc->type_offset >= size)
 			return -1;
 
diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 44ef968a7ad3..5e080d7e22c2 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -267,7 +267,7 @@ Dwarf_Die *die_get_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)
 }
 
 /* Get a type die, but skip qualifiers */
-static Dwarf_Die *__die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)
+Dwarf_Die *__die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)
 {
 	int tag;
 
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index 24446412b869..336a3a183a78 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -56,6 +56,8 @@ const char *die_get_decl_file(Dwarf_Die *dw_die);
 /* Get type die */
 Dwarf_Die *die_get_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem);
 
+/* Get a type die, but skip qualifiers */
+Dwarf_Die *__die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem);
 /* Get a type die, but skip qualifiers and typedef */
 Dwarf_Die *die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem);
 
-- 
2.46.0.rc2.264.g509ed76dc8-goog
Re: [PATCH] perf annotate-data: Show typedef names properly
Posted by Arnaldo Carvalho de Melo 1 year, 6 months ago 
On Wed, Aug 07, 2024 at 03:31:29PM -0700, Namhyung Kim wrote:
> The die_get_typename() would resolve typedef and get to the original
> type.  But sometimes the original type is a struct without name and it
> makes the output confusing and hard to read.
> 
> This is a diff of perf report -s type before and after the change.
> New types such as atomic{,64}_t and sigset_t appeared and the portion
> of unnamed struct was reduced.  Also u32, u64 and size_t were splitted
> from the base types.
> 
>   --- b   2024-08-01 17:02:34.307809952 -0700
>   +++ a   2024-08-07 14:17:05.245853999 -0700
>   -     2.40%  long unsigned int
>   +     2.26%  long unsigned int
>   -     1.56%  unsigned int
>   +     1.27%  unsigned int
>   -     0.98%  struct
>   -     0.79%  long long unsigned int
>   +     0.58%  long long unsigned int
>   +     0.36%  struct
>   +     0.27%  atomic64_t
>   +     0.22%  u32
>   +     0.21%  u64
>   +     0.19%  atomic_t
>   +     0.13%  size_t
>   -     0.08%  struct seqcount_spinlock
>   +     0.08%  seqcount_spinlock_t
>   +     0.08%  sigset_t
>   +     0.08%  __poll_t
> 
> Let's use the typedef name directly and the resolved to get the size of
> the type.

Great improvement! Tested and applied.

- Arnaldo
 
> Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/annotate-data.c | 39 +++++++++++++++++++++++++--------
>  tools/perf/util/dwarf-aux.c     |  2 +-
>  tools/perf/util/dwarf-aux.h     |  2 ++
>  3 files changed, 33 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> index 734acdd8c4b7..092809bb92f5 100644
> --- a/tools/perf/util/annotate-data.c
> +++ b/tools/perf/util/annotate-data.c
> @@ -217,8 +217,13 @@ static int __add_member_cb(Dwarf_Die *die, void *arg)
>  	strbuf_init(&sb, 32);
>  	die_get_typename(die, &sb);
>  
> -	die_get_real_type(die, &member_type);
> -	if (dwarf_aggregate_size(&member_type, &size) < 0)
> +	__die_get_real_type(die, &member_type);
> +	if (dwarf_tag(&member_type) == DW_TAG_typedef)
> +		die_get_real_type(&member_type, &die_mem);
> +	else
> +		die_mem = member_type;
> +
> +	if (dwarf_aggregate_size(&die_mem, &size) < 0)
>  		size = 0;
>  
>  	if (!dwarf_attr_integrate(die, DW_AT_data_member_location, &attr))
> @@ -235,11 +240,11 @@ 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(&member_type);
> +	tag = dwarf_tag(&die_mem);
>  	switch (tag) {
>  	case DW_TAG_structure_type:
>  	case DW_TAG_union_type:
> -		die_find_child(&member_type, __add_member_cb, member, &die_mem);
> +		die_find_child(&die_mem, __add_member_cb, member, &die_mem);
>  		break;
>  	default:
>  		break;
> @@ -281,6 +286,10 @@ static struct annotated_data_type *dso__findnew_data_type(struct dso *dso,
>  	if (die_get_typename_from_type(type_die, &sb) < 0)
>  		strbuf_add(&sb, "(unknown type)", 14);
>  	type_name = strbuf_detach(&sb, NULL);
> +
> +	if (dwarf_tag(type_die) == DW_TAG_typedef)
> +		die_get_real_type(type_die, type_die);
> +
>  	dwarf_aggregate_size(type_die, &size);
>  
>  	/* Check existing nodes in dso->data_types tree */
> @@ -342,6 +351,7 @@ static int check_variable(struct data_loc_info *dloc, Dwarf_Die *var_die,
>  {
>  	Dwarf_Word size;
>  	bool is_pointer = true;
> +	Dwarf_Die sized_type;
>  
>  	if (reg == DWARF_REG_PC)
>  		is_pointer = false;
> @@ -351,7 +361,7 @@ static int check_variable(struct data_loc_info *dloc, Dwarf_Die *var_die,
>  		is_pointer = false;
>  
>  	/* Get the type of the variable */
> -	if (die_get_real_type(var_die, type_die) == NULL) {
> +	if (__die_get_real_type(var_die, type_die) == NULL) {
>  		pr_debug_dtp("variable has no type\n");
>  		ann_data_stat.no_typeinfo++;
>  		return -1;
> @@ -365,15 +375,20 @@ static int check_variable(struct data_loc_info *dloc, Dwarf_Die *var_die,
>  	if (is_pointer) {
>  		if ((dwarf_tag(type_die) != DW_TAG_pointer_type &&
>  		     dwarf_tag(type_die) != DW_TAG_array_type) ||
> -		    die_get_real_type(type_die, type_die) == NULL) {
> +		    __die_get_real_type(type_die, type_die) == NULL) {
>  			pr_debug_dtp("no pointer or no type\n");
>  			ann_data_stat.no_typeinfo++;
>  			return -1;
>  		}
>  	}
>  
> +	if (dwarf_tag(type_die) == DW_TAG_typedef)
> +		die_get_real_type(type_die, &sized_type);
> +	else
> +		sized_type = *type_die;
> +
>  	/* Get the size of the actual type */
> -	if (dwarf_aggregate_size(type_die, &size) < 0) {
> +	if (dwarf_aggregate_size(&sized_type, &size) < 0) {
>  		pr_debug_dtp("type size is unknown\n");
>  		ann_data_stat.invalid_size++;
>  		return -1;
> @@ -846,6 +861,7 @@ static int check_matching_type(struct type_state *state,
>  
>  	if (state->regs[reg].ok && state->regs[reg].kind == TSR_KIND_TYPE) {
>  		int tag = dwarf_tag(&state->regs[reg].type);
> +		Dwarf_Die sized_type;
>  
>  		/*
>  		 * Normal registers should hold a pointer (or array) to
> @@ -862,13 +878,18 @@ static int check_matching_type(struct type_state *state,
>  		pr_debug_dtp("\n");
>  
>  		/* Remove the pointer and get the target type */
> -		if (die_get_real_type(&state->regs[reg].type, type_die) == NULL)
> +		if (__die_get_real_type(&state->regs[reg].type, type_die) == NULL)
>  			return -1;
>  
>  		dloc->type_offset = dloc->op->offset;
>  
> +		if (dwarf_tag(type_die) == DW_TAG_typedef)
> +			die_get_real_type(type_die, &sized_type);
> +		else
> +			sized_type = *type_die;
> +
>  		/* Get the size of the actual type */
> -		if (dwarf_aggregate_size(type_die, &size) < 0 ||
> +		if (dwarf_aggregate_size(&sized_type, &size) < 0 ||
>  		    (unsigned)dloc->type_offset >= size)
>  			return -1;
>  
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 44ef968a7ad3..5e080d7e22c2 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -267,7 +267,7 @@ Dwarf_Die *die_get_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)
>  }
>  
>  /* Get a type die, but skip qualifiers */
> -static Dwarf_Die *__die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)
> +Dwarf_Die *__die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)
>  {
>  	int tag;
>  
> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
> index 24446412b869..336a3a183a78 100644
> --- a/tools/perf/util/dwarf-aux.h
> +++ b/tools/perf/util/dwarf-aux.h
> @@ -56,6 +56,8 @@ const char *die_get_decl_file(Dwarf_Die *dw_die);
>  /* Get type die */
>  Dwarf_Die *die_get_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem);
>  
> +/* Get a type die, but skip qualifiers */
> +Dwarf_Die *__die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem);
>  /* Get a type die, but skip qualifiers and typedef */
>  Dwarf_Die *die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem);
>  
> -- 
> 2.46.0.rc2.264.g509ed76dc8-goog

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.