From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4752 invoked by alias); 30 Nov 2016 18:33:01 -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 4665 invoked by uid 89); 30 Nov 2016 18:33:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.8 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 30 Nov 2016 18:32:51 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ED990CA610; Wed, 30 Nov 2016 18:32:49 +0000 (UTC) Received: from [127.0.0.1] (ovpn03.gateway.prod.ext.phx2.redhat.com [10.5.9.3]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uAUIWmB8025520; Wed, 30 Nov 2016 13:32:49 -0500 Subject: Re: [PATCH 2/2] [AArch64] Recognize STR instruction in prologue To: Yao Qi , gdb-patches@sourceware.org References: <1480428758-2481-1-git-send-email-yao.qi@linaro.org> <1480428758-2481-2-git-send-email-yao.qi@linaro.org> From: Pedro Alves Message-ID: Date: Wed, 30 Nov 2016 18:33:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1480428758-2481-2-git-send-email-yao.qi@linaro.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-11/txt/msg01010.txt.bz2 On 11/29/2016 02:12 PM, Yao Qi wrote: > This patch teaches GDB AArch64 backend to recognize STR instructions > in prologue, like 'str x19, [sp, #-48]!' or 'str w0, [sp, #44]'. > The unit test is added too. > > gdb: > > 2016-11-28 Yao Qi > > * aarch64-tdep.c (aarch64_analyze_prologue): Recognize STR > instruction. > (aarch64_analyze_prologue_test): More tests. > --- > gdb/aarch64-tdep.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 68 insertions(+) > > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c > index b10002a..fc68b8a 100644 > --- a/gdb/aarch64-tdep.c > +++ b/gdb/aarch64-tdep.c > @@ -397,6 +397,35 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, > regs[rn] = pv_add_constant (regs[rn], imm); > > } > + else if ((inst.opcode->iclass == ldst_imm9 /* Signed immediate. */ > + || (inst.opcode->iclass == ldst_pos /* Unsigned immediate. */ > + && (inst.opcode->op == OP_STR_POS > + || inst.opcode->op == OP_STRF_POS))) > + && inst.operands[1].addr.base_regno == AARCH64_SP_REGNUM > + && strcmp ("str", inst.opcode->name) == 0) > + { > + /* STR (immediate) */ > + unsigned int rt = inst.operands[0].reg.regno; > + int32_t imm = inst.operands[1].addr.offset.imm; > + unsigned rn = inst.operands[1].addr.base_regno; > + int is64 > + = (aarch64_get_qualifier_esize (inst.operands[0].qualifier) == 8); > + gdb_assert (inst.operands[0].type == AARCH64_OPND_Rt > + || inst.operands[0].type == AARCH64_OPND_Ft); > + > + if (inst.operands[0].type == AARCH64_OPND_Ft) > + { > + /* Only bottom 64-bit of each V register (D register) need > + to be preserved. */ > + gdb_assert (inst.operands[0].qualifier == AARCH64_OPND_QLF_S_D); > + rt += AARCH64_X_REGISTER_COUNT; > + } > + > + pv_area_store (stack, pv_add_constant (regs[rn], imm), > + is64 ? 8 : 4, regs[rt]); > + if (inst.operands[1].addr.writeback) > + regs[rn] = pv_add_constant (regs[rn], imm); > + } > else if (inst.opcode->iclass == testbranch) > { > /* Stop analysis on branch. */ > @@ -540,6 +569,45 @@ aarch64_analyze_prologue_test (void) > > SELF_CHECK (cache.saved_regs[i + regnum + AARCH64_D0_REGNUM].addr == -1); > } > + > + /* Second test. */ I'd suggest putting each test in its own scope, to avoid accidental local variable reuse (or unintentionally reusing internal state of internal variables). Like /* Second test. */ { /* test code here */ } Also, I think you should expand that "second test" comment. It should inform the reader about what is important about the test. In this case, it should probably say something about "STR". Likewise for the first test. > + cache.saved_regs = trad_frame_alloc_saved_regs (gdbarch); > + > + instruction_reader_test test2 { > + 0xf81d0ff3, /* str x19, [sp, #-48]! */ > + 0xb9002fe0, /* str w0, [sp, #44] */ > + 0xf90013e1, /* str x1, [sp, #32]*/ > + 0xfd000fe0, /* str d0, [sp, #24] */ > + 0xaa0203f3, /* mov x19, x2 */ > + 0xf94013e0, /* ldr x0, [sp, #32] */ > + }; Indentation looks odd here too. Thanks, Pedro Alves