Messages in this thread Patch in this message | | | Date | Tue, 2 Aug 2016 16:33:16 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH] perf tests: objdump output can contain multi byte chunks |
| |
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 > > >
| |