From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 454 invoked by alias); 12 Oct 2011 00:47:12 -0000 Received: (qmail 446 invoked by uid 22791); 12 Oct 2011 00:47:11 -0000 X-SWARE-Spam-Status: No, hits=-1.5 required=5.0 tests=AWL,BAYES_00,TW_EG,TW_XT X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 12 Oct 2011 00:46:56 +0000 Received: from nat-jpt.mentorg.com ([192.94.33.2] helo=PR1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1RDmyF-0004Nd-CM from Yao_Qi@mentor.com ; Tue, 11 Oct 2011 17:46:55 -0700 Received: from [127.0.0.1] ([172.16.63.104]) by PR1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Wed, 12 Oct 2011 09:46:53 +0900 Message-ID: <4E94E367.3040109@codesourcery.com> Date: Wed, 12 Oct 2011 00:47:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:7.0) Gecko/20110923 Thunderbird/7.0 MIME-Version: 1.0 To: Ulrich Weigand CC: gdb-patches@sourceware.org Subject: Re: [PATCH] Fix that different function breakpoints are set@same pc address (PR gdb/12703) References: <201110101446.p9AEklVX022612@d06av02.portsmouth.uk.ibm.com> In-Reply-To: <201110101446.p9AEklVX022612@d06av02.portsmouth.uk.ibm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes 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 X-SW-Source: 2011-10/txt/msg00342.txt.bz2 On 10/10/2011 10:46 PM, Ulrich Weigand wrote: >> > - else if ((insn & 0xe000) == 0xe000) >> > + else if ((insn & 0xe000) == 0xe000 && (insn & 0x1800) != 0) >> > { > Instead of open-coding the check, I think it would be preferable to > use the thumb_insn_size routine instead. > > Note that there are a number of other places that either already open- > code the correct check, or -worse- use the same incorrect check as the > code above: > > - thumb_in_function_epilogue_p > - thumb_get_next_pc_raw > - arm_breakpoint_from_pc > > Would you mind converting them all to thumb_insn_size? Yeah, we should replace these checks around many places with thumb_insn_size, I agree. Here is the patch for this purpose. Regression tested on arm-linux-gnueabi with both -marm and -mthumb. OK for mainline? -- Yao (齐尧) PR gdb/12703 * arm-tdep.c (thumb_analyze_prologue): Call thumb_insn_size to check whether insn is a 32-bit Thumb-2 instruction. (thumb_in_function_epilogue_p): Likewise. (thumb_get_next_pc_raw): Likewise. (arm_breakpoint_from_pc): Likewise. diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 278e6e9..0db8b5f 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -231,6 +231,8 @@ static void arm_neon_quad_write (struct gdbarch *gdbarch, struct regcache *regcache, int regnum, const gdb_byte *buf); +static int thumb_insn_size (unsigned short inst1); + struct arm_prologue_cache { /* The stack pointer at the time this frame was created; i.e. the @@ -836,7 +838,7 @@ thumb_analyze_prologue (struct gdbarch *gdbarch, constant = read_memory_unsigned_integer (loc, 4, byte_order); regs[bits (insn, 8, 10)] = pv_constant (constant); } - else if ((insn & 0xe000) == 0xe000) + else if (thumb_insn_size (insn) == 4) /* 32-bit Thumb-2 instructions. */ { unsigned short inst2; @@ -3093,7 +3095,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc) if (insn & 0x0100) /* include PC. */ found_return = 1; } - else if ((insn & 0xe000) == 0xe000) /* 32-bit Thumb-2 instruction */ + else if (thumb_insn_size (insn) == 4) /* 32-bit Thumb-2 instruction */ { if (target_read_memory (scan_pc, buf, 2)) break; @@ -4335,14 +4337,9 @@ thumb_get_next_pc_raw (struct frame_info *frame, CORE_ADDR pc) int cond = itstate >> 4; if (! condition_true (cond, status)) - { - /* Advance to the next instruction. All the 32-bit - instructions share a common prefix. */ - if ((inst1 & 0xe000) == 0xe000 && (inst1 & 0x1800) != 0) - return MAKE_THUMB_ADDR (pc + 4); - else - return MAKE_THUMB_ADDR (pc + 2); - } + /* Advance to the next instruction. All the 32-bit + instructions share a common prefix. */ + return MAKE_THUMB_ADDR (pc + thumb_insn_size (inst1)); /* Otherwise, handle the instruction normally. */ } @@ -4376,7 +4373,7 @@ thumb_get_next_pc_raw (struct frame_info *frame, CORE_ADDR pc) { nextpc = pc_val + (sbits (inst1, 0, 10) << 1); } - else if ((inst1 & 0xe000) == 0xe000) /* 32-bit instruction */ + else if (thumb_insn_size (inst1) == 4) /* 32-bit instruction */ { unsigned short inst2; inst2 = read_memory_unsigned_integer (pc + 2, 2, byte_order_for_code); @@ -8473,7 +8470,7 @@ arm_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr, int *lenptr) { unsigned short inst1; inst1 = extract_unsigned_integer (buf, 2, byte_order_for_code); - if ((inst1 & 0xe000) == 0xe000 && (inst1 & 0x1800) != 0) + if (thumb_insn_size (inst1) == 4) { *lenptr = tdep->thumb2_breakpoint_size; return tdep->thumb2_breakpoint;