From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28911 invoked by alias); 3 Jul 2014 08:31:11 -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 28889 invoked by uid 89); 3 Jul 2014 08:31:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ig0-f170.google.com Received: from mail-ig0-f170.google.com (HELO mail-ig0-f170.google.com) (209.85.213.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 03 Jul 2014 08:31:06 +0000 Received: by mail-ig0-f170.google.com with SMTP id h15so7368708igd.5 for ; Thu, 03 Jul 2014 01:31:04 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=6uxpHnYmCw9kTKMA7fIvut9w6Zo3KsehziYGW9t6xZc=; b=VdosEiKavg2qcCkQ0t593BHEpTPxzpcvm7lyRCXXrjzkqDUg59llqd5OAvjFkOMFfP 5EROHhm7WYixBqOqX4pRc2gDKI9DYzVC+zU1LkaXbh/qU9rH2+6E7Rmrmqw3y21R0uN1 uBXXSOXMkv4JEO22eW9Og9AgZXlqa6GELqE58jOX50nMYDIRRu26c28DKYsQG7MwtWXQ b1Bdt5LqDuu7qr8S12uK+xQ6S+ypZd+ABEbkFC9Um3kVZW19cJ7AkpMgdEFkJE5Lfgir wmJg8Djjr7l5ppfwIiV4D7rUQGpUM+TRurPYh1D2f8AVm5VVh8YzuJt5tXcOVjaTdH3O /4xA== X-Gm-Message-State: ALoCoQlIyVD2GaCY+N71MFxpFMOJ8QKCTIcIfIfWu3axSNYWsa1Os4o4G5Ub+Tct7KdS0tWfZRPW MIME-Version: 1.0 X-Received: by 10.50.57.80 with SMTP id g16mr53573777igq.30.1404376264763; Thu, 03 Jul 2014 01:31:04 -0700 (PDT) Received: by 10.64.135.33 with HTTP; Thu, 3 Jul 2014 01:31:04 -0700 (PDT) In-Reply-To: <1404367792-23234-2-git-send-email-yao@codesourcery.com> References: <1404367792-23234-1-git-send-email-yao@codesourcery.com> <1404367792-23234-2-git-send-email-yao@codesourcery.com> Date: Thu, 03 Jul 2014 08:31:00 -0000 Message-ID: Subject: Re: [PATCH 1/4] Restrict matching add/sub sp, #imm From: Will Newton To: Yao Qi Cc: "gdb-patches@sourceware.org" Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-07/txt/msg00046.txt.bz2 On 3 July 2014 07:09, Yao Qi wrote: > Currently, GDB matches both add/sub sp, #imm in prologue and epilogue, > which is not very precise. On the instruction level, the immediate > number in both instruction can't be negative, so 'sub sp, #imm' only > appears in prologue while 'add sp, #imm' only appears in epilogue. > Note that on assembly level, we can write 'add sp, -8', but gas will > translate to 'sub sp, 8' instruction. > > This patch is to only match 'sub sp, #imm' in prologue and match > 'add sp, #immm' in epilogue. It paves the way for the following > patch. > > gdb: > > 2014-07-02 Yao Qi > > * arm-tdep.c (thumb_analyze_prologue): Don't match instruction > 'add sp, #immm'. One too many ms? > (thumb_in_function_epilogue_p): Don't match 'sub sp, #imm'. > --- > gdb/arm-tdep.c | 15 +++++---------- > 1 file changed, 5 insertions(+), 10 deletions(-) > > diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c > index 8cc60a4..0fc7fc1 100644 > --- a/gdb/arm-tdep.c > +++ b/gdb/arm-tdep.c > @@ -737,16 +737,11 @@ thumb_analyze_prologue (struct gdbarch *gdbarch, > pv_area_store (stack, regs[ARM_SP_REGNUM], 4, regs[regno]); > } > } > - else if ((insn & 0xff00) == 0xb000) /* add sp, #simm OR > - sub sp, #simm */ > + else if ((insn & 0xff80) == 0xb080) /* sub sp, #simm */ I wonder if we should adjust the comment to just #imm, as #simm implies it is a signed quantity. > { > offset = (insn & 0x7f) << 2; /* get scaled offset */ > - if (insn & 0x80) /* Check for SUB. */ > - regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM], > - -offset); > - else > - regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM], > - offset); > + regs[ARM_SP_REGNUM] = pv_add_constant (regs[ARM_SP_REGNUM], > + -offset); > } > else if ((insn & 0xf800) == 0xa800) /* add Rd, sp, #imm */ > regs[bits (insn, 8, 10)] = pv_add_constant (regs[ARM_SP_REGNUM], > @@ -3264,7 +3259,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc) > found_return = 1; > else if (insn == 0x46bd) /* mov sp, r7 */ > found_stack_adjust = 1; > - else if ((insn & 0xff00) == 0xb000) /* add sp, imm or sub sp, imm */ > + else if ((insn & 0xff80) == 0xb000) /* add sp, imm */ > found_stack_adjust = 1; > else if ((insn & 0xfe00) == 0xbc00) /* pop */ > { > @@ -3324,7 +3319,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc) > > if (insn2 == 0x46bd) /* mov sp, r7 */ > found_stack_adjust = 1; > - else if ((insn2 & 0xff00) == 0xb000) /* add sp, imm or sub sp, imm */ > + else if ((insn2 & 0xff80) == 0xb000) /* add sp, imm */ > found_stack_adjust = 1; > else if ((insn2 & 0xff00) == 0xbc00) /* pop without PC */ > found_stack_adjust = 1; Otherwise the patch looks good to me. -- Will Newton Toolchain Working Group, Linaro