[08/48] perf dwarf-aux: Factor out __die_get_typename()

Message ID 20231012035111.676789-9-namhyung@kernel.org
State New
Headers
Series perf tools: Introduce data type profiling (v1) |

Commit Message

Namhyung Kim Oct. 12, 2023, 3:50 a.m. UTC
  The __die_get_typename() is to get the name of the given DIE in C-style
type name.  The difference from the die_get_typename() is that it does
not retrieve the DW_AT_type and use the given DIE directly.  This will
be used when users know the type DIE already.

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/dwarf-aux.c | 38 ++++++++++++++++++++++++++-----------
 tools/perf/util/dwarf-aux.h |  3 +++
 2 files changed, 30 insertions(+), 11 deletions(-)
  

Comments

Masami Hiramatsu (Google) Nov. 5, 2023, 9:07 a.m. UTC | #1
On Wed, 11 Oct 2023 20:50:31 -0700
Namhyung Kim <namhyung@kernel.org> wrote:

> The __die_get_typename() is to get the name of the given DIE in C-style
> type name.  The difference from the die_get_typename() is that it does
> not retrieve the DW_AT_type and use the given DIE directly.  This will
> be used when users know the type DIE already.

Hmm, I would rather like to have another name for this function.
What about 'die_get_typename_from_type()' ?

> 
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/dwarf-aux.c | 38 ++++++++++++++++++++++++++-----------
>  tools/perf/util/dwarf-aux.h |  3 +++
>  2 files changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 652e6e7368a2..5bb05c84d249 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -1051,32 +1051,28 @@ Dwarf_Die *die_find_member(Dwarf_Die *st_die, const char *name,
>  }
>  
>  /**
> - * die_get_typename - Get the name of given variable DIE
> - * @vr_die: a variable DIE
> + * __die_get_typename - Get the name of given type DIE
> + * @type: a type DIE
>   * @buf: a strbuf for result type name
>   *
> - * Get the name of @vr_die and stores it to @buf. Return 0 if succeeded.
> + * Get the name of @type_die and stores it to @buf. Return 0 if succeeded.
>   * and Return -ENOENT if failed to find type name.
>   * Note that the result will stores typedef name if possible, and stores
>   * "*(function_type)" if the type is a function pointer.
>   */
> -int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
> +int __die_get_typename(Dwarf_Die *type, struct strbuf *buf)

Could you use 'type_die' instead of 'type' if it is exposed?

Thank you,

>  {
> -	Dwarf_Die type;
>  	int tag, ret;
>  	const char *tmp = "";
>  
> -	if (__die_get_real_type(vr_die, &type) == NULL)
> -		return -ENOENT;
> -
> -	tag = dwarf_tag(&type);
> +	tag = dwarf_tag(type);
>  	if (tag == DW_TAG_array_type || tag == DW_TAG_pointer_type)
>  		tmp = "*";
>  	else if (tag == DW_TAG_subroutine_type) {
>  		/* Function pointer */
>  		return strbuf_add(buf, "(function_type)", 15);
>  	} else {
> -		const char *name = dwarf_diename(&type);
> +		const char *name = dwarf_diename(type);
>  
>  		if (tag == DW_TAG_union_type)
>  			tmp = "union ";
> @@ -1089,7 +1085,7 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
>  		/* Write a base name */
>  		return strbuf_addf(buf, "%s%s", tmp, name ?: "");
>  	}
> -	ret = die_get_typename(&type, buf);
> +	ret = die_get_typename(type, buf);
>  	if (ret < 0) {
>  		/* void pointer has no type attribute */
>  		if (tag == DW_TAG_pointer_type && ret == -ENOENT)
> @@ -1100,6 +1096,26 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
>  	return strbuf_addstr(buf, tmp);
>  }
>  
> +/**
> + * die_get_typename - Get the name of given variable DIE
> + * @vr_die: a variable DIE
> + * @buf: a strbuf for result type name
> + *
> + * Get the name of @vr_die and stores it to @buf. Return 0 if succeeded.
> + * and Return -ENOENT if failed to find type name.
> + * Note that the result will stores typedef name if possible, and stores
> + * "*(function_type)" if the type is a function pointer.
> + */
> +int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
> +{
> +	Dwarf_Die type;
> +
> +	if (__die_get_real_type(vr_die, &type) == NULL)
> +		return -ENOENT;
> +
> +	return __die_get_typename(&type, buf);
> +}
> +
>  /**
>   * die_get_varname - Get the name and type of given variable DIE
>   * @vr_die: a variable DIE
> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
> index b6f430730bd1..574405c57d3b 100644
> --- a/tools/perf/util/dwarf-aux.h
> +++ b/tools/perf/util/dwarf-aux.h
> @@ -116,6 +116,9 @@ Dwarf_Die *die_find_variable_at(Dwarf_Die *sp_die, const char *name,
>  Dwarf_Die *die_find_member(Dwarf_Die *st_die, const char *name,
>  			   Dwarf_Die *die_mem);
>  
> +/* Get the name of given type DIE */
> +int __die_get_typename(Dwarf_Die *type, struct strbuf *buf);
> +
>  /* Get the name of given variable DIE */
>  int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);
>  
> -- 
> 2.42.0.655.g421f12c284-goog
>
  
