public inbox for linux-perf-users@vger.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: "Steinar H. Gunderson" <sesse@google.com>,
	Namhyung Kim <namhyung@kernel.org>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] perf probe: Improve C++ support in arguments
Date: Wed, 1 Apr 2026 18:11:16 -0300	[thread overview]
Message-ID: <ac2J9BRFsmzdJo2J@x1> (raw)
In-Reply-To: <CAP-5=fW7y93ZfSF0Mrt3nB1Jqu4HaKypEUhH88CTyy9-v4JPdA@mail.gmail.com>

On Wed, Apr 01, 2026 at 02:03:42PM -0700, Ian Rogers wrote:
> On Fri, Dec 5, 2025 at 4:36 AM Steinar H. Gunderson <sesse@google.com> wrote:
> >
> > For purposes of specifying arguments in tracepoints, C++ classes
> > work exactly like structs, and C++ references work almost exactly
> > like pointers (they are dereferenced with . instead of ->).
> > However, they use different tags in DWARF, which means you cannot
> > use them in arguments.
> >
> > Implement support for both, based on the existing struct and pointer
> > support. This allows us to to something like this in Chromium:
> >
> >   perf probe --exec content_shell --add 'PerformanceMark=\
> > _ZN5blink15PerformanceMark6CreateEPNS_11ScriptStateERKNS_12\
> > AtomicStringEPNS_22PerformanceMarkOptionsERNS_14ExceptionStateE \
> > mark_name.string_.impl_.ptr_'
> >
> > even though mark_name is an class AtomicString&.
> >
> > We don't support advanced C++ features, such as operator overloading.
> > Unfortunately, we also don't support C++-style strings where there
> > is a separate length field instead of null-termination. We also cannot
> > go past ptr_ in the example above (e.g. add ->length_), due to
> > unrelated DWARF issues.
> >
> > Signed-off-by: Steinar H. Gunderson <sesse@google.com>
> > Cc: Ian Rogers <irogers@google.com>
> 
> This change makes sense to me, Namhyung?

Agreed, and there are some other C++ specific (Go as well), like
DW_TAG_rvalue_reference_type, that we may want to support, also it would
be great to have some extra workload in 'perf test -w' to have a shell
testing this feature, no?

- Arnaldo
 
