* [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