Namhyung Kim Nov. 6, 2023, 4:01 a.m. UTC | #2
Hi Masami,

On Sun, Nov 5, 2023 at 1:07 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> On Wed, 11 Oct 2023 20:50:31 -0700
> Namhyung Kim <namhyung@kernel.org> wrote:
>
> > The __die_get_typename() is to get the name of the given DIE in C-style
> > type name.  The difference from the die_get_typename() is that it does
> > not retrieve the DW_AT_type and use the given DIE directly.  This will
> > be used when users know the type DIE already.
>
> Hmm, I would rather like to have another name for this function.
> What about 'die_get_typename_from_type()' ?

Ok.

> >
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/util/dwarf-aux.c | 38 ++++++++++++++++++++++++++-----------
> >  tools/perf/util/dwarf-aux.h |  3 +++
> >  2 files changed, 30 insertions(+), 11 deletions(-)
> >
> > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> > index 652e6e7368a2..5bb05c84d249 100644
> > --- a/tools/perf/util/dwarf-aux.c
> > +++ b/tools/perf/util/dwarf-aux.c
> > @@ -1051,32 +1051,28 @@ Dwarf_Die *die_find_member(Dwarf_Die *st_die, const char *name,
> >  }
> >
> >  /**
> > - * die_get_typename - Get the name of given variable DIE
> > - * @vr_die: a variable DIE
> > + * __die_get_typename - Get the name of given type DIE
> > + * @type: a type DIE
> >   * @buf: a strbuf for result type name
> >   *
> > - * Get the name of @vr_die and stores it to @buf. Return 0 if succeeded.
> > + * Get the name of @type_die and stores it to @buf. Return 0 if succeeded.
> >   * and Return -ENOENT if failed to find type name.
> >   * Note that the result will stores typedef name if possible, and stores
> >   * "*(function_type)" if the type is a function pointer.
> >   */
> > -int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
> > +int __die_get_typename(Dwarf_Die *type, struct strbuf *buf)
>
> Could you use 'type_die' instead of 'type' if it is exposed?

Will change, thanks for your review!
Namhyung


> >  {
> > -     Dwarf_Die type;
> >       int tag, ret;
> >       const char *tmp = "";
> >
> > -     if (__die_get_real_type(vr_die, &type) == NULL)
> > -             return -ENOENT;
> > -
> > -     tag = dwarf_tag(&type);
> > +     tag = dwarf_tag(type);
> >       if (tag == DW_TAG_array_type || tag == DW_TAG_pointer_type)
> >               tmp = "*";
> >       else if (tag == DW_TAG_subroutine_type) {
> >               /* Function pointer */
> >               return strbuf_add(buf, "(function_type)", 15);
> >       } else {
> > -             const char *name = dwarf_diename(&type);
> > +             const char *name = dwarf_diename(type);
> >
> >               if (tag == DW_TAG_union_type)
> >                       tmp = "union ";
> > @@ -1089,7 +1085,7 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
> >               /* Write a base name */
> >               return strbuf_addf(buf, "%s%s", tmp, name ?: "");
> >       }
> > -     ret = die_get_typename(&type, buf);
> > +     ret = die_get_typename(type, buf);
> >       if (ret < 0) {
> >               /* void pointer has no type attribute */
> >               if (tag == DW_TAG_pointer_type && ret == -ENOENT)
> > @@ -1100,6 +1096,26 @@ int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
> >       return strbuf_addstr(buf, tmp);
> >  }
> >
> > +/**
> > + * die_get_typename - Get the name of given variable DIE
> > + * @vr_die: a variable DIE
> > + * @buf: a strbuf for result type name
> > + *
> > + * Get the name of @vr_die and stores it to @buf. Return 0 if succeeded.
> > + * and Return -ENOENT if failed to find type name.
> > + * Note that the result will stores typedef name if possible, and stores
> > + * "*(function_type)" if the type is a function pointer.
> > + */
> > +int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
> > +{
> > +     Dwarf_Die type;
> > +
> > +     if (__die_get_real_type(vr_die, &type) == NULL)
> > +             return -ENOENT;
> > +
> > +     return __die_get_typename(&type, buf);
> > +}
> > +
> >  /**
> >   * die_get_varname - Get the name and type of given variable DIE
> >   * @vr_die: a variable DIE
> > diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
> > index b6f430730bd1..574405c57d3b 100644
> > --- a/tools/perf/util/dwarf-aux.h
> > +++ b/tools/perf/util/dwarf-aux.h
> > @@ -116,6 +116,9 @@ Dwarf_Die *die_find_variable_at(Dwarf_Die *sp_die, const char *name,
> >  Dwarf_Die *die_find_member(Dwarf_Die *st_die, const char *name,
> >                          Dwarf_Die *die_mem);
> >
> > +/* Get the name of given type DIE */
> > +int __die_get_typename(Dwarf_Die *type, struct strbuf *buf);
> > +
> >  /* Get the name of given variable DIE */
> >  int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);
> >
> > --
> > 2.42.0.655.g421f12c284-goog
> >
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
  

