From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27586 invoked by alias); 24 Jun 2011 10:39:10 -0000 Received: (qmail 27576 invoked by uid 22791); 24 Jun 2011 10:39:09 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 24 Jun 2011 10:38:44 +0000 Received: (qmail 16026 invoked from network); 24 Jun 2011 10:38:42 -0000 Received: from unknown (HELO ?192.168.0.101?) (yao@127.0.0.2) by mail.codesourcery.com with ESMTPA; 24 Jun 2011 10:38:42 -0000 Message-ID: <4E04692F.3030500@codesourcery.com> Date: Fri, 24 Jun 2011 10:39:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.17) Gecko/20110424 Lightning/1.0b2 Thunderbird/3.1.10 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: [PATCH] Fix that different function breakpoints are set at same pc address (PR gdb/12703) References: <000001cc3216$b96ba290$2c42e7b0$@guo@arm.com> <4E040A9A.5020807@codesourcery.com> <201106240959.08358.pedro@codesourcery.com> In-Reply-To: <201106240959.08358.pedro@codesourcery.com> Content-Type: multipart/mixed; boundary="------------080406010304020206080409" 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-06/txt/msg00370.txt.bz2 This is a multi-part message in MIME format. --------------080406010304020206080409 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-length: 2620 On 06/24/2011 04:59 PM, Pedro Alves wrote: > On Friday 24 June 2011 04:55:06, Yao Qi wrote: >> On 06/24/2011 10:30 AM, Terry Guo wrote: > >> >> IMO, this is a target-specific bug, so this PR's component should be >> tdept, so it should be "PR tdept/12703" instead of "PR gdb/12703". >> >> I'd move your test cases break-function.{c,exp} to gdb.arch/ dir, >> because it is target-dependent fix. I am sure this case is useful to >> other ports. > > The testcase might help catch the same issue in other archs. > IMO, it should stay generic if possible. > OK. Let us leave it in gdb.base. I suggest that test case can be renamed to reflect what we want to test here, such as "break-outside-function.exp". > I agree with Yao when he says in the PR that there seems to be > some other root cause for the bug. Shouldn't > thumb_instruction_changes_pc have caught that "b.n" ? > > 00008160 : > 8160: e7fe b.n 8160 > ... > > 00008164 : > 8164: 4a05 ldr r2, [pc, #20] ; (817c ) > thumb_instruction_changes_pc can handle "b.n". AFAICS, the problem is in thumb_analyze_prologue. In thumb_analyze_prologue, there are a lot if/else branches, like below, else if ((insn & 0xe000) == 0xe000) // <-- [1] { .... else if (thumb2_instruction_changes_pc (insn, inst2)) { /* Don't scan past anything that might change control flow. */ break; } else { /* The optimizer might shove anything into the prologue, so we just skip what we don't recognize. */ unrecognized_pc = start; } start += 2; } else if (thumb_instruction_changes_pc (insn)) { /* Don't scan past anything that might change control flow. */ break; } The instruction "b.n 8160" is 0xe7fe, so condition check [1] is true, and thumb_instruction_changes_pc is unreachable. This is cause of this problem, I doubt. The line of code [1] is discussed in this patch [rfa] ARM prologue parsing support for Thumb-2 instructions http://sourceware.org/ml/gdb-patches/2010-10/msg00132.html IIUC, condition check [1] is for 32-bit Thumb-2 instructions (I may be wrong, of course). I have an untested patch. >>> +void foo(void) >> >> This doesn't comply to GNU coding standard. Please move "foo ()" to >> next line. > > Note that test code does not strictly _need_ to follow the > coding standards. Though it's indeed nice when it does. > GDB should be able to debug non-GNU code too. :-) > Oh, I don't know that. Sorry about the noise I made here. -- Yao (齐尧) --------------080406010304020206080409 Content-Type: text/x-patch; name="thumb_skip_prologue.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="thumb_skip_prologue.patch" Content-length: 647 gdb/ * arm-tdep.c (thumb_analyze_prologue): Check condition for 32-bit Thumb-2 instructions. diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c index 2dd8c9e..7f5a0e1 100644 --- a/gdb/arm-tdep.c +++ b/gdb/arm-tdep.c @@ -832,8 +832,9 @@ 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 ((insn & 0xe000) == 0xe000 && (insn & 0x1800) != 0) { + /* 32-bit Thumb-2 instructions. */ unsigned short inst2; inst2 = read_memory_unsigned_integer (start + 2, 2, --------------080406010304020206080409--