If a symbol name is longer than the maximum event length (64 bytes),
generate an new event name with below combination:
TruncatedSymbol + '_' + HashString + '__return' + '\0'
`> 46B + 1B + 8B + 8B + 1B = 64 Bytes.
With this change, a probe can be injected for long symbol.
Before:
# nm test_cpp_mangle | grep -E "print_data|Point"
0000000000000cac t _GLOBAL__sub_I__Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi
0000000000000b50 T _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzR5Point
0000000000000b14 T _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi
# perf probe -x test_cpp_mangle --add \
"_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi"
snprintf() failed: -7; the event name nbase='_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi' is too long
Error: Failed to add events.
After:
# perf probe -x test_cpp_mangle --add \
"_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi"
Probe event='_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi' is too long (>= 64 bytes).
Generate hashed event name='_Z62this_is_a_very_very_long_print_data_abcdef_91f40679'
Added new event:
probe_test_cpp_mangle: _Z62this_is_a_very_very_long_print_data_abcdef_91f40679
(on _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi in /mnt/test_cpp_mangle)
You can now use it in all perf tools, such as:
perf record -e probe_test_cpp_mangle: _Z62this_is_a_very_very_long_print_data_abcdef_91f40679 -aR sleep 1
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
tools/perf/util/probe-event.c | 42 ++++++++++++++++++++++++++++++++++-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 71acea07cb46..bacd29b95c75 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2837,6 +2837,32 @@ static void warn_uprobe_event_compat(struct probe_trace_event *tev)
/* Defined in kernel/trace/trace.h */
#define MAX_EVENT_NAME_LEN 64
+static char *probe_trace_event__hash_event(const char *event)
+{
+ char *str = NULL;
+ size_t hash;
+
+ str = malloc(MAX_EVENT_NAME_LEN);
+ if (!str)
+ return NULL;
+
+ hash = str_hash(event);
+
+ /*
+ * Reserve characters for the "__return" suffix for the return probe.
+ * Thus the string buffer (64 bytes) are used for:
+ * Truncated event: 46 bytes
+ * '_' : 1 byte
+ * hash string : 8 bytes
+ * reserved : 8 bytes (for suffix "__return")
+ * '\0' : 1 byte
+ */
+ strncpy(str, event, 46);
+ /* '_' + hash string + '\0' */
+ snprintf(str + 46, 10, "_%lx", hash);
+ return str;
+}
+
/* Set new name from original perf_probe_event and namelist */
static int probe_trace_event__set_name(struct probe_trace_event *tev,
struct perf_probe_event *pev,
@@ -2844,7 +2870,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
bool allow_suffix)
{
const char *event, *group;
- char *buf;
+ char *buf, *hash_event = NULL;
int ret;
buf = malloc(MAX_EVENT_NAME_LEN);
@@ -2864,6 +2890,19 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
event = pev->point.function;
else
event = tev->point.realname;
+
+ if (strlen(event) >= MAX_EVENT_NAME_LEN) {
+ pr_warning("Probe event='%s' is too long (>= %d bytes).\n",
+ event, MAX_EVENT_NAME_LEN);
+
+ hash_event = probe_trace_event__hash_event(event);
+ if (!hash_event) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ pr_warning("Generate hashed event name='%s'\n", hash_event);
+ event = hash_event;
+ }
}
if (pev->group && !pev->sdt)
group = pev->group;
@@ -2903,6 +2942,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
strlist__add(namelist, event);
out:
+ free(hash_event);
free(buf);
return ret < 0 ? ret : 0;
}
--
2.34.1On Mon, 7 Oct 2024 15:11:16 +0100
Leo Yan <leo.yan@arm.com> wrote:
> If a symbol name is longer than the maximum event length (64 bytes),
> generate an new event name with below combination:
>
> TruncatedSymbol + '_' + HashString + '__return' + '\0'
> `> 46B + 1B + 8B + 8B + 1B = 64 Bytes.
>
> With this change, a probe can be injected for long symbol.
>
> Before:
>
> # nm test_cpp_mangle | grep -E "print_data|Point"
> 0000000000000cac t _GLOBAL__sub_I__Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi
> 0000000000000b50 T _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzR5Point
> 0000000000000b14 T _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi
>
> # perf probe -x test_cpp_mangle --add \
> "_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi"
> snprintf() failed: -7; the event name nbase='_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi' is too long
> Error: Failed to add events.
>
> After:
>
> # perf probe -x test_cpp_mangle --add \
> "_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi"
>
> Probe event='_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi' is too long (>= 64 bytes).
> Generate hashed event name='_Z62this_is_a_very_very_long_print_data_abcdef_91f40679'
>
> Added new event:
> probe_test_cpp_mangle: _Z62this_is_a_very_very_long_print_data_abcdef_91f40679
> (on _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi in /mnt/test_cpp_mangle)
>
> You can now use it in all perf tools, such as:
>
> perf record -e probe_test_cpp_mangle: _Z62this_is_a_very_very_long_print_data_abcdef_91f40679 -aR sleep 1
OK, personally, I recommend you to specify event name instead of generating
long event name in this case. But I understand sometimes this kind of feature
is good for someone.
BTW, I would like to confirm. Can't we demangle the symbol name and parse it?
Thank you,
>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
> tools/perf/util/probe-event.c | 42 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 71acea07cb46..bacd29b95c75 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2837,6 +2837,32 @@ static void warn_uprobe_event_compat(struct probe_trace_event *tev)
> /* Defined in kernel/trace/trace.h */
> #define MAX_EVENT_NAME_LEN 64
>
> +static char *probe_trace_event__hash_event(const char *event)
> +{
> + char *str = NULL;
> + size_t hash;
> +
> + str = malloc(MAX_EVENT_NAME_LEN);
> + if (!str)
> + return NULL;
> +
> + hash = str_hash(event);
> +
> + /*
> + * Reserve characters for the "__return" suffix for the return probe.
> + * Thus the string buffer (64 bytes) are used for:
> + * Truncated event: 46 bytes
> + * '_' : 1 byte
> + * hash string : 8 bytes
> + * reserved : 8 bytes (for suffix "__return")
> + * '\0' : 1 byte
> + */
> + strncpy(str, event, 46);
> + /* '_' + hash string + '\0' */
> + snprintf(str + 46, 10, "_%lx", hash);
> + return str;
> +}
> +
> /* Set new name from original perf_probe_event and namelist */
> static int probe_trace_event__set_name(struct probe_trace_event *tev,
> struct perf_probe_event *pev,
> @@ -2844,7 +2870,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
> bool allow_suffix)
> {
> const char *event, *group;
> - char *buf;
> + char *buf, *hash_event = NULL;
> int ret;
>
> buf = malloc(MAX_EVENT_NAME_LEN);
> @@ -2864,6 +2890,19 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
> event = pev->point.function;
> else
> event = tev->point.realname;
> +
> + if (strlen(event) >= MAX_EVENT_NAME_LEN) {
> + pr_warning("Probe event='%s' is too long (>= %d bytes).\n",
> + event, MAX_EVENT_NAME_LEN);
> +
> + hash_event = probe_trace_event__hash_event(event);
> + if (!hash_event) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + pr_warning("Generate hashed event name='%s'\n", hash_event);
> + event = hash_event;
> + }
> }
> if (pev->group && !pev->sdt)
> group = pev->group;
> @@ -2903,6 +2942,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
> strlist__add(namelist, event);
>
> out:
> + free(hash_event);
> free(buf);
> return ret < 0 ? ret : 0;
> }
> --
> 2.34.1
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
Hi Masami, On 10/10/24 16:34, Masami Hiramatsu (Google) wrote: > > > On Mon, 7 Oct 2024 15:11:16 +0100 > Leo Yan <leo.yan@arm.com> wrote: > >> If a symbol name is longer than the maximum event length (64 bytes), >> generate an new event name with below combination: >> >> TruncatedSymbol + '_' + HashString + '__return' + '\0' >> `> 46B + 1B + 8B + 8B + 1B = 64 Bytes. >> >> With this change, a probe can be injected for long symbol. >> >> Before: >> >> # nm test_cpp_mangle | grep -E "print_data|Point" >> 0000000000000cac t _GLOBAL__sub_I__Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi >> 0000000000000b50 T _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzR5Point >> 0000000000000b14 T _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi >> >> # perf probe -x test_cpp_mangle --add \ >> "_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi" >> snprintf() failed: -7; the event name nbase='_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi' is too long >> Error: Failed to add events. >> >> After: >> >> # perf probe -x test_cpp_mangle --add \ >> "_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi" >> >> Probe event='_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi' is too long (>= 64 bytes). >> Generate hashed event name='_Z62this_is_a_very_very_long_print_data_abcdef_91f40679' >> >> Added new event: >> probe_test_cpp_mangle: _Z62this_is_a_very_very_long_print_data_abcdef_91f40679 >> (on _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi in /mnt/test_cpp_mangle) >> >> You can now use it in all perf tools, such as: >> >> perf record -e probe_test_cpp_mangle: _Z62this_is_a_very_very_long_print_data_abcdef_91f40679 -aR sleep 1 > > OK, personally, I recommend you to specify event name instead of generating > long event name in this case. But I understand sometimes this kind of feature > is good for someone. Sometimes, users try to add probe for long symbol and returns error, but there have no clue for proceeding. Either we automatically generate a hashed name, or a guidance in the failure log for setting event name would be helpful. If you have concern for hashed name, maybe we can refine the log to give info for setting event name? > BTW, I would like to confirm. Can't we demangle the symbol name and parse it? I did test for C++ demangle symbols with the command: perf probe -x /mnt/test_cpp_mangle -F --demangle The command doesn't work as I cannot see it output correctly for demangled symbols. But I don't look into details why this does not work at my side. Thanks for review. Leo
On Thu, 10 Oct 2024 16:53:05 +0100 Leo Yan <leo.yan@arm.com> wrote: > Hi Masami, > > On 10/10/24 16:34, Masami Hiramatsu (Google) wrote: > > > > > > On Mon, 7 Oct 2024 15:11:16 +0100 > > Leo Yan <leo.yan@arm.com> wrote: > > > >> If a symbol name is longer than the maximum event length (64 bytes), > >> generate an new event name with below combination: > >> > >> TruncatedSymbol + '_' + HashString + '__return' + '\0' > >> `> 46B + 1B + 8B + 8B + 1B = 64 Bytes. > >> > >> With this change, a probe can be injected for long symbol. > >> > >> Before: > >> > >> # nm test_cpp_mangle | grep -E "print_data|Point" > >> 0000000000000cac t _GLOBAL__sub_I__Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi > >> 0000000000000b50 T _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzR5Point > >> 0000000000000b14 T _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi > >> > >> # perf probe -x test_cpp_mangle --add \ > >> "_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi" > >> snprintf() failed: -7; the event name nbase='_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi' is too long > >> Error: Failed to add events. > >> > >> After: > >> > >> # perf probe -x test_cpp_mangle --add \ > >> "_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi" > >> > >> Probe event='_Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi' is too long (>= 64 bytes). > >> Generate hashed event name='_Z62this_is_a_very_very_long_print_data_abcdef_91f40679' > >> > >> Added new event: > >> probe_test_cpp_mangle: _Z62this_is_a_very_very_long_print_data_abcdef_91f40679 > >> (on _Z62this_is_a_very_very_long_print_data_abcdefghijklmnopqrstuvwxyzi in /mnt/test_cpp_mangle) > >> > >> You can now use it in all perf tools, such as: > >> > >> perf record -e probe_test_cpp_mangle: _Z62this_is_a_very_very_long_print_data_abcdef_91f40679 -aR sleep 1 > > > > OK, personally, I recommend you to specify event name instead of generating > > long event name in this case. But I understand sometimes this kind of feature > > is good for someone. > > Sometimes, users try to add probe for long symbol and returns error, but there > have no clue for proceeding. OK, no warning messsage is not good. It should warn them to recommend adding it with their own event name too. > Either we automatically generate a hashed name, or a guidance in the failure > log for setting event name would be helpful. If you have concern for hashed > name, maybe we can refine the log to give info for setting event name? Yeah, I think this long event name is not useful for user to type. > > BTW, I would like to confirm. Can't we demangle the symbol name and parse it? > > I did test for C++ demangle symbols with the command: > > perf probe -x /mnt/test_cpp_mangle -F --demangle > > The command doesn't work as I cannot see it output correctly for demangled > symbols. But I don't look into details why this does not work at my side. Oops, that is another issue to be solved. Thank you, > > Thanks for review. > > Leo -- Masami Hiramatsu (Google) <mhiramat@kernel.org>
On 10/11/2024 4:07 AM, Masami Hiramatsu (Google) wrote:
[...]
>>> OK, personally, I recommend you to specify event name instead of generating
>>> long event name in this case. But I understand sometimes this kind of feature
>>> is good for someone.
>>
>> Sometimes, users try to add probe for long symbol and returns error, but there
>> have no clue for proceeding.
>
> OK, no warning messsage is not good.
> It should warn them to recommend adding it with their own event name too.
Okay, will do this in next spin.
>> Either we automatically generate a hashed name, or a guidance in the failure
>> log for setting event name would be helpful. If you have concern for hashed
>> name, maybe we can refine the log to give info for setting event name?
>
> Yeah, I think this long event name is not useful for user to type.
Agreed.
>>> BTW, I would like to confirm. Can't we demangle the symbol name and parse it?
>>
>> I did test for C++ demangle symbols with the command:
>>
>> perf probe -x /mnt/test_cpp_mangle -F --demangle
>>
>> The command doesn't work as I cannot see it output correctly for demangled
>> symbols. But I don't look into details why this does not work at my side.
>
> Oops, that is another issue to be solved.
After install libiberty, then I can see the tool can show demangled symbols.
However, I found another issue for probing demangle symbol, please see details
in below patch and let me know if makes sense.
---8<---
From 3b09a6f89c7e383c6b1d2b7e6bd80c6bfa658d5b Mon Sep 17 00:00:00 2001
From: Leo Yan <leo.yan@arm.com>
Date: Fri, 11 Oct 2024 07:58:08 +0000
Subject: [PATCH] perf probe: Correct demangled symbols in C++ program
An issue can be observed when probe C++ demangled symbol with steps:
# nm test_cpp_mangle | grep print_data
0000000000000c94 t _GLOBAL__sub_I__Z10print_datai
0000000000000afc T _Z10print_datai
0000000000000b38 T _Z10print_dataR5Point
# ./perf probe -x /home/niayan01/test_cpp_mangle -F --demangle
...
print_data(Point&)
print_data(int)
...
# ./perf --debug verbose=3 probe -x test_cpp_mangle --add "test=print_data(int)"
probe-definition(0): test=print_data(int)
symbol:print_data(int) file:(null) line:0 offset:0 return:0 lazy:(null)
0 arguments
Open Debuginfo file: /home/niayan01/test_cpp_mangle
Try to find probe point from debuginfo.
Symbol print_data(int) address found : afc
Matched function: print_data [2ccf]
Probe point found: print_data+0
Found 1 probe_trace_events.
Opening /sys/kernel/tracing//uprobe_events write=1
Opening /sys/kernel/tracing//README write=0
Writing event: p:probe_test_cpp_mangle/test /home/niayan01/test_cpp_mangle:0xb38
...
When tried to probe symbol "print_data(int)", the log show:
Symbol print_data(int) address found : afc
The found address is 0xafc - which is right if we connect with the
output result from nm. Afterwards when write event, the command uses
offset 0xb38 in the last log, which is a wrong address.
The dwarf_diename() gets a common function name, in above case, it
returns string "print_data". As a result, the tool parses the offset
based on the common name. This leads to probe at the wrong symbol
"print_data(Point&)".
To fix the issue, use the die_get_linkage_name() function to retrieve
the distinct linkage name - this is the mangled name for the C++ case.
Based on this unique name, the tool can get a correct offset for
probing. Based on DWARF doc, it is possible the linkage name is missed
in the DIE, it rolls back to use dwarf_diename().
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
tools/perf/util/probe-finder.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 630e16c54ed5..28a14005fc1f 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1583,8 +1583,21 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr,
/* Find a corresponding function (name, baseline and baseaddr) */
if (die_find_realfunc(&cudie, (Dwarf_Addr)addr, &spdie)) {
- /* Get function entry information */
- func = basefunc = dwarf_diename(&spdie);
+ /*
+ * Get function entry information.
+ *
+ * As described in the document DWARF Debugging Information
+ * Format Version 5, section 2.22 Linkage Names, "mangled names,
+ * are used in various ways, ... to distinguish multiple
+ * entities that have the same name".
+ *
+ * Firstly try to get distinct linkage name, if fail then
+ * rollback to get associated name in DIE.
+ */
+ func = basefunc = die_get_linkage_name(&spdie);
+ if (!func)
+ func = basefunc = dwarf_diename(&spdie);
+
if (!func ||
die_entrypc(&spdie, &baseaddr) != 0 ||
dwarf_decl_line(&spdie, &baseline) != 0) {
--
2.25.1
On Fri, 11 Oct 2024 09:41:39 +0100
Leo Yan <leo.yan@arm.com> wrote:
> On 10/11/2024 4:07 AM, Masami Hiramatsu (Google) wrote:
>
> [...]
>
> >>> OK, personally, I recommend you to specify event name instead of generating
> >>> long event name in this case. But I understand sometimes this kind of feature
> >>> is good for someone.
> >>
> >> Sometimes, users try to add probe for long symbol and returns error, but there
> >> have no clue for proceeding.
> >
> > OK, no warning messsage is not good.
> > It should warn them to recommend adding it with their own event name too.
>
> Okay, will do this in next spin.
>
> >> Either we automatically generate a hashed name, or a guidance in the failure
> >> log for setting event name would be helpful. If you have concern for hashed
> >> name, maybe we can refine the log to give info for setting event name?
> >
> > Yeah, I think this long event name is not useful for user to type.
>
> Agreed.
>
> >>> BTW, I would like to confirm. Can't we demangle the symbol name and parse it?
> >>
> >> I did test for C++ demangle symbols with the command:
> >>
> >> perf probe -x /mnt/test_cpp_mangle -F --demangle
> >>
> >> The command doesn't work as I cannot see it output correctly for demangled
> >> symbols. But I don't look into details why this does not work at my side.
> >
> > Oops, that is another issue to be solved.
>
> After install libiberty, then I can see the tool can show demangled symbols.
> However, I found another issue for probing demangle symbol, please see details
> in below patch and let me know if makes sense.
Yeah, I think that will fixes a mangled symbol problem.
But I have one comment below.
>
> ---8<---
>
> From 3b09a6f89c7e383c6b1d2b7e6bd80c6bfa658d5b Mon Sep 17 00:00:00 2001
> From: Leo Yan <leo.yan@arm.com>
> Date: Fri, 11 Oct 2024 07:58:08 +0000
> Subject: [PATCH] perf probe: Correct demangled symbols in C++ program
>
> An issue can be observed when probe C++ demangled symbol with steps:
>
> # nm test_cpp_mangle | grep print_data
> 0000000000000c94 t _GLOBAL__sub_I__Z10print_datai
> 0000000000000afc T _Z10print_datai
> 0000000000000b38 T _Z10print_dataR5Point
>
> # ./perf probe -x /home/niayan01/test_cpp_mangle -F --demangle
> ...
> print_data(Point&)
> print_data(int)
> ...
>
> # ./perf --debug verbose=3 probe -x test_cpp_mangle --add "test=print_data(int)"
> probe-definition(0): test=print_data(int)
> symbol:print_data(int) file:(null) line:0 offset:0 return:0 lazy:(null)
> 0 arguments
> Open Debuginfo file: /home/niayan01/test_cpp_mangle
> Try to find probe point from debuginfo.
> Symbol print_data(int) address found : afc
> Matched function: print_data [2ccf]
> Probe point found: print_data+0
> Found 1 probe_trace_events.
> Opening /sys/kernel/tracing//uprobe_events write=1
> Opening /sys/kernel/tracing//README write=0
> Writing event: p:probe_test_cpp_mangle/test /home/niayan01/test_cpp_mangle:0xb38
> ...
>
> When tried to probe symbol "print_data(int)", the log show:
>
> Symbol print_data(int) address found : afc
>
> The found address is 0xafc - which is right if we connect with the
> output result from nm. Afterwards when write event, the command uses
> offset 0xb38 in the last log, which is a wrong address.
>
> The dwarf_diename() gets a common function name, in above case, it
> returns string "print_data". As a result, the tool parses the offset
> based on the common name. This leads to probe at the wrong symbol
> "print_data(Point&)".
>
> To fix the issue, use the die_get_linkage_name() function to retrieve
> the distinct linkage name - this is the mangled name for the C++ case.
> Based on this unique name, the tool can get a correct offset for
> probing. Based on DWARF doc, it is possible the linkage name is missed
> in the DIE, it rolls back to use dwarf_diename().
Can you add the result after applying this patch here?
Thank you,
>
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
> tools/perf/util/probe-finder.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 630e16c54ed5..28a14005fc1f 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -1583,8 +1583,21 @@ int debuginfo__find_probe_point(struct debuginfo *dbg, u64 addr,
>
> /* Find a corresponding function (name, baseline and baseaddr) */
> if (die_find_realfunc(&cudie, (Dwarf_Addr)addr, &spdie)) {
> - /* Get function entry information */
> - func = basefunc = dwarf_diename(&spdie);
> + /*
> + * Get function entry information.
> + *
> + * As described in the document DWARF Debugging Information
> + * Format Version 5, section 2.22 Linkage Names, "mangled names,
> + * are used in various ways, ... to distinguish multiple
> + * entities that have the same name".
> + *
> + * Firstly try to get distinct linkage name, if fail then
> + * rollback to get associated name in DIE.
> + */
> + func = basefunc = die_get_linkage_name(&spdie);
> + if (!func)
> + func = basefunc = dwarf_diename(&spdie);
> +
> if (!func ||
> die_entrypc(&spdie, &baseaddr) != 0 ||
> dwarf_decl_line(&spdie, &baseline) != 0) {
> --
> 2.25.1
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
On 10/12/24 06:30, Masami Hiramatsu (Google) wrote:
[...]
>> ---8<---
>>
>> From 3b09a6f89c7e383c6b1d2b7e6bd80c6bfa658d5b Mon Sep 17 00:00:00 2001
>> From: Leo Yan <leo.yan@arm.com>
>> Date: Fri, 11 Oct 2024 07:58:08 +0000
>> Subject: [PATCH] perf probe: Correct demangled symbols in C++ program
>>
>> An issue can be observed when probe C++ demangled symbol with steps:
>>
>> # nm test_cpp_mangle | grep print_data
>> 0000000000000c94 t _GLOBAL__sub_I__Z10print_datai
>> 0000000000000afc T _Z10print_datai
>> 0000000000000b38 T _Z10print_dataR5Point
>>
>> # ./perf probe -x /home/niayan01/test_cpp_mangle -F --demangle
>> ...
>> print_data(Point&)
>> print_data(int)
>> ...
>>
>> # ./perf --debug verbose=3 probe -x test_cpp_mangle --add "test=print_data(int)"
>> probe-definition(0): test=print_data(int)
>> symbol:print_data(int) file:(null) line:0 offset:0 return:0 lazy:(null)
>> 0 arguments
>> Open Debuginfo file: /home/niayan01/test_cpp_mangle
>> Try to find probe point from debuginfo.
>> Symbol print_data(int) address found : afc
>> Matched function: print_data [2ccf]
>> Probe point found: print_data+0
>> Found 1 probe_trace_events.
>> Opening /sys/kernel/tracing//uprobe_events write=1
>> Opening /sys/kernel/tracing//README write=0
>> Writing event: p:probe_test_cpp_mangle/test /home/niayan01/test_cpp_mangle:0xb38
>> ...
>>
>> When tried to probe symbol "print_data(int)", the log show:
>>
>> Symbol print_data(int) address found : afc
>>
>> The found address is 0xafc - which is right if we connect with the
>> output result from nm. Afterwards when write event, the command uses
>> offset 0xb38 in the last log, which is a wrong address.
>>
>> The dwarf_diename() gets a common function name, in above case, it
>> returns string "print_data". As a result, the tool parses the offset
>> based on the common name. This leads to probe at the wrong symbol
>> "print_data(Point&)".
>>
>> To fix the issue, use the die_get_linkage_name() function to retrieve
>> the distinct linkage name - this is the mangled name for the C++ case.
>> Based on this unique name, the tool can get a correct offset for
>> probing. Based on DWARF doc, it is possible the linkage name is missed
>> in the DIE, it rolls back to use dwarf_diename().
>
> Can you add the result after applying this patch here?
Sure. The result with this patch is:
# perf --debug verbose=3 probe -x test_cpp_mangle --add "test=print_data(int)"
probe-definition(0): test=print_data(int)
symbol:print_data(int) file:(null) line:0 offset:0 return:0 lazy:(null)
0 arguments
Open Debuginfo file: /home/niayan01/test_cpp_mangle
Try to find probe point from debuginfo.
Symbol print_data(int) address found : afc
Matched function: print_data [2d06]
Probe point found: print_data+0
Found 1 probe_trace_events.
Opening /sys/kernel/tracing//uprobe_events write=1
Opening /sys/kernel/tracing//README write=0
Writing event: p:probe_test_cpp_mangle/test /home/niayan01/test_cpp_mangle:0xafc
Added new event:
probe_test_cpp_mangle:test (on print_data(int) in /home/niayan01/test_cpp_mangle)
You can now use it in all perf tools, such as:
perf record -e probe_test_cpp_mangle:test -aR sleep 1
# perf --debug verbose=3 probe -x test_cpp_mangle --add "test2=print_data(Point&)"
probe-definition(0): test2=print_data(Point&)
symbol:print_data(Point&) file:(null) line:0 offset:0 return:0 lazy:(null)
0 arguments
Open Debuginfo file: /home/niayan01/test_cpp_mangle
Try to find probe point from debuginfo.
Symbol print_data(Point&) address found : b38
Matched function: print_data [2ccf]
Probe point found: print_data+0
Found 1 probe_trace_events.
Opening /sys/kernel/tracing//uprobe_events write=1
Parsing probe_events: p:probe_test_cpp_mangle/test /home/niayan01/test_cpp_mangle:0x0000000000000afc
Group:probe_test_cpp_mangle Event:test probe:p
Opening /sys/kernel/tracing//README write=0
Writing event: p:probe_test_cpp_mangle/test2 /home/niayan01/test_cpp_mangle:0xb38
Added new event:
probe_test_cpp_mangle:test2 (on print_data(Point&) in /home/niayan01/test_cpp_mangle)
You can now use it in all perf tools, such as:
perf record -e probe_test_cpp_mangle:test2 -aR sleep 1
I have sent out a formal patch (with fixed tag):
https://lore.kernel.org/linux-perf-users/20241012141432.877894-1-leo.yan@arm.com/T/#u
Thanks for review!
Leo
© 2016 - 2024 Red Hat, Inc.