From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 66017 invoked by alias); 13 Apr 2016 10:00:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 65897 invoked by uid 89); 13 Apr 2016 10:00:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=pierre, H*f:sk:1460473, H*MI:sk:1460473, H*i:sk:1460473 X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 13 Apr 2016 10:00:16 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6041D28; Wed, 13 Apr 2016 02:59:01 -0700 (PDT) Received: from [10.1.201.136] (e105615-lin.cambridge.arm.com [10.1.201.136]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 945343F25E; Wed, 13 Apr 2016 03:00:14 -0700 (PDT) Subject: Re: [PATCH] Fix aarch64 ftrace JIT condition testcase To: Antoine Tremblay , gdb-patches@sourceware.org References: <1460473982-20054-1-git-send-email-antoine.tremblay@ericsson.com> From: Pierre Langlois Message-ID: <570E18AD.4040108@foss.arm.com> Date: Wed, 13 Apr 2016 10:00:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <1460473982-20054-1-git-send-email-antoine.tremblay@ericsson.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-04/txt/msg00264.txt.bz2 Hi Antoine, On 12/04/16 16:13, Antoine Tremblay wrote: > This patch fixes the following failure: > FAIL: gdb.trace/trace-condition.exp: ftrace: -(21 << 1) == -42: check 10 > frames were collected. > > This was due to aarch64_emit_sub using the wrong order in its operands, so the > operation would end up being 42 - 0 rather than 0 - 42. Ooops, thanks for the fix! I was a little confused how I could have missed this, it turns out I had forgotten to had parentheses in `-(21 << 1) == -42' at the time, so `emit_sub' was not tested here. I aimed at testing `emit_sub' with the following test case which is clearly not good enough: gdb.trace/trace-condition.exp: ftrace: 21 - 21 == 0 Would you be OK with changing it to "42 - 21 == 21" or something? Thanks again! Pierre > > This patch also fixes the order of aarch64_emit_add for clarity. > > Tested on aarch64-native-extended-gdbserver. > > Note: trace-condition.exp was broken a bit so I had to modify it to run > the test. A fix is coming for that in another patch. > > gdb/gdbserver/ChangeLog: > > * linux-aarch64-low.c (aarch64_emit_add): Switch x1 and x0. > (aarch64_emit_sub): Likewise. > --- > gdb/gdbserver/linux-aarch64-low.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c > index 12fe2e6..d237bde 100644 > --- a/gdb/gdbserver/linux-aarch64-low.c > +++ b/gdb/gdbserver/linux-aarch64-low.c > @@ -2258,7 +2258,7 @@ aarch64_emit_add (void) > uint32_t *p = buf; > > p += emit_pop (p, x1); > - p += emit_add (p, x0, x0, register_operand (x1)); > + p += emit_add (p, x0, x1, register_operand (x0)); > > emit_ops_insns (buf, p - buf); > } > @@ -2272,7 +2272,7 @@ aarch64_emit_sub (void) > uint32_t *p = buf; > > p += emit_pop (p, x1); > - p += emit_sub (p, x0, x0, register_operand (x1)); > + p += emit_sub (p, x0, x1, register_operand (x0)); > > emit_ops_insns (buf, p - buf); > } >