From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756607AbcHBTlU (ORCPT ); Tue, 2 Aug 2016 15:41:20 -0400 Received: from mail.kernel.org ([198.145.29.136]:52180 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754156AbcHBTlJ (ORCPT ); Tue, 2 Aug 2016 15:41:09 -0400 Date: Tue, 2 Aug 2016 16:33:16 -0300 From: Arnaldo Carvalho de Melo To: Jan Stancek Cc: Adrian Hunter , Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, xiakaixu@huawei.com, cjashfor@linux.vnet.ibm.com, dsahern@gmail.com, fweisbec@gmail.com, jolsa@kernel.org, namhyung@kernel.org, paulus@samba.org, a.p.zijlstra@chello.nl Subject: Re: [PATCH] perf tests: objdump output can contain multi byte chunks Message-ID: <20160802193316.GB14639@kernel.org> References: <07f0f7bcbda78deb423298708ef9b6a54d6b92bd.1452592712.git.jstancek@redhat.com> <20160112153054.GF21083@redhat.com> <5696092C.30100@intel.com> <20160802190700.GA14639@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160802190700.GA14639@kernel.org> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.6.2 (2016-07-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > 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 > > >> Reported-and-tested-by: Kaixu Xia > > >> Cc: Arnaldo Carvalho de Melo > > >> Cc: Adrian Hunter > > >> Cc: Corey Ashford > > >> Cc: David Ahern > > >> Cc: Frederic Weisbecker > > >> Cc: Jiri Olsa > > >> Cc: Namhyung Kim > > >> Cc: Paul Mackerras > > >> Cc: Peter Zijlstra > > >> --- > > >> 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 > > >