Re: [PATCH 4/4] perf annotate: More exactly grep -v of the objdump command

From: Taeung Song
Date: Tue Mar 21 2017 - 12:23:11 EST


Thank you for your review :)

On 03/21/2017 11:37 PM, Arnaldo Carvalho de Melo wrote:
Em Mon, Mar 20, 2017 at 11:56:57AM +0900, Taeung Song escreveu:
grep -v "file name" in the objdump command
cause a side effect eliminating filename:linenr of
output of 'objdump -l' if the object file name and
source file name are about the same so fix it.

The objdump command in symbol__disassemble() can be as below

$ objdump -l -d -S -C /home/taeung/hello --start-address=...

/home/taeung/hello: file format elf64-x86-64

Disassembly of section .text:

0000000000400526 <main>:
main():
/home/taeung/hello.c:4

void main()
{
400526: 55 push %rbp
400527: 48 89 e5 mov %rsp,%rbp
/home/taeung/hello.c:5
...

But currently it use grep -v "file name" e.g. "/home/taeung/hello"
in the objdump command to remove the first line containing file name
and file format such as,

Before:
$ objdump -l -d -S -C /home/taeung/hello | grep /home/taeung/hello

But it cause a side effect removing filename:linenr
because the object file and source file has same name
e.g. "/home/taueng/hello", "/home/taeung/hello.c"

So more exactly grep -v as below
to correctly remove the one line
e.g. "/home/taeung/hello: file format elf64-x86-64"

After:
$ objdump -l -d -S -C /home/taeung/hello | grep /home/taeung/hello:

What environment is this? I tried to reproduce this here but got
different results, and I had to figure out the 'perf record' you used,
you forgot to mention it, i.e. 'perf record ./hello' doesn't work, one
has to use 'perf record /full/path/to/hello', anyway, with my

Sorry for my insufficient commit message.

$ objdump -v
GNU objdump (GNU Binutils) 2.28.51.20170309
...

$ pwd
/home/taeung

$ perf record ./hello
hello, world
^C[ perf record: Woken up 3 times to write data ]
[ perf record: Captured and wrote 0.676 MB perf.data (17380 samples) ]

And, when running 'perf annotate',
I internally checked the full objdump command line with gdb and printf.
Here it is,

objdump --start-address=0x0000000000400526 --stop-address=0x0000000000400567 -l -d --no-show-raw -S -C /home/taeung/hello 2>/dev/null|grep -v /home/taeung/hello|expand

If just running the command, the output is like below.

$ objdump --start-address=0x0000000000400526 --stop-address=0x0000000000400567 -l -d --no-show-raw -S -C /home/taeung/hello 2>/dev/null|grep -v /home/taeung/hello|expand|head -16



Disassembly of section .text:

0000000000400526 <main>:
main():
#include <stdio.h>

void main()
400526: push %rbp
400527: mov %rsp,%rbp
40052a: sub $0x10,%rsp
{
unsigned long long i;

As you can see, grep -v eliminate "filename:linenr"
such as /home/taeung/hello.c:3 and etc

But if running the command line without grep -v as below,
we can see "filename:linenr".

$ objdump --start-address=0x0000000000400526 --stop-address=0x0000000000400567 -l -d --no-show-raw -S -C /home/taeung/hello 2>/dev/null|expand|head -16

/home/taeung/hello: file format elf64-x86-64


Disassembly of section .text:

0000000000400526 <main>:
main():
/home/taeung/hello.c:3
#include <stdio.h>

void main()
400526: push %rbp
400527: mov %rsp,%rbp
40052a: sub $0x10,%rsp
/home/taeung/hello.c:6

So append ":" into grep -v as below.
(it is contents of my patch)

$ objdump --start-address=0x0000000000400526 --stop-address=0x0000000000400567 -l -d --no-show-raw -S -C /home/taeung/hello 2>/dev/null|grep -v /home/taeung/hello:|expand|head -16



Disassembly of section .text:

0000000000400526 <main>:
main():
/home/taeung/hello.c:3
#include <stdio.h>

void main()
400526: push %rbp
400527: mov %rsp,%rbp
40052a: sub $0x10,%rsp
/home/taeung/hello.c:6
{

I'll change the commit message to be more clearly..

environemnt, which is:

[root@jouet ~]# objdump -v
GNU objdump version 2.26.1-1.fc25

I end up getting:

Sorted summary for file /home/acme/c/hello
----------------------------------------------

52.67 hello.c:7
47.32 hello.c:8
Percent | Source code & Disassembly of hello for cycles:u (83555 samples)
-------------------------------------------------------------------------------
: 00000000004004f6 <main>:
: main():
: #include <stdio.h>
:
: int main(void)
: {
0.00 : 4004f6: push %rbp
0.00 : 4004f7: mov %rsp,%rbp
0.00 : 4004fa: sub $0x10,%rsp
: unsigned long long i;
: printf("hello, world\n");
0.00 : 4004fe: mov $0x4005d0,%edi
0.00 : 400503: callq 4003f0 <puts@plt>
: while (i < (unsigned long long)-1) {
0.00 : 400508: jmp 40052d <main+0x37>
: i *= 171;
hello.c:8 8.66 : 40050a: mov -0x8(%rbp),%rdx
0.01 : 40050e: mov %rdx,%rax
0.00 : 400511: shl $0x3,%rax
hello.c:8 0.84 : 400515: add %rdx,%rax
hello.c:8 8.60 : 400518: add %rax,%rax
hello.c:8 3.46 : 40051b: add %rdx,%rax
hello.c:8 8.57 : 40051e: lea 0x0(,%rax,8),%rdx
8.65 : 400526: add %rdx,%rax
8.56 : 400529: mov %rax,-0x8(%rbp)
<SNIP>

Which looks ok, i.e. since this version of objdump puts just the basename (hello.c),
that grep doesn't nuke it.

But yeah, your patch is more robust, just fix what I described below, i.e. use
quotes in the grep -v bit, probably.

- Arnaldo

Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
Signed-off-by: Taeung Song <treeze.taeung@xxxxxxxxx>
---
tools/perf/util/annotate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 63130ec..e49eb7e 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1443,7 +1443,7 @@ int symbol__disassemble(struct symbol *sym, struct map *map, const char *arch_na
snprintf(command, sizeof(command),
"%s %s%s --start-address=0x%016" PRIx64
" --stop-address=0x%016" PRIx64
- " -l -d %s %s -C %s 2>/dev/null|grep -v %s|expand",
+ " -l -d %s %s -C %s 2>/dev/null|grep -v %s: |expand",


Are you sure this works as you expect? I.e. both lines will have '%s:'

Yes, if we do, we can exactly the only below line without the side effect IMHO.

"/home/taeung/hello: file format elf64-x86-64"

that space just before the '|' will not be considered by grep


IIUC,
Just append ":" into grep -v leaving out " " space before the '|' ?
as below

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 273f21f..4d325cd 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1435,7 +1435,7 @@ int symbol__disassemble(struct symbol *sym, struct map *map, const char *arch_na
snprintf(command, sizeof(command),
"%s %s%s --start-address=0x%016" PRIx64
" --stop-address=0x%016" PRIx64
- " -l -d %s %s -C %s 2>/dev/null|grep -v %s|expand",
+ " -l -d %s %s -C %s 2>/dev/null|grep -v %s:|expand",
objdump_path ? objdump_path : "objdump",
disassembler_style ? "-M " : "",
disassembler_style ? disassembler_style : "",


Thanks,
Taeung