[PATCH] perf tests: improve reading of objdump output

[PATCH] perf tests: improve reading of objdump output

[Jan Stancek]

There are couple of situations, where objdump output doesn't
match "code reading" test expectations:

1. gaps in output, objdump skips zero blocks by default

  ffffffff816704fe <sysret_check+0x4b>:
  ffffffff816704fe:	7b 34                	jnp    ffffffff81670534  <sysret_signal+0x1c>
        ...
  ffffffff81670501 <sysret_careful>:
  ffffffff81670501:	0f ba e2 03          	bt     $0x3,%edx
  ffffffff81670505:	73 11                	jae    ffffffff81670518  <sysret_signal>

This patch adds "-z" to objdump parameters.

2. bytes can be repeated in objdump output, test reads it all sequentially
   assuming each address is represented in output only once.

  ffffffff8164efb3 <retint_swapgs+0x9>:
  ffffffff8164efb3:       c1 5d 00 eb             rcrl   $0xeb,0x0(%rbp)
  ffffffff8164efb7:       00 4c 8b 5c             add    %cl,0x5c(%rbx,%rcx,4)

  ffffffff8164efb8 <restore_c_regs_and_iret>:
  ffffffff8164efb8:       4c 8b 5c 24 30          mov    0x30(%rsp),%r11
  ffffffff8164efbd:       4c 8b 54 24 38          mov    0x38(%rsp),%r10

This patch stores objdump output to buffer according to address
on each line.

3. objdump output can span across multiple sections

  Disassembly of section .text:
    0000000000000008 <crc32c+0x8>:
       8:	48 89 e5             	mov    %rsp,%rbp
       b:	53                   	push   %rbx
       c:	8b 01                	mov    (%rcx),%eax
    <snip>
      6b:	90                   	nop

  Disassembly of section .init.text:
    0000000000000008 <init_module+0x8>:
       8:	00 00                	add    %al,(%rax)
       a:	00 00                	add    %al,(%rax)
       c:	48 89 e5

This patch aborts further reading if address starts going backwards,
assuming we crossed sections.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 tools/perf/tests/code-reading.c | 74 +++++++++++++++++++++++++++++++++--------
 1 file changed, 60 insertions(+), 14 deletions(-)

diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 39c784a100a9..2d21183bd661 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -33,20 +33,20 @@ static unsigned int hex(char c)
 	return c - 'A' + 10;
 }
 
