* [PATCH] Fix jump parsing for C++ code.
@ 2021-02-11 12:37 Martin Liška
2021-02-11 12:59 ` Arnaldo Carvalho de Melo
2021-02-11 17:16 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 3+ messages in thread
From: Martin Liška @ 2021-02-11 12:37 UTC (permalink / raw)
To: linux-kernel; +Cc: Jiri Slaby, linux-perf-users
Considering the following testcase:
int
foo(int a, int b)
{
for (unsigned i = 0; i < 1000000000; i++)
a += b;
return a;
}
int main()
{
foo (3, 4);
return 0;
}
perf annotate displays:
86.52 │40055e: → ja 40056c <foo(int, int)+0x26>
13.37 │400560: mov -0x18(%rbp),%eax
│400563: add %eax,-0x14(%rbp)
│400566: addl $0x1,-0x4(%rbp)
0.11 │40056a: → jmp 400557 <foo(int, int)+0x11>
│40056c: mov -0x14(%rbp),%eax
│40056f: pop %rbp
and the 'ja 40056c' does not link to the location in the function.
It's caused by fact that comma is wrongly parsed, it's part
of function signature.
With my patch I see:
86.52 │ ┌──ja 26
13.37 │ │ mov -0x18(%rbp),%eax
│ │ add %eax,-0x14(%rbp)
│ │ addl $0x1,-0x4(%rbp)
0.11 │ │↑ jmp 11
│26:└─→mov -0x14(%rbp),%eax
and 'o' output prints:
86.52 │4005┌── ↓ ja 40056c <foo(int, int)+0x26>
13.37 │4005│0: mov -0x18(%rbp),%eax
│4005│3: add %eax,-0x14(%rbp)
│4005│6: addl $0x1,-0x4(%rbp)
0.11 │4005│a: ↑ jmp 400557 <foo(int, int)+0x11>
│4005└─→ mov -0x14(%rbp),%eax
On the contrary, compiling the very same file with gcc -x c, the parsing
is fine because function arguments are not displyed:
jmp 400543 <foo+0x1d>
Signed-off-by: Martin Liška <mliska@suse.cz>
---
tools/perf/util/annotate.c | 6 ++++++
tools/perf/util/annotate.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index ce8c07bc8c56..e3eae646be3e 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -321,11 +321,16 @@ bool ins__is_call(const struct ins *ins)
/*
* Prevents from matching commas in the comment section, e.g.:
* ffff200008446e70: b.cs ffff2000084470f4 <generic_exec_single+0x314> // b.hs, b.nlast
+ *
+ * and skip comma as part of function arguments, e.g.:
+ * 1d8b4ac <linemap_lookup(line_maps const*, unsigned int)+0xcc>
*/
static inline const char *validate_comma(const char *c, struct ins_operands *ops)
{
if (ops->raw_comment && c > ops->raw_comment)
return NULL;
+ else if (ops->raw_func_start && c > ops->raw_func_start)
+ return NULL;
return c;
}
@@ -341,6 +346,7 @@ static int jump__parse(struct arch *arch, struct ins_operands *ops, struct map_s
u64 start, end;
ops->raw_comment = strchr(ops->raw, arch->objdump.comment_char);
+ ops->raw_func_start = strchr(ops->raw, '<');
c = validate_comma(c, ops);
/*
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 0a0cd4f32175..096cdaf21b01 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -32,6 +32,7 @@ struct ins {
struct ins_operands {
char *raw;
char *raw_comment;
+ char *raw_func_start;
struct {
char *raw;
char *name;
--
2.30.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix jump parsing for C++ code.
2021-02-11 12:37 [PATCH] Fix jump parsing for C++ code Martin Liška
@ 2021-02-11 12:59 ` Arnaldo Carvalho de Melo
2021-02-11 17:16 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-02-11 12:59 UTC (permalink / raw)
To: Martin Liška
Cc: linux-kernel, Jiri Slaby, linux-perf-users, Jiri Olsa, Namhyung Kim
Em Thu, Feb 11, 2021 at 01:37:55PM +0100, Martin Liška escreveu:
> Considering the following testcase:
Cool, applied, next time please use a Summary like:
Subject: Re: [PATCH] perf annotate: Fix jump parsing for C++ code.
So that it stands out more clearly in my inbox, this time I saw your
name as a previous perf contributor and figured it should be for perf
:-)
Thanks!
- Arnaldo
> int
> foo(int a, int b)
> {
> for (unsigned i = 0; i < 1000000000; i++)
> a += b;
> return a;
> }
>
> int main()
> {
> foo (3, 4);
> return 0;
> }
>
> perf annotate displays:
> 86.52 │40055e: → ja 40056c <foo(int, int)+0x26>
> 13.37 │400560: mov -0x18(%rbp),%eax
> │400563: add %eax,-0x14(%rbp)
> │400566: addl $0x1,-0x4(%rbp)
> 0.11 │40056a: → jmp 400557 <foo(int, int)+0x11>
> │40056c: mov -0x14(%rbp),%eax
> │40056f: pop %rbp
>
> and the 'ja 40056c' does not link to the location in the function.
> It's caused by fact that comma is wrongly parsed, it's part
> of function signature.
>
> With my patch I see:
>
> 86.52 │ ┌──ja 26
> 13.37 │ │ mov -0x18(%rbp),%eax
> │ │ add %eax,-0x14(%rbp)
> │ │ addl $0x1,-0x4(%rbp)
> 0.11 │ │↑ jmp 11
> │26:└─→mov -0x14(%rbp),%eax
>
> and 'o' output prints:
> 86.52 │4005┌── ↓ ja 40056c <foo(int, int)+0x26>
> 13.37 │4005│0: mov -0x18(%rbp),%eax
> │4005│3: add %eax,-0x14(%rbp)
> │4005│6: addl $0x1,-0x4(%rbp)
> 0.11 │4005│a: ↑ jmp 400557 <foo(int, int)+0x11>
> │4005└─→ mov -0x14(%rbp),%eax
>
> On the contrary, compiling the very same file with gcc -x c, the parsing
> is fine because function arguments are not displyed:
>
> jmp 400543 <foo+0x1d>
>
> Signed-off-by: Martin Liška <mliska@suse.cz>
> ---
> tools/perf/util/annotate.c | 6 ++++++
> tools/perf/util/annotate.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index ce8c07bc8c56..e3eae646be3e 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -321,11 +321,16 @@ bool ins__is_call(const struct ins *ins)
> /*
> * Prevents from matching commas in the comment section, e.g.:
> * ffff200008446e70: b.cs ffff2000084470f4 <generic_exec_single+0x314> // b.hs, b.nlast
> + *
> + * and skip comma as part of function arguments, e.g.:
> + * 1d8b4ac <linemap_lookup(line_maps const*, unsigned int)+0xcc>
> */
> static inline const char *validate_comma(const char *c, struct ins_operands *ops)
> {
> if (ops->raw_comment && c > ops->raw_comment)
> return NULL;
> + else if (ops->raw_func_start && c > ops->raw_func_start)
> + return NULL;
> return c;
> }
> @@ -341,6 +346,7 @@ static int jump__parse(struct arch *arch, struct ins_operands *ops, struct map_s
> u64 start, end;
> ops->raw_comment = strchr(ops->raw, arch->objdump.comment_char);
> + ops->raw_func_start = strchr(ops->raw, '<');
> c = validate_comma(c, ops);
> /*
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 0a0cd4f32175..096cdaf21b01 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -32,6 +32,7 @@ struct ins {
> struct ins_operands {
> char *raw;
> char *raw_comment;
> + char *raw_func_start;
> struct {
> char *raw;
> char *name;
> --
> 2.30.0
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix jump parsing for C++ code.
2021-02-11 12:37 [PATCH] Fix jump parsing for C++ code Martin Liška
2021-02-11 12:59 ` Arnaldo Carvalho de Melo
@ 2021-02-11 17:16 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-02-11 17:16 UTC (permalink / raw)
To: Martin Liška; +Cc: linux-kernel, Jiri Slaby, linux-perf-users
Em Thu, Feb 11, 2021 at 01:37:55PM +0100, Martin Liška escreveu:
> Considering the following testcase:
>
> int
> foo(int a, int b)
> {
> for (unsigned i = 0; i < 1000000000; i++)
> a += b;
> return a;
> }
>
> int main()
> {
> foo (3, 4);
> return 0;
> }
>
> perf annotate displays:
> 86.52 │40055e: → ja 40056c <foo(int, int)+0x26>
> 13.37 │400560: mov -0x18(%rbp),%eax
> │400563: add %eax,-0x14(%rbp)
> │400566: addl $0x1,-0x4(%rbp)
> 0.11 │40056a: → jmp 400557 <foo(int, int)+0x11>
> │40056c: mov -0x14(%rbp),%eax
> │40056f: pop %rbp
>
> and the 'ja 40056c' does not link to the location in the function.
> It's caused by fact that comma is wrongly parsed, it's part
> of function signature.
>
> With my patch I see:
>
> 86.52 │ ┌──ja 26
> 13.37 │ │ mov -0x18(%rbp),%eax
> │ │ add %eax,-0x14(%rbp)
> │ │ addl $0x1,-0x4(%rbp)
> 0.11 │ │↑ jmp 11
> │26:└─→mov -0x14(%rbp),%eax
>
> and 'o' output prints:
> 86.52 │4005┌── ↓ ja 40056c <foo(int, int)+0x26>
> 13.37 │4005│0: mov -0x18(%rbp),%eax
> │4005│3: add %eax,-0x14(%rbp)
> │4005│6: addl $0x1,-0x4(%rbp)
> 0.11 │4005│a: ↑ jmp 400557 <foo(int, int)+0x11>
> │4005└─→ mov -0x14(%rbp),%eax
So, before your patch, this is what I am seeing:
[acme@five c]$ cat cpp_args_annotate.c
int
foo(int a, int b)
{
for (unsigned i = 0; i < 1000000000; i++)
a += b;
return a;
}
int main()
{
foo (3, 4);
return 0;
}
[acme@five c]$ gcc --version |& head -1
gcc (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9)
[acme@five c]$ gcc -g cpp_args_annotate.c -o cpp_args_annotate
[acme@five c]$ perf record ./cpp_args_annotate
[ perf record: Woken up 2 times to write data ]
[ perf record: Captured and wrote 0.275 MB perf.data (7188 samples) ]
[acme@five c]$ perf annotate --stdio2 foo
Samples: 7K of event 'cycles:u', 4000 Hz, Event count (approx.): 7468429289, [percent: local period]
foo() /home/acme/c/cpp_args_annotate
Percent
0000000000401106 <foo>:
foo():
int
foo(int a, int b)
{
push %rbp
mov %rsp,%rbp
mov %edi,-0x14(%rbp)
mov %esi,-0x18(%rbp)
for (unsigned i = 0; i < 1000000000; i++)
movl $0x0,-0x4(%rbp)
↓ jmp 1d
a += b;
13.45 13: mov -0x18(%rbp),%eax
add %eax,-0x14(%rbp)
for (unsigned i = 0; i < 1000000000; i++)
addl $0x1,-0x4(%rbp)
0.09 1d: cmpl $0x3b9ac9ff,-0x4(%rbp)
86.46 ↑ jbe 13
return a;
mov -0x14(%rbp),%eax
}
pop %rbp
← retq
[acme@five c]$
Ok, now I see:
[acme@five c]$ g++ -g cpp_args_annotate.c -o cpp_args_annotate
[acme@five c]$ perf record ./cpp_args_annotate
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.268 MB perf.data (6976 samples) ]
[acme@five c]$ perf annotate --stdio2 foo
Samples: 6K of event 'cycles:u', 4000 Hz, Event count (approx.): 7380681761, [percent: local period]
foo() /home/acme/c/cpp_args_annotate
Percent
0000000000401106 <foo(int, int)>:
foo(int, int):
int
foo(int a, int b)
{
push %rbp
mov %rsp,%rbp
mov %edi,-0x14(%rbp)
mov %esi,-0x18(%rbp)
for (unsigned i = 0; i < 1000000000; i++)
movl $0x0,-0x4(%rbp)
cmpl $0x3b9ac9ff,-0x4(%rbp)
86.53 → ja 40112c <foo(int, int)+0x26>
a += b;
13.32 mov -0x18(%rbp),%eax
0.00 add %eax,-0x14(%rbp)
for (unsigned i = 0; i < 1000000000; i++)
addl $0x1,-0x4(%rbp)
0.15 → jmp 401117 <foo(int, int)+0x11>
return a;
mov -0x14(%rbp),%eax
}
pop %rbp
← retq
[acme@five c]$
Ok, continuing the test...
- Arnaldo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-02-11 17:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-11 12:37 [PATCH] Fix jump parsing for C++ code Martin Liška
2021-02-11 12:59 ` Arnaldo Carvalho de Melo
2021-02-11 17:16 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).