From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: "Martin Liška" <mliska@suse.cz>
Cc: linux-kernel@vger.kernel.org, Jiri Slaby <jslaby@suse.cz>,
linux-perf-users@vger.kernel.org, Jiri Olsa <jolsa@kernel.org>,
Namhyung Kim <namhyung@kernel.org>
Subject: Re: [PATCH] Fix jump parsing for C++ code.
Date: Thu, 11 Feb 2021 09:59:06 -0300 [thread overview]
Message-ID: <20210211125906.GC1131885@kernel.org> (raw)
In-Reply-To: <13e1a405-edf9-e4c2-4327-a9b454353730@suse.cz>
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
next prev parent reply other threads:[~2021-02-11 13:03 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-11 12:37 [PATCH] Fix jump parsing for C++ code Martin Liška
2021-02-11 12:59 ` Arnaldo Carvalho de Melo [this message]
2021-02-11 17:16 ` Arnaldo Carvalho de Melo
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=20210211125906.GC1131885@kernel.org \
--to=acme@kernel.org \
--cc=jolsa@kernel.org \
--cc=jslaby@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mliska@suse.cz \
--cc=namhyung@kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).