> Thanks,
> Ian
> 
> > ---
> >  tools/perf/util/annotate-data.c |  3 ++-
> >  tools/perf/util/dwarf-aux.c     | 13 ++++++++++---
> >  tools/perf/util/probe-finder.c  | 24 +++++++++++++++++-------
> >  3 files changed, 29 insertions(+), 11 deletions(-)
> >
> > diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
> > index 07cf9c334be0..d97b5fc61e6f 100644
> > --- a/tools/perf/util/annotate-data.c
> > +++ b/tools/perf/util/annotate-data.c
> > @@ -459,7 +459,8 @@ static bool is_pointer_type(Dwarf_Die *type_die)
> >  {
> >         int tag = dwarf_tag(type_die);
> >
> > -       return tag == DW_TAG_pointer_type || tag == DW_TAG_array_type;
> > +       return tag == DW_TAG_pointer_type || tag == DW_TAG_reference_type ||
> > +              tag == DW_TAG_array_type;
> >  }
> >
> >  static bool is_compound_type(Dwarf_Die *type_die)
> > diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> > index 9267af204c7d..25bbc840d1a8 100644
> > --- a/tools/perf/util/dwarf-aux.c
> > +++ b/tools/perf/util/dwarf-aux.c
> > @@ -1112,6 +1112,8 @@ int die_get_typename_from_type(Dwarf_Die *type_die, struct strbuf *buf)
> >         tag = dwarf_tag(type_die);
> >         if (tag == DW_TAG_pointer_type)
> >                 tmp = "*";
> > +       else if (tag == DW_TAG_reference_type)
> > +               tmp = "&";
> >         else if (tag == DW_TAG_array_type)
> >                 tmp = "[]";
> >         else if (tag == DW_TAG_subroutine_type) {
> > @@ -1124,6 +1126,8 @@ int die_get_typename_from_type(Dwarf_Die *type_die, struct strbuf *buf)
> >                         tmp = "union ";
> >                 else if (tag == DW_TAG_structure_type)
> >                         tmp = "struct ";
> > +               else if (tag == DW_TAG_class_type)
> > +                       tmp = "class ";
> >                 else if (tag == DW_TAG_enumeration_type)
> >                         tmp = "enum ";
> >                 else if (name == NULL)
> > @@ -2065,7 +2069,8 @@ 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 (tag != DW_TAG_structure_type && tag != DW_TAG_class_type &&
> > +           tag != DW_TAG_union_type) {
> >                 Dwarf_Word size;
> >
> >                 if (dwarf_aggregate_size(type_die, &size) < 0)
> > @@ -2080,7 +2085,8 @@ 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 (tag == DW_TAG_structure_type || tag == DW_TAG_class_type ||
> > +              tag == DW_TAG_union_type) {
> >                 member = die_find_child(&mb_type, __die_find_member_offset_cb,
> >                                         (void *)(long)offset, die_mem);
> >                 if (member == NULL)
> > @@ -2091,7 +2097,8 @@ 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) {
> > +               if (tag == DW_TAG_structure_type || tag == DW_TAG_class_type ||
> > +                   tag == DW_TAG_union_type) {
> >                         Dwarf_Word loc;
> >
> >                         /* Update offset for the start of the member struct */
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index 5ffd97ee4898..ca4734c75bba 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -360,12 +360,20 @@ static int convert_variable_fields(Dwarf_Die *vr_die, const char *varname,
> >                 ref->offset += dwarf_bytesize(&type) * field->index;
> >                 ref->user_access = user_access;
> >                 goto next;
> > -       } else if (tag == DW_TAG_pointer_type) {
> > +       } else if (tag == DW_TAG_pointer_type || tag == DW_TAG_reference_type) {
> >                 /* Check the pointer and dereference */
> > -               if (!field->ref) {
> > -                       pr_err("Semantic error: %s must be referred by '->'\n",
> > -                              field->name);
> > -                       return -EINVAL;
> > +               if (tag == DW_TAG_pointer_type) {
> > +                       if (!field->ref) {
> > +                               pr_err("Semantic error: %s must be referred by '->'\n",
> > +                                      field->name);
> > +                               return -EINVAL;
> > +                       }
> > +               } else {
> > +                       if (field->ref) {
> > +                               pr_err("Semantic error: %s must be referred by '.'\n",
> > +                                      field->name);
> > +                               return -EINVAL;
> > +                       }
> >                 }
> >                 /* Get the type pointed by this pointer */
> >                 if (die_get_real_type(&type, &type) == NULL) {
> > @@ -374,7 +382,8 @@ static int convert_variable_fields(Dwarf_Die *vr_die, const char *varname,
> >                 }
> >                 /* Verify it is a data structure  */
> >                 tag = dwarf_tag(&type);
> > -               if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type) {
> > +               if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type &&
> > +                   tag != DW_TAG_class_type) {
> >                         pr_warning("%s is not a data structure nor a union.\n",
> >                                    varname);
> >                         return -EINVAL;
> > @@ -389,7 +398,8 @@ static int convert_variable_fields(Dwarf_Die *vr_die, const char *varname,
> >                         *ref_ptr = ref;
> >         } else {
> >                 /* Verify it is a data structure  */
> > -               if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type) {
> > +               if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type &&
> > +                   tag != DW_TAG_class_type) {
> >                         pr_warning("%s is not a data structure nor a union.\n",
> >                                    varname);
> >                         return -EINVAL;
> > --
> > 2.51.0
> >

  reply	other threads:[~2026-04-01 21:11 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-05 12:36 [PATCH] perf probe: Improve C++ support in arguments Steinar H. Gunderson
2026-04-01 21:03 ` Ian Rogers
2026-04-01 21:11   ` Arnaldo Carvalho de Melo [this message]
2026-04-01 23:23     ` 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=ac2J9BRFsmzdJo2J@x1 \
    --to=acme@kernel.org \
    --cc=irogers@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=namhyung@kernel.org \
    --cc=sesse@google.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox