* [PATCH] perf tests: objdump output can contain multi byte chunks
@ 2016-01-12 10:07 Jan Stancek
2016-01-12 15:30 ` Arnaldo Carvalho de Melo
2016-08-04 9:14 ` [tip:perf/urgent] " tip-bot for Jan Stancek
0 siblings, 2 replies; 7+ messages in thread
From: Jan Stancek @ 2016-01-12 10:07 UTC (permalink / raw)
To: linux-kernel, acme, xiakaixu
Cc: jstancek, adrian.hunter, cjashfor, dsahern, fweisbec, jolsa,
namhyung, paulus, a.p.zijlstra
objdump's raw insn output can vary across architectures on number of
bytes per chunk (bpc) displayed and their endian.
code-reading test relied on reading objdump output as 1 bpc. Kaixu Xia
reported test failure on ARM64, where objdump displays 4 bpc:
70c48: f90027bf str xzr, [x29,#72]
70c4c: 91224000 add x0, x0, #0x890
70c50: f90023a0 str x0, [x29,#64]
This patch adds support to read raw insn output for any bpc length.
In case of 2+ bpc it also guesses objdump's display endian.
Signed-off-by: Jan Stancek <jstancek@redhat.com>
Reported-and-tested-by: Kaixu Xia <xiakaixu@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@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 | 100 ++++++++++++++++++++++++++++------------
1 file changed, 71 insertions(+), 29 deletions(-)
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index a767a6400c5c..0108cb22d1a2 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -33,44 +33,86 @@ static unsigned int hex(char c)
return c - 'A' + 10;
}
-static size_t read_objdump_line(const char *line, size_t line_len, void *buf,
- size_t len)
+static size_t read_objdump_chunk(const char **line, unsigned char **buf,
+ size_t *buf_len)
+{
+ size_t bytes_read = 0;
+ unsigned char *chunk_start = *buf;
+
+ /* Read bytes */
+ while (*buf_len > 0) {
+ char c1, c2;
+
+ /* Get 2 hex digits */
+ c1 = *(*line)++;
+ if (!isxdigit(c1))
+ break;
+ c2 = *(*line)++;
+ if (!isxdigit(c2))
+ break;
+
+ /* Store byte and advance buf */
+ **buf = (hex(c1) << 4) | hex(c2);
+ (*buf)++;
+ (*buf_len)--;
+ bytes_read++;
+
+ /* End of chunk? */
+ if (isspace(**line))
+ break;
+ }
+
+ /*
+ * objdump will display raw insn as LE if code endian
+ * is LE and bytes_per_chunk > 1. In that case reverse
+ * the chunk we just read.
+ *
+ * see disassemble_bytes() at binutils/objdump.c for details
+ * how objdump chooses display endian)
+ */
+ if (bytes_read > 1 && !bigendian()) {
+ unsigned char *chunk_end = chunk_start + bytes_read - 1;
+ unsigned char tmp;
+
+ while (chunk_start < chunk_end) {
+ tmp = *chunk_start;
+ *chunk_start = *chunk_end;
+ *chunk_end = tmp;
+ chunk_start++;
+ chunk_end--;
+ }
+ }
+
+ return bytes_read;
+}
+
+static size_t read_objdump_line(const char *line, unsigned char *buf,
+ size_t buf_len)
{
const char *p;
- size_t i, j = 0;
+ size_t ret, bytes_read = 0;
/* Skip to a colon */
p = strchr(line, ':');
if (!p)
return 0;
- i = p + 1 - line;
+ p++;
- /* Read bytes */
- while (j < len) {
- char c1, c2;
-
- /* Skip spaces */
- for (; i < line_len; i++) {
- if (!isspace(line[i]))
- break;
- }
- /* Get 2 hex digits */
- if (i >= line_len || !isxdigit(line[i]))
- break;
- c1 = line[i++];
- if (i >= line_len || !isxdigit(line[i]))
- break;
- c2 = line[i++];
- /* Followed by a space */
- if (i < line_len && line[i] && !isspace(line[i]))
+ /* Skip initial spaces */
+ while (*p) {
+ if (!isspace(*p))
break;
- /* Store byte */
- *(unsigned char *)buf = (hex(c1) << 4) | hex(c2);
- buf += 1;
- j++;
+ p++;
}
+
+ do {
+ ret = read_objdump_chunk(&p, &buf, &buf_len);
+ bytes_read += ret;
+ p++;
+ } while (ret > 0);
+
/* return number of successfully read bytes */
- return j;
+ return bytes_read;
}
static int read_objdump_output(FILE *f, void *buf, size_t *len, u64 start_addr)
@@ -95,7 +137,7 @@ static int read_objdump_output(FILE *f, void *buf, size_t *len, u64 start_addr)
}
/* read objdump data into temporary buffer */
- read_bytes = read_objdump_line(line, ret, tmp, sizeof(tmp));
+ read_bytes = read_objdump_line(line, tmp, sizeof(tmp));
if (!read_bytes)
continue;
@@ -152,7 +194,7 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf,
ret = read_objdump_output(f, buf, &len, addr);
if (len) {
- pr_debug("objdump read too few bytes\n");
+ pr_debug("objdump read too few bytes: %lu\n", len);
if (!ret)
ret = len;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] perf tests: objdump output can contain multi byte chunks
2016-01-12 10:07 [PATCH] perf tests: objdump output can contain multi byte chunks Jan Stancek
@ 2016-01-12 15:30 ` Arnaldo Carvalho de Melo
2016-01-13 8:22 ` Adrian Hunter
2016-08-04 9:14 ` [tip:perf/urgent] " tip-bot for Jan Stancek
1 sibling, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-01-12 15:30 UTC (permalink / raw)
To: Adrian Hunter
Cc: Jan Stancek, linux-kernel, xiakaixu, cjashfor, dsahern, fweisbec,
jolsa, acme, namhyung, paulus, a.p.zijlstra
Em Tue, Jan 12, 2016 at 11:07:44AM +0100, Jan Stancek escreveu:
> objdump's raw insn output can vary across architectures on number of
> bytes per chunk (bpc) displayed and their endian.
>
> code-reading test relied on reading objdump output as 1 bpc. Kaixu Xia
> reported test failure on ARM64, where objdump displays 4 bpc:
> 70c48: f90027bf str xzr, [x29,#72]
> 70c4c: 91224000 add x0, x0, #0x890
> 70c50: f90023a0 str x0, [x29,#64]
>
> This patch adds support to read raw insn output for any bpc length.
> In case of 2+ bpc it also guesses objdump's display endian.
Adrian, from a quick read of the patch and problem/solution description,
I think this is ok and Kaixu reports it solves the problem on ARM64, can
I have your reviewed-by/Acked-by?
Thanks,
- Arnaldo
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> Reported-and-tested-by: Kaixu Xia <xiakaixu@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Jiri Olsa <jolsa@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 | 100 ++++++++++++++++++++++++++++------------
> 1 file changed, 71 insertions(+), 29 deletions(-)
>
> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> index a767a6400c5c..0108cb22d1a2 100644
> --- a/tools/perf/tests/code-reading.c
> +++ b/tools/perf/tests/code-reading.c
> @@ -33,44 +33,86 @@ static unsigned int hex(char c)
> return c - 'A' + 10;
> }
>
> -static size_t read_objdump_line(const char *line, size_t line_len, void *buf,
> - size_t len)
> +static size_t read_objdump_chunk(const char **line, unsigned char **buf,
> + size_t *buf_len)
> +{
> + size_t bytes_read = 0;
> + unsigned char *chunk_start = *buf;
> +
> + /* Read bytes */
> + while (*buf_len > 0) {
> + char c1, c2;
> +
> + /* Get 2 hex digits */
> + c1 = *(*line)++;
> + if (!isxdigit(c1))
> + break;
> + c2 = *(*line)++;
> + if (!isxdigit(c2))
> + break;
> +
> + /* Store byte and advance buf */
> + **buf = (hex(c1) << 4) | hex(c2);
> + (*buf)++;
> + (*buf_len)--;
> + bytes_read++;
> +
> + /* End of chunk? */
> + if (isspace(**line))
> + break;
> + }
> +
> + /*
> + * objdump will display raw insn as LE if code endian
> + * is LE and bytes_per_chunk > 1. In that case reverse
> + * the chunk we just read.
> + *
> + * see disassemble_bytes() at binutils/objdump.c for details
> + * how objdump chooses display endian)
> + */
> + if (bytes_read > 1 && !bigendian()) {
> + unsigned char *chunk_end = chunk_start + bytes_read - 1;
> + unsigned char tmp;
> +
> + while (chunk_start < chunk_end) {
> + tmp = *chunk_start;
> + *chunk_start = *chunk_end;
> + *chunk_end = tmp;
> + chunk_start++;
> + chunk_end--;
> + }
> + }
> +
> + return bytes_read;
> +}
> +
> +static size_t read_objdump_line(const char *line, unsigned char *buf,
> + size_t buf_len)
> {
> const char *p;
> - size_t i, j = 0;
> + size_t ret, bytes_read = 0;
>
> /* Skip to a colon */
> p = strchr(line, ':');
> if (!p)
> return 0;
> - i = p + 1 - line;
> + p++;
>
> - /* Read bytes */
> - while (j < len) {
> - char c1, c2;
> -
> - /* Skip spaces */
> - for (; i < line_len; i++) {
> - if (!isspace(line[i]))
> - break;
> - }
> - /* Get 2 hex digits */
> - if (i >= line_len || !isxdigit(line[i]))
> - break;
> - c1 = line[i++];
> - if (i >= line_len || !isxdigit(line[i]))
> - break;
> - c2 = line[i++];
> - /* Followed by a space */
> - if (i < line_len && line[i] && !isspace(line[i]))
> + /* Skip initial spaces */
> + while (*p) {
> + if (!isspace(*p))
> break;
> - /* Store byte */
> - *(unsigned char *)buf = (hex(c1) << 4) | hex(c2);
> - buf += 1;
> - j++;
> + p++;
> }
> +
> + do {
> + ret = read_objdump_chunk(&p, &buf, &buf_len);
> + bytes_read += ret;
> + p++;
> + } while (ret > 0);
> +
> /* return number of successfully read bytes */
> - return j;
> + return bytes_read;
> }
>
> static int read_objdump_output(FILE *f, void *buf, size_t *len, u64 start_addr)
> @@ -95,7 +137,7 @@ static int read_objdump_output(FILE *f, void *buf, size_t *len, u64 start_addr)
> }
>
> /* read objdump data into temporary buffer */
> - read_bytes = read_objdump_line(line, ret, tmp, sizeof(tmp));
> + read_bytes = read_objdump_line(line, tmp, sizeof(tmp));
> if (!read_bytes)
> continue;
>
> @@ -152,7 +194,7 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf,
>
> ret = read_objdump_output(f, buf, &len, addr);
> if (len) {
> - pr_debug("objdump read too few bytes\n");
> + pr_debug("objdump read too few bytes: %lu\n", len);
> if (!ret)
> ret = len;
> }
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] perf tests: objdump output can contain multi byte chunks
2016-01-12 15:30 ` Arnaldo Carvalho de Melo
@ 2016-01-13 8:22 ` Adrian Hunter
2016-08-02 19:07 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 7+ messages in thread
From: Adrian Hunter @ 2016-01-13 8:22 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jan Stancek, linux-kernel, xiakaixu, cjashfor, dsahern, fweisbec,
jolsa, acme, namhyung, paulus, a.p.zijlstra
On 12/01/16 17:30, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jan 12, 2016 at 11:07:44AM +0100, Jan Stancek escreveu:
>> objdump's raw insn output can vary across architectures on number of
>> bytes per chunk (bpc) displayed and their endian.
>>
>> code-reading test relied on reading objdump output as 1 bpc. Kaixu Xia
>> reported test failure on ARM64, where objdump displays 4 bpc:
>> 70c48: f90027bf str xzr, [x29,#72]
>> 70c4c: 91224000 add x0, x0, #0x890
>> 70c50: f90023a0 str x0, [x29,#64]
>>
>> This patch adds support to read raw insn output for any bpc length.
>> In case of 2+ bpc it also guesses objdump's display endian.
>
> Adrian, from a quick read of the patch and problem/solution description,
> I think this is ok and Kaixu reports it solves the problem on ARM64, can
> I have your reviewed-by/Acked-by?
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>
> Thanks,
>
> - Arnaldo
>
>> Signed-off-by: Jan Stancek <jstancek@redhat.com>
>> Reported-and-tested-by: Kaixu Xia <xiakaixu@huawei.com>
>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>> Cc: Adrian Hunter <adrian.hunter@intel.com>
>> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
>> Cc: David Ahern <dsahern@gmail.com>
>> Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> Cc: Jiri Olsa <jolsa@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 | 100 ++++++++++++++++++++++++++++------------
>> 1 file changed, 71 insertions(+), 29 deletions(-)
>>
>> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
>> index a767a6400c5c..0108cb22d1a2 100644
>> --- a/tools/perf/tests/code-reading.c
>> +++ b/tools/perf/tests/code-reading.c
>> @@ -33,44 +33,86 @@ static unsigned int hex(char c)
>> return c - 'A' + 10;
>> }
>>
>> -static size_t read_objdump_line(const char *line, size_t line_len, void *buf,
>> - size_t len)
>> +static size_t read_objdump_chunk(const char **line, unsigned char **buf,
>> + size_t *buf_len)
>> +{
>> + size_t bytes_read = 0;
>> + unsigned char *chunk_start = *buf;
>> +
>> + /* Read bytes */
>> + while (*buf_len > 0) {
>> + char c1, c2;
>> +
>> + /* Get 2 hex digits */
>> + c1 = *(*line)++;
>> + if (!isxdigit(c1))
>> + break;
>> + c2 = *(*line)++;
>> + if (!isxdigit(c2))
>> + break;
>> +
>> + /* Store byte and advance buf */
>> + **buf = (hex(c1) << 4) | hex(c2);
>> + (*buf)++;
>> + (*buf_len)--;
>> + bytes_read++;
>> +
>> + /* End of chunk? */
>> + if (isspace(**line))
>> + break;
>> + }
>> +
>> + /*
>> + * objdump will display raw insn as LE if code endian
>> + * is LE and bytes_per_chunk > 1. In that case reverse
>> + * the chunk we just read.
>> + *
>> + * see disassemble_bytes() at binutils/objdump.c for details
>> + * how objdump chooses display endian)
>> + */
>> + if (bytes_read > 1 && !bigendian()) {
>> + unsigned char *chunk_end = chunk_start + bytes_read - 1;
>> + unsigned char tmp;
>> +
>> + while (chunk_start < chunk_end) {
>> + tmp = *chunk_start;
>> + *chunk_start = *chunk_end;
>> + *chunk_end = tmp;
>> + chunk_start++;
>> + chunk_end--;
>> + }
>> + }
>> +
>> + return bytes_read;
>> +}
>> +
>> +static size_t read_objdump_line(const char *line, unsigned char *buf,
>> + size_t buf_len)
>> {
>> const char *p;
>> - size_t i, j = 0;
>> + size_t ret, bytes_read = 0;
>>
>> /* Skip to a colon */
>> p = strchr(line, ':');
>> if (!p)
>> return 0;
>> - i = p + 1 - line;
>> + p++;
>>
>> - /* Read bytes */
>> - while (j < len) {
>> - char c1, c2;
>> -
>> - /* Skip spaces */
>> - for (; i < line_len; i++) {
>> - if (!isspace(line[i]))
>> - break;
>> - }
>> - /* Get 2 hex digits */
>> - if (i >= line_len || !isxdigit(line[i]))
>> - break;
>> - c1 = line[i++];
>> - if (i >= line_len || !isxdigit(line[i]))
>> - break;
>> - c2 = line[i++];
>> - /* Followed by a space */
>> - if (i < line_len && line[i] && !isspace(line[i]))
>> + /* Skip initial spaces */
>> + while (*p) {
>> + if (!isspace(*p))
>> break;
>> - /* Store byte */
>> - *(unsigned char *)buf = (hex(c1) << 4) | hex(c2);
>> - buf += 1;
>> - j++;
>> + p++;
>> }
>> +
>> + do {
>> + ret = read_objdump_chunk(&p, &buf, &buf_len);
>> + bytes_read += ret;
>> + p++;
>> + } while (ret > 0);
>> +
>> /* return number of successfully read bytes */
>> - return j;
>> + return bytes_read;
>> }
>>
>> static int read_objdump_output(FILE *f, void *buf, size_t *len, u64 start_addr)
>> @@ -95,7 +137,7 @@ static int read_objdump_output(FILE *f, void *buf, size_t *len, u64 start_addr)
>> }
>>
>> /* read objdump data into temporary buffer */
>> - read_bytes = read_objdump_line(line, ret, tmp, sizeof(tmp));
>> + read_bytes = read_objdump_line(line, tmp, sizeof(tmp));
>> if (!read_bytes)
>> continue;
>>
>> @@ -152,7 +194,7 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf,
>>
>> ret = read_objdump_output(f, buf, &len, addr);
>> if (len) {
>> - pr_debug("objdump read too few bytes\n");
>> + pr_debug("objdump read too few bytes: %lu\n", len);
>> if (!ret)
>> ret = len;
>> }
>> --
>> 1.8.3.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] perf tests: objdump output can contain multi byte chunks
2016-01-13 8:22 ` Adrian Hunter
@ 2016-08-02 19:07 ` Arnaldo Carvalho de Melo
2016-08-02 19:33 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-08-02 19:07 UTC (permalink / raw)
To: Adrian Hunter
Cc: Arnaldo Carvalho de Melo, Jan Stancek, linux-kernel, xiakaixu,
cjashfor, dsahern, fweisbec, jolsa, namhyung, paulus,
a.p.zijlstra
Em Wed, Jan 13, 2016 at 10:22:04AM +0200, Adrian Hunter escreveu:
> On 12/01/16 17:30, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Jan 12, 2016 at 11:07:44AM +0100, Jan Stancek escreveu:
> >> objdump's raw insn output can vary across architectures on number of
> >> bytes per chunk (bpc) displayed and their endian.
> >>
> >> code-reading test relied on reading objdump output as 1 bpc. Kaixu Xia
> >> reported test failure on ARM64, where objdump displays 4 bpc:
> >> 70c48: f90027bf str xzr, [x29,#72]
> >> 70c4c: 91224000 add x0, x0, #0x890
> >> 70c50: f90023a0 str x0, [x29,#64]
> >>
> >> This patch adds support to read raw insn output for any bpc length.
> >> In case of 2+ bpc it also guesses objdump's display endian.
> >
> > Adrian, from a quick read of the patch and problem/solution description,
> > I think this is ok and Kaixu reports it solves the problem on ARM64, can
> > I have your reviewed-by/Acked-by?
>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Oops, this one got lost, finally applying...
- Arnaldo
> >
> > Thanks,
> >
> > - Arnaldo
> >
> >> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> >> Reported-and-tested-by: Kaixu Xia <xiakaixu@huawei.com>
> >> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> >> Cc: Adrian Hunter <adrian.hunter@intel.com>
> >> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> >> Cc: David Ahern <dsahern@gmail.com>
> >> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> >> Cc: Jiri Olsa <jolsa@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 | 100 ++++++++++++++++++++++++++++------------
> >> 1 file changed, 71 insertions(+), 29 deletions(-)
> >>
> >> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> >> index a767a6400c5c..0108cb22d1a2 100644
> >> --- a/tools/perf/tests/code-reading.c
> >> +++ b/tools/perf/tests/code-reading.c
> >> @@ -33,44 +33,86 @@ static unsigned int hex(char c)
> >> return c - 'A' + 10;
> >> }
> >>
> >> -static size_t read_objdump_line(const char *line, size_t line_len, void *buf,
> >> - size_t len)
> >> +static size_t read_objdump_chunk(const char **line, unsigned char **buf,
> >> + size_t *buf_len)
> >> +{
> >> + size_t bytes_read = 0;
> >> + unsigned char *chunk_start = *buf;
> >> +
> >> + /* Read bytes */
> >> + while (*buf_len > 0) {
> >> + char c1, c2;
> >> +
> >> + /* Get 2 hex digits */
> >> + c1 = *(*line)++;
> >> + if (!isxdigit(c1))
> >> + break;
> >> + c2 = *(*line)++;
> >> + if (!isxdigit(c2))
> >> + break;
> >> +
> >> + /* Store byte and advance buf */
> >> + **buf = (hex(c1) << 4) | hex(c2);
> >> + (*buf)++;
> >> + (*buf_len)--;
> >> + bytes_read++;
> >> +
> >> + /* End of chunk? */
> >> + if (isspace(**line))
> >> + break;
> >> + }
> >> +
> >> + /*
> >> + * objdump will display raw insn as LE if code endian
> >> + * is LE and bytes_per_chunk > 1. In that case reverse
> >> + * the chunk we just read.
> >> + *
> >> + * see disassemble_bytes() at binutils/objdump.c for details
> >> + * how objdump chooses display endian)
> >> + */
> >> + if (bytes_read > 1 && !bigendian()) {
> >> + unsigned char *chunk_end = chunk_start + bytes_read - 1;
> >> + unsigned char tmp;
> >> +
> >> + while (chunk_start < chunk_end) {
> >> + tmp = *chunk_start;
> >> + *chunk_start = *chunk_end;
> >> + *chunk_end = tmp;
> >> + chunk_start++;
> >> + chunk_end--;
> >> + }
> >> + }
> >> +
> >> + return bytes_read;
> >> +}
> >> +
> >> +static size_t read_objdump_line(const char *line, unsigned char *buf,
> >> + size_t buf_len)
> >> {
> >> const char *p;
> >> - size_t i, j = 0;
> >> + size_t ret, bytes_read = 0;
> >>
> >> /* Skip to a colon */
> >> p = strchr(line, ':');
> >> if (!p)
> >> return 0;
> >> - i = p + 1 - line;
> >> + p++;
> >>
> >> - /* Read bytes */
> >> - while (j < len) {
> >> - char c1, c2;
> >> -
> >> - /* Skip spaces */
> >> - for (; i < line_len; i++) {
> >> - if (!isspace(line[i]))
> >> - break;
> >> - }
> >> - /* Get 2 hex digits */
> >> - if (i >= line_len || !isxdigit(line[i]))
> >> - break;
> >> - c1 = line[i++];
> >> - if (i >= line_len || !isxdigit(line[i]))
> >> - break;
> >> - c2 = line[i++];
> >> - /* Followed by a space */
> >> - if (i < line_len && line[i] && !isspace(line[i]))
> >> + /* Skip initial spaces */
> >> + while (*p) {
> >> + if (!isspace(*p))
> >> break;
> >> - /* Store byte */
> >> - *(unsigned char *)buf = (hex(c1) << 4) | hex(c2);
> >> - buf += 1;
> >> - j++;
> >> + p++;
> >> }
> >> +
> >> + do {
> >> + ret = read_objdump_chunk(&p, &buf, &buf_len);
> >> + bytes_read += ret;
> >> + p++;
> >> + } while (ret > 0);
> >> +
> >> /* return number of successfully read bytes */
> >> - return j;
> >> + return bytes_read;
> >> }
> >>
> >> static int read_objdump_output(FILE *f, void *buf, size_t *len, u64 start_addr)
> >> @@ -95,7 +137,7 @@ static int read_objdump_output(FILE *f, void *buf, size_t *len, u64 start_addr)
> >> }
> >>
> >> /* read objdump data into temporary buffer */
> >> - read_bytes = read_objdump_line(line, ret, tmp, sizeof(tmp));
> >> + read_bytes = read_objdump_line(line, tmp, sizeof(tmp));
> >> if (!read_bytes)
> >> continue;
> >>
> >> @@ -152,7 +194,7 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf,
> >>
> >> ret = read_objdump_output(f, buf, &len, addr);
> >> if (len) {
> >> - pr_debug("objdump read too few bytes\n");
> >> + pr_debug("objdump read too few bytes: %lu\n", len);
> >> if (!ret)
> >> ret = len;
> >> }
> >> --
> >> 1.8.3.1
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] perf tests: objdump output can contain multi byte chunks
2016-08-02 19:07 ` Arnaldo Carvalho de Melo
@ 2016-08-02 19:33 ` Arnaldo Carvalho de Melo
2016-08-02 19:47 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-08-02 19:33 UTC (permalink / raw)
To: Jan Stancek
Cc: Adrian Hunter, Arnaldo Carvalho de Melo, linux-kernel, xiakaixu,
cjashfor, dsahern, fweisbec, jolsa, namhyung, paulus,
a.p.zijlstra
Em Tue, Aug 02, 2016 at 04:07:00PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Jan 13, 2016 at 10:22:04AM +0200, Adrian Hunter escreveu:
> > On 12/01/16 17:30, Arnaldo Carvalho de Melo wrote:
> > > Em Tue, Jan 12, 2016 at 11:07:44AM +0100, Jan Stancek escreveu:
> > >> objdump's raw insn output can vary across architectures on number of
> > >> bytes per chunk (bpc) displayed and their endian.
> > >>
> > >> code-reading test relied on reading objdump output as 1 bpc. Kaixu Xia
> > >> reported test failure on ARM64, where objdump displays 4 bpc:
> > >> 70c48: f90027bf str xzr, [x29,#72]
> > >> 70c4c: 91224000 add x0, x0, #0x890
> > >> 70c50: f90023a0 str x0, [x29,#64]
> > >>
> > >> This patch adds support to read raw insn output for any bpc length.
> > >> In case of 2+ bpc it also guesses objdump's display endian.
> > >
> > > Adrian, from a quick read of the patch and problem/solution description,
> > > I think this is ok and Kaixu reports it solves the problem on ARM64, can
> > > I have your reviewed-by/Acked-by?
> >
> > Acked-by: Adrian Hunter <adrian.hunter@intel.com>
>
> Oops, this one got lost, finally applying...
Ok, it failed for:
ubuntu:16.04-x-armhf: FAIL
ubuntu:16.04-x-powerpc64: FAIL
Both are related to, which I am fixing:
CC /tmp/build/perf/tests/code-reading.o
In file included from /git/linux/tools/perf/util/cpumap.h:9:0,
from /git/linux/tools/perf/util/evsel.h:11,
from /git/linux/tools/perf/util/evlist.h:10,
from tests/code-reading.c:9:
tests/code-reading.c: In function 'read_via_objdump':
tests/code-reading.c:197:12: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t {aka unsigned int}' [-Werror=format=]
pr_debug("objdump read too few bytes: %lu\n", len);
^
/git/linux/tools/perf/util/debug.h:18:21: note: in definition of macro 'pr_fmt'
#define pr_fmt(fmt) fmt
^
tests/code-reading.c:197:3: note: in expansion of macro 'pr_debug'
pr_debug("objdump read too few bytes: %lu\n", len);
^
> - Arnaldo
>
> > >
> > > Thanks,
> > >
> > > - Arnaldo
> > >
> > >> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> > >> Reported-and-tested-by: Kaixu Xia <xiakaixu@huawei.com>
> > >> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > >> Cc: Adrian Hunter <adrian.hunter@intel.com>
> > >> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> > >> Cc: David Ahern <dsahern@gmail.com>
> > >> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> > >> Cc: Jiri Olsa <jolsa@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 | 100 ++++++++++++++++++++++++++++------------
> > >> 1 file changed, 71 insertions(+), 29 deletions(-)
> > >>
> > >> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> > >> index a767a6400c5c..0108cb22d1a2 100644
> > >> --- a/tools/perf/tests/code-reading.c
> > >> +++ b/tools/perf/tests/code-reading.c
> > >> @@ -33,44 +33,86 @@ static unsigned int hex(char c)
> > >> return c - 'A' + 10;
> > >> }
> > >>
> > >> -static size_t read_objdump_line(const char *line, size_t line_len, void *buf,
> > >> - size_t len)
> > >> +static size_t read_objdump_chunk(const char **line, unsigned char **buf,
> > >> + size_t *buf_len)
> > >> +{
> > >> + size_t bytes_read = 0;
> > >> + unsigned char *chunk_start = *buf;
> > >> +
> > >> + /* Read bytes */
> > >> + while (*buf_len > 0) {
> > >> + char c1, c2;
> > >> +
> > >> + /* Get 2 hex digits */
> > >> + c1 = *(*line)++;
> > >> + if (!isxdigit(c1))
> > >> + break;
> > >> + c2 = *(*line)++;
> > >> + if (!isxdigit(c2))
> > >> + break;
> > >> +
> > >> + /* Store byte and advance buf */
> > >> + **buf = (hex(c1) << 4) | hex(c2);
> > >> + (*buf)++;
> > >> + (*buf_len)--;
> > >> + bytes_read++;
> > >> +
> > >> + /* End of chunk? */
> > >> + if (isspace(**line))
> > >> + break;
> > >> + }
> > >> +
> > >> + /*
> > >> + * objdump will display raw insn as LE if code endian
> > >> + * is LE and bytes_per_chunk > 1. In that case reverse
> > >> + * the chunk we just read.
> > >> + *
> > >> + * see disassemble_bytes() at binutils/objdump.c for details
> > >> + * how objdump chooses display endian)
> > >> + */
> > >> + if (bytes_read > 1 && !bigendian()) {
> > >> + unsigned char *chunk_end = chunk_start + bytes_read - 1;
> > >> + unsigned char tmp;
> > >> +
> > >> + while (chunk_start < chunk_end) {
> > >> + tmp = *chunk_start;
> > >> + *chunk_start = *chunk_end;
> > >> + *chunk_end = tmp;
> > >> + chunk_start++;
> > >> + chunk_end--;
> > >> + }
> > >> + }
> > >> +
> > >> + return bytes_read;
> > >> +}
> > >> +
> > >> +static size_t read_objdump_line(const char *line, unsigned char *buf,
> > >> + size_t buf_len)
> > >> {
> > >> const char *p;
> > >> - size_t i, j = 0;
> > >> + size_t ret, bytes_read = 0;
> > >>
> > >> /* Skip to a colon */
> > >> p = strchr(line, ':');
> > >> if (!p)
> > >> return 0;
> > >> - i = p + 1 - line;
> > >> + p++;
> > >>
> > >> - /* Read bytes */
> > >> - while (j < len) {
> > >> - char c1, c2;
> > >> -
> > >> - /* Skip spaces */
> > >> - for (; i < line_len; i++) {
> > >> - if (!isspace(line[i]))
> > >> - break;
> > >> - }
> > >> - /* Get 2 hex digits */
> > >> - if (i >= line_len || !isxdigit(line[i]))
> > >> - break;
> > >> - c1 = line[i++];
> > >> - if (i >= line_len || !isxdigit(line[i]))
> > >> - break;
> > >> - c2 = line[i++];
> > >> - /* Followed by a space */
> > >> - if (i < line_len && line[i] && !isspace(line[i]))
> > >> + /* Skip initial spaces */
> > >> + while (*p) {
> > >> + if (!isspace(*p))
> > >> break;
> > >> - /* Store byte */
> > >> - *(unsigned char *)buf = (hex(c1) << 4) | hex(c2);
> > >> - buf += 1;
> > >> - j++;
> > >> + p++;
> > >> }
> > >> +
> > >> + do {
> > >> + ret = read_objdump_chunk(&p, &buf, &buf_len);
> > >> + bytes_read += ret;
> > >> + p++;
> > >> + } while (ret > 0);
> > >> +
> > >> /* return number of successfully read bytes */
> > >> - return j;
> > >> + return bytes_read;
> > >> }
> > >>
> > >> static int read_objdump_output(FILE *f, void *buf, size_t *len, u64 start_addr)
> > >> @@ -95,7 +137,7 @@ static int read_objdump_output(FILE *f, void *buf, size_t *len, u64 start_addr)
> > >> }
> > >>
> > >> /* read objdump data into temporary buffer */
> > >> - read_bytes = read_objdump_line(line, ret, tmp, sizeof(tmp));
> > >> + read_bytes = read_objdump_line(line, tmp, sizeof(tmp));
> > >> if (!read_bytes)
> > >> continue;
> > >>
> > >> @@ -152,7 +194,7 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf,
> > >>
> > >> ret = read_objdump_output(f, buf, &len, addr);
> > >> if (len) {
> > >> - pr_debug("objdump read too few bytes\n");
> > >> + pr_debug("objdump read too few bytes: %lu\n", len);
> > >> if (!ret)
> > >> ret = len;
> > >> }
> > >> --
> > >> 1.8.3.1
> > >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] perf tests: objdump output can contain multi byte chunks
2016-08-02 19:33 ` Arnaldo Carvalho de Melo
@ 2016-08-02 19:47 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-08-02 19:47 UTC (permalink / raw)
To: Jan Stancek
Cc: Adrian Hunter, Arnaldo Carvalho de Melo, linux-kernel, xiakaixu,
cjashfor, dsahern, fweisbec, jolsa, namhyung, paulus,
a.p.zijlstra
Em Tue, Aug 02, 2016 at 04:33:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Aug 02, 2016 at 04:07:00PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Jan 13, 2016 at 10:22:04AM +0200, Adrian Hunter escreveu:
> > > Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> > Oops, this one got lost, finally applying...
> Ok, it failed for:
> ubuntu:16.04-x-armhf: FAIL
> ubuntu:16.04-x-powerpc64: FAIL
>
> Both are related to, which I am fixing:
>
> CC /tmp/build/perf/tests/code-reading.o
> In file included from /git/linux/tools/perf/util/cpumap.h:9:0,
> from /git/linux/tools/perf/util/evsel.h:11,
> from /git/linux/tools/perf/util/evlist.h:10,
> from tests/code-reading.c:9:
> tests/code-reading.c: In function 'read_via_objdump':
> tests/code-reading.c:197:12: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t {aka unsigned int}' [-Werror=format=]
> pr_debug("objdump read too few bytes: %lu\n", len);
> ^
> /git/linux/tools/perf/util/debug.h:18:21: note: in definition of macro 'pr_fmt'
> #define pr_fmt(fmt) fmt
> ^
> tests/code-reading.c:197:3: note: in expansion of macro 'pr_debug'
> pr_debug("objdump read too few bytes: %lu\n", len);
> ^
Just use %zd for size_t:
[root@jouet ~]# dm ubuntu:16.04-x-armhf ubuntu:16.04-x-powerpc64
1: ubuntu:16.04-x-armhf: Ok
2: ubuntu:16.04-x-powerpc64: Ok
[root@jouet ~]#
- Arnaldo
^ permalink raw reply [flat|nested] 7+ messages in thread
* [tip:perf/urgent] perf tests: objdump output can contain multi byte chunks
2016-01-12 10:07 [PATCH] perf tests: objdump output can contain multi byte chunks Jan Stancek
2016-01-12 15:30 ` Arnaldo Carvalho de Melo
@ 2016-08-04 9:14 ` tip-bot for Jan Stancek
1 sibling, 0 replies; 7+ messages in thread
From: tip-bot for Jan Stancek @ 2016-08-04 9:14 UTC (permalink / raw)
To: linux-tip-commits
Cc: jolsa, hpa, xiakaixu, jstancek, cjashfor, linux-kernel, namhyung,
a.p.zijlstra, adrian.hunter, fweisbec, dsahern, paulus, acme,
tglx, mingo
Commit-ID: b2d0dbf09772d091368261ce95db3afce45d994d
Gitweb: http://git.kernel.org/tip/b2d0dbf09772d091368261ce95db3afce45d994d
Author: Jan Stancek <jstancek@redhat.com>
AuthorDate: Tue, 12 Jan 2016 11:07:44 +0100
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 2 Aug 2016 16:42:51 -0300
perf tests: objdump output can contain multi byte chunks
objdump's raw insn output can vary across architectures on the number of
bytes per chunk (bpc) displayed and their endianness.
The code-reading test relied on reading objdump output as 1 bpc. Kaixu
Xia reported test failure on ARM64, where objdump displays 4 bpc:
70c48: f90027bf str xzr, [x29,#72]
70c4c: 91224000 add x0, x0, #0x890
70c50: f90023a0 str x0, [x29,#64]
This patch adds support to read raw insn output for any bpc length.
In case of 2+ bpc it also guesses objdump's display endian.
Reported-and-Tested-by: Kaixu Xia <xiakaixu@huawei.com>
Signed-off-by: Jan Stancek <jstancek@redhat.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/07f0f7bcbda78deb423298708ef9b6a54d6b92bd.1452592712.git.jstancek@redhat.com
[ Fix up pr_fmt() call to use %zd for size_t variables, fixing the build on Ubuntu cross-compiling to armhf and ppc64 ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/tests/code-reading.c | 100 ++++++++++++++++++++++++++++------------
1 file changed, 71 insertions(+), 29 deletions(-)
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 68a69a1..2af156a 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -33,44 +33,86 @@ static unsigned int hex(char c)
return c - 'A' + 10;
}
-static size_t read_objdump_line(const char *line, size_t line_len, void *buf,
- size_t len)
+static size_t read_objdump_chunk(const char **line, unsigned char **buf,
+ size_t *buf_len)
{
- const char *p;
- size_t i, j = 0;
-
- /* Skip to a colon */
- p = strchr(line, ':');
- if (!p)
- return 0;
- i = p + 1 - line;
+ size_t bytes_read = 0;
+ unsigned char *chunk_start = *buf;
/* Read bytes */
- while (j < len) {
+ while (*buf_len > 0) {
char c1, c2;
- /* Skip spaces */
- for (; i < line_len; i++) {
- if (!isspace(line[i]))
- break;
- }
/* Get 2 hex digits */
- if (i >= line_len || !isxdigit(line[i]))
+ c1 = *(*line)++;
+ if (!isxdigit(c1))
break;
- c1 = line[i++];
- if (i >= line_len || !isxdigit(line[i]))
+ c2 = *(*line)++;
+ if (!isxdigit(c2))
break;
- c2 = line[i++];
- /* Followed by a space */
- if (i < line_len && line[i] && !isspace(line[i]))
+
+ /* Store byte and advance buf */
+ **buf = (hex(c1) << 4) | hex(c2);
+ (*buf)++;
+ (*buf_len)--;
+ bytes_read++;
+
+ /* End of chunk? */
+ if (isspace(**line))
break;
- /* Store byte */
- *(unsigned char *)buf = (hex(c1) << 4) | hex(c2);
- buf += 1;
- j++;
}
+
+ /*
+ * objdump will display raw insn as LE if code endian
+ * is LE and bytes_per_chunk > 1. In that case reverse
+ * the chunk we just read.
+ *
+ * see disassemble_bytes() at binutils/objdump.c for details
+ * how objdump chooses display endian)
+ */
+ if (bytes_read > 1 && !bigendian()) {
+ unsigned char *chunk_end = chunk_start + bytes_read - 1;
+ unsigned char tmp;
+
+ while (chunk_start < chunk_end) {
+ tmp = *chunk_start;
+ *chunk_start = *chunk_end;
+ *chunk_end = tmp;
+ chunk_start++;
+ chunk_end--;
+ }
+ }
+
+ return bytes_read;
+}
+
+static size_t read_objdump_line(const char *line, unsigned char *buf,
+ size_t buf_len)
+{
+ const char *p;
+ size_t ret, bytes_read = 0;
+
+ /* Skip to a colon */
+ p = strchr(line, ':');
+ if (!p)
+ return 0;
+ p++;
+
+ /* Skip initial spaces */
+ while (*p) {
+ if (!isspace(*p))
+ break;
+ p++;
+ }
+
+ do {
+ ret = read_objdump_chunk(&p, &buf, &buf_len);
+ bytes_read += ret;
+ p++;
+ } while (ret > 0);
+
/* return number of successfully read bytes */
- return j;
+ return bytes_read;
}
static int read_objdump_output(FILE *f, void *buf, size_t *len, u64 start_addr)
@@ -95,7 +137,7 @@ static int read_objdump_output(FILE *f, void *buf, size_t *len, u64 start_addr)
}
/* read objdump data into temporary buffer */
- read_bytes = read_objdump_line(line, ret, tmp, sizeof(tmp));
+ read_bytes = read_objdump_line(line, tmp, sizeof(tmp));
if (!read_bytes)
continue;
@@ -152,7 +194,7 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf,
ret = read_objdump_output(f, buf, &len, addr);
if (len) {
- pr_debug("objdump read too few bytes\n");
+ pr_debug("objdump read too few bytes: %zd\n", len);
if (!ret)
ret = len;
}
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-08-04 9:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-12 10:07 [PATCH] perf tests: objdump output can contain multi byte chunks Jan Stancek
2016-01-12 15:30 ` Arnaldo Carvalho de Melo
2016-01-13 8:22 ` Adrian Hunter
2016-08-02 19:07 ` Arnaldo Carvalho de Melo
2016-08-02 19:33 ` Arnaldo Carvalho de Melo
2016-08-02 19:47 ` Arnaldo Carvalho de Melo
2016-08-04 9:14 ` [tip:perf/urgent] " tip-bot for Jan Stancek
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).