Patch

diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
index 652e6e7368a2..5bb05c84d249 100644
--- a/tools/perf/util/dwarf-aux.c
+++ b/tools/perf/util/dwarf-aux.c
@@ -1051,32 +1051,28 @@  Dwarf_Die *die_find_member(Dwarf_Die *st_die, const char *name,
 }
 
 /**
- * die_get_typename - Get the name of given variable DIE
- * @vr_die: a variable DIE
+ * __die_get_typename - Get the name of given type DIE
+ * @type: a type DIE
  * @buf: a strbuf for result type name
  *
- * Get the name of @vr_die and stores it to @buf. Return 0 if succeeded.
+ * Get the name of @type_die and stores it to @buf. Return 0 if succeeded.
  * and Return -ENOENT if failed to find type name.
  * Note that the result will stores typedef name if possible, and stores
  * "*(function_type)" if the type is a function pointer.
  */
-int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
+int __die_get_typename(Dwarf_Die *type, struct strbuf *buf)
 {
-	Dwarf_Die type;
 	int tag, ret;
 	const char *tmp = "";
 
-	if (__die_get_real_type(vr_die, &type) == NULL)
-		return -ENOENT;
-
-	tag = dwarf_tag(&type);
+	tag = dwarf_tag(type);
 	if (tag == DW_TAG_array_type || tag == DW_TAG_pointer_type)
 		tmp = "*";
 	else if (tag == DW_TAG_subroutine_type) {
 		/* Function pointer */
 		return strbuf_add(buf, "(function_type)", 15);
 	} else {
-		const char *name = dwarf_diename(&type);
+		const char *name = dwarf_diename(type);
 
 		if (tag == DW_TAG_union_type)
 			tmp = "union ";
@@ -1089,7 +1085,7 @@  int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
 		/* Write a base name */
 		return strbuf_addf(buf, "%s%s", tmp, name ?: "");
 	}
-	ret = die_get_typename(&type, buf);
+	ret = die_get_typename(type, buf);
 	if (ret < 0) {
 		/* void pointer has no type attribute */
 		if (tag == DW_TAG_pointer_type && ret == -ENOENT)
@@ -1100,6 +1096,26 @@  int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
 	return strbuf_addstr(buf, tmp);
 }
 
+/**
+ * die_get_typename - Get the name of given variable DIE
+ * @vr_die: a variable DIE
+ * @buf: a strbuf for result type name
+ *
+ * Get the name of @vr_die and stores it to @buf. Return 0 if succeeded.
+ * and Return -ENOENT if failed to find type name.
+ * Note that the result will stores typedef name if possible, and stores
+ * "*(function_type)" if the type is a function pointer.
+ */
+int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf)
+{
+	Dwarf_Die type;
+
+	if (__die_get_real_type(vr_die, &type) == NULL)
+		return -ENOENT;
+
+	return __die_get_typename(&type, buf);
+}
+
 /**
  * die_get_varname - Get the name and type of given variable DIE
  * @vr_die: a variable DIE
diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
index b6f430730bd1..574405c57d3b 100644
--- a/tools/perf/util/dwarf-aux.h
+++ b/tools/perf/util/dwarf-aux.h
@@ -116,6 +116,9 @@  Dwarf_Die *die_find_variable_at(Dwarf_Die *sp_die, const char *name,
 Dwarf_Die *die_find_member(Dwarf_Die *st_die, const char *name,
 			   Dwarf_Die *die_mem);
 
+/* Get the name of given type DIE */
+int __die_get_typename(Dwarf_Die *type, struct strbuf *buf);
+
 /* Get the name of given variable DIE */
 int die_get_typename(Dwarf_Die *vr_die, struct strbuf *buf);