-static void read_objdump_line(const char *line, size_t line_len, void **buf,
-			      size_t *len)
+static size_t read_objdump_line(const char *line, size_t line_len, void *buf,
+			      size_t len)
 {
 	const char *p;
-	size_t i;
+	size_t i, j = 0;
 
 	/* Skip to a colon */
 	p = strchr(line, ':');
 	if (!p)
-		return;
+		return 0;
 	i = p + 1 - line;
 
 	/* Read bytes */
-	while (*len) {
+	while (j < len) {
 		char c1, c2;
 
 		/* Skip spaces */
@@ -65,20 +65,26 @@ static void read_objdump_line(const char *line, size_t line_len, void **buf,
 		if (i < line_len && line[i] && !isspace(line[i]))
 			break;
 		/* Store byte */
-		*(unsigned char *)*buf = (hex(c1) << 4) | hex(c2);
-		*buf += 1;
-		*len -= 1;
+		*(unsigned char *)buf = (hex(c1) << 4) | hex(c2);
+		buf += 1;
+		j++;
 	}
+	/* return number of successfully read bytes */
+	return j;
 }
 
-static int read_objdump_output(FILE *f, void **buf, size_t *len)
+static int read_objdump_output(FILE *f, void *buf, size_t *len, u64 start_addr)
 {
 	char *line = NULL;
-	size_t line_len;
+	size_t line_len, off_last = 0;
 	ssize_t ret;
 	int err = 0;
+	u64 addr, last_addr = start_addr;
+
+	while (off_last < *len) {
+		size_t off, read_bytes, written_bytes;
+		unsigned char tmp[BUFSZ];
 
-	while (1) {
 		ret = getline(&line, &line_len, f);
 		if (feof(f))
 			break;
@@ -87,9 +93,33 @@ static int read_objdump_output(FILE *f, void **buf, size_t *len)
 			err = -1;
 			break;
 		}
-		read_objdump_line(line, ret, buf, len);
+
+		/* read objdump data into temporary buffer */
+		read_bytes = read_objdump_line(line, ret, tmp, sizeof(tmp));
+		if (!read_bytes)
+			continue;
+
+		if (sscanf(line, "%"PRIx64, &addr) != 1)
+			continue;
+		if (addr < last_addr) {
+			pr_debug("addr going backwards, read beyond section?\n");
+			break;
+		}
+		last_addr = addr;
+
+		/* copy it from temporary buffer to 'buf' according
+		 * to address on current objdump line */
+		off = addr - start_addr;
+		if (off >= *len)
+			break;
+		written_bytes = MIN(read_bytes, *len - off);
+		memcpy(buf + off, tmp, written_bytes);
+		off_last = off + written_bytes;
 	}
 
+	/* len returns number of bytes that could not be read */
+	*len -= off_last;
+
 	free(line);
 
 	return err;
@@ -103,7 +133,7 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf,
 	FILE *f;
 	int ret;
 
-	fmt = "%s -d --start-address=0x%"PRIx64" --stop-address=0x%"PRIx64" %s";
+	fmt = "%s -z -d --start-address=0x%"PRIx64" --stop-address=0x%"PRIx64" %s";
 	ret = snprintf(cmd, sizeof(cmd), fmt, "objdump", addr, addr + len,
 		       filename);
 	if (ret <= 0 || (size_t)ret >= sizeof(cmd))
@@ -120,7 +150,7 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf,
 		return -1;
 	}
 
-	ret = read_objdump_output(f, &buf, &len);
+	ret = read_objdump_output(f, buf, &len, addr);
 	if (len) {
 		pr_debug("objdump read too few bytes\n");
 		if (!ret)
@@ -132,6 +162,18 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf,
 	return ret;
 }
 
+static void dump_buf(unsigned char *buf, size_t len)
+{
+	size_t i;
+
+	for (i = 0; i < len; i++) {
+		pr_debug("0x%02x ", buf[i]);
+		if (i % 16 == 15)
+			pr_debug("\n");
+	}
+	pr_debug("\n");
+}
+
 static int read_object_code(u64 addr, size_t len, u8 cpumode,
 			    struct thread *thread, struct state *state)
 {
@@ -234,6 +276,10 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
 	/* The results should be identical */
 	if (memcmp(buf1, buf2, len)) {
 		pr_debug("Bytes read differ from those read by objdump\n");
+		pr_debug("buf1 (dso):\n");
+		dump_buf(buf1, len);
+		pr_debug("buf2 (objdump):\n");
+		dump_buf(buf2, len);
 		return -1;
 	}
 	pr_debug("Bytes read match those read by objdump\n");
-- 
1.8.3.1

Re: [PATCH] perf tests: improve reading of objdump output

[Peter Zijlstra]

On Mon, Aug 31, 2015 at 09:18:33AM +0200, Jan Stancek wrote:
> There are couple of situations, where objdump output doesn't
> match "code reading" test expectations:
> 
> 1. gaps in output, objdump skips zero blocks by default
> 
>   ffffffff816704fe <sysret_check+0x4b>:
>   ffffffff816704fe:	7b 34                	jnp    ffffffff81670534  <sysret_signal+0x1c>
>         ...
>   ffffffff81670501 <sysret_careful>:
>   ffffffff81670501:	0f ba e2 03          	bt     $0x3,%edx
>   ffffffff81670505:	73 11                	jae    ffffffff81670518  <sysret_signal>
> 
> This patch adds "-z" to objdump parameters.
> 
> 2. bytes can be repeated in objdump output, test reads it all sequentially
>    assuming each address is represented in output only once.
> 
>   ffffffff8164efb3 <retint_swapgs+0x9>:
>   ffffffff8164efb3:       c1 5d 00 eb             rcrl   $0xeb,0x0(%rbp)
>   ffffffff8164efb7:       00 4c 8b 5c             add    %cl,0x5c(%rbx,%rcx,4)
> 
>   ffffffff8164efb8 <restore_c_regs_and_iret>:
>   ffffffff8164efb8:       4c 8b 5c 24 30          mov    0x30(%rsp),%r11
>   ffffffff8164efbd:       4c 8b 54 24 38          mov    0x38(%rsp),%r10
> 
> This patch stores objdump output to buffer according to address
> on each line.
> 
> 3. objdump output can span across multiple sections
> 
>   Disassembly of section .text:
>     0000000000000008 <crc32c+0x8>:
>        8:	48 89 e5             	mov    %rsp,%rbp
>        b:	53                   	push   %rbx
>        c:	8b 01                	mov    (%rcx),%eax
>     <snip>
>       6b:	90                   	nop
> 
>   Disassembly of section .init.text:
>     0000000000000008 <init_module+0x8>:
>        8:	00 00                	add    %al,(%rax)
>        a:	00 00                	add    %al,(%rax)
>        c:	48 89 e5
> 
> This patch aborts further reading if address starts going backwards,
> assuming we crossed sections.

I would have expected 3 patches, each patch doing exactly one of the
above.