* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.