From patchwork Thu May 29 20:11:20 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Brobecker X-Patchwork-Id: 1201 Received: (qmail 4097 invoked by alias); 29 May 2014 20:11:34 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 4086 invoked by uid 89); 29 May 2014 20:11:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 29 May 2014 20:11:31 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 1917811625B; Thu, 29 May 2014 16:11:30 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id hym7kOuP3ZAe; Thu, 29 May 2014 16:11:30 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id D635E116255; Thu, 29 May 2014 16:11:29 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 24B3D4001D; Thu, 29 May 2014 13:11:29 -0700 (PDT) From: Joel Brobecker To: gdb-patches@sourceware.org Cc: palves@redhat.com Subject: [RFA/7.8] user breakpoint not inserted if software-single-step at same location Date: Thu, 29 May 2014 13:11:20 -0700 Message-Id: <1401394280-14999-1-git-send-email-brobecker@adacore.com> Hello, with the following code... 12 Nested; -- break #1 13 return I; -- break #2 14 end; (line 12 is a call to function Nested) ... we have noticed the following errorneous behavior on ppc-aix, where, after having inserted a breakpoint at line 12 and line 13, and continuing from the breakpoint at line 12, the program never stops at line 13, running away until the program terminates: % gdb -q func (gdb) b func.adb:12 Breakpoint 1 at 0x10000a24: file func.adb, line 12. (gdb) b func.adb:13 Breakpoint 2 at 0x10000a28: file func.adb, line 13. (gdb) run Starting program: /[...]/func Breakpoint 1, func () at func.adb:12 12 Nested; -- break #1 (gdb) c Continuing. [Inferior 1 (process 4128872) exited with code 02] When resuming from the first breakpoint, GDB first tries to step out of that first breakpoint. We rely on software single-stepping on this platform, and it just so happens that the address of the first software single-step breakpoint is the same as the user's breakpoint #2 (0x10000a28). So, with infrun and target traces turned on (but uninteresting traces snip'ed off), the "continue" operation looks like this: (gdb) c ### First, we insert the user breakpoints (the second one is an internal ### breakpoint on __pthread_init). The first user breakpoint is not ### inserted as we need to step out of it first. target_insert_breakpoint (0x0000000010000a28, xxx) = 0 target_insert_breakpoint (0x00000000d03f3800, xxx) = 0 ### Then we proceed with the step-out-of-breakpoint... infrun: resume (step=1, signal=GDB_SIGNAL_0), trap_expected=1, current thread [process 15335610] at 0x10000a24 ### That's when we insert the SSS breakpoints... target_insert_breakpoint (0x0000000010000a28, xxx) = 0 target_insert_breakpoint (0x00000000100009ac, xxx) = 0 ### ... then let the inferior resume... target_resume (15335610, continue, 0) infrun: wait_for_inferior () target_wait (-1, status, options={}) = 15335610, status->kind = stopped, signal = GDB_SIGNAL_TRAP infrun: target_wait (-1, status) = infrun: 15335610 [process 15335610], infrun: status->kind = stopped, signal = GDB_SIGNAL_TRAP infrun: infwait_normal_state infrun: TARGET_WAITKIND_STOPPED infrun: stop_pc = 0x100009ac ### At this point, we stopped at the second SSS breakpoint... target_stopped_by_watchpoint () = 0 ### We remove the SSS breakpoints... target_remove_breakpoint (0x0000000010000a28, xxx) = 0 target_remove_breakpoint (0x00000000100009ac, xxx) = 0 target_stopped_by_watchpoint () = 0 ### We find that we're not done, so we resume.... infrun: no stepping, continue ### And thus insert the user breakpoints again, except we're not ### inserting the second breakpoint?!? target_insert_breakpoint (0x0000000010000a24, xxx) = 0 infrun: resume (step=0, signal=GDB_SIGNAL_0), trap_expected=0, current thread [process 15335610] at 0x100009ac target_resume (-1, continue, 0) infrun: prepare_to_wait target_wait (-1, status, options={}) = 15335610, status->kind = exited, status = 2 What happens is that the removal of the software single-step breakpoints effectively removed the breakpoint instruction from inferior memory. But because such breakpoints are inserted directly as raw breakpoints rather than through the normal chain of breakpoints, we fail to notice that one of the user breakpoints points to the same address and that this user breakpoint is therefore effectively un-inserted. When resuming after the single-step, GDB thinks that the user breakpoint is still inserted and therefore does not need to insert it again. This patch fixes the problem by avoiding the creation of a raw breakpoint for any of the software single-step breakpoints when their address correspond to a user breakpoint which has already been inserted and whose address matches. The rest of the patch adjusts the code removing the software single-step breakpoints to take this possibility into account. As it happens, it simplifies the code a little bit. This patch does make one assumption, however, which is the fact that user breakpoints get inserted before software single-step breakpoints. I think this is a fair assumption, as software single-step breakpoints get created as part of the target-step operation (resume with step=1), which logically can only happen after we've inserted all breakpoints. For the record, this behavior started appearing after commit 31e77af205cf6564c2bf4c18400b4ca16bdf92cd (PR breakpoints/7143 - Watchpoint does not trigger when first set) was applied, but I have not checked whether it was really directly related to that commit, or a latent issue. gdb/ChangeLog: PR breakpoints/7143 * breakpoint.c (non_sss_software_breakpoint_inserted_here_p): New function, extracted from software_breakpoint_inserted_here_p. (software_breakpoint_inserted_here_p): Remove factored out code by call to non_sss_software_breakpoint_inserted_here_p. (insert_single_step_breakpoint): Do nothing if a non-software- single-step breakpoint was already inserted at the same address. (remove_single_step_breakpoints): Adjust to take into account the fact that the first software single-step may not have been inserted. Tested on ppc-aix with AdaCore's testsuite. Tested on x86_64-linux with the official testsuite. OK to commit? Also, due to the nature of the regression (compared to 7.7), I think we should wait for a resolution of this issue before creating the gdb 7.8 release branch. Thanks! diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 676c7b8..f9dc413 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -4153,12 +4153,12 @@ breakpoint_inserted_here_p (struct address_space *aspace, CORE_ADDR pc) return 0; } -/* This function returns non-zero iff there is a software breakpoint - inserted at PC. */ +/* Ignoring software single-step breakpoints, return non-zero iff + there is a software breakpoint inserted at PC. */ -int -software_breakpoint_inserted_here_p (struct address_space *aspace, - CORE_ADDR pc) +static int +non_sss_software_breakpoint_inserted_here_p (struct address_space *aspace, + CORE_ADDR pc) { struct bp_location *bl, **blp_tmp; @@ -4180,6 +4180,19 @@ software_breakpoint_inserted_here_p (struct address_space *aspace, } } + return 0; +} + +/* This function returns non-zero iff there is a software breakpoint + inserted at PC. */ + +int +software_breakpoint_inserted_here_p (struct address_space *aspace, + CORE_ADDR pc) +{ + if (non_sss_software_breakpoint_inserted_here_p (aspace, pc)) + return 1; + /* Also check for software single-step breakpoints. */ if (single_step_breakpoint_inserted_here_p (aspace, pc)) return 1; @@ -15149,6 +15162,13 @@ insert_single_step_breakpoint (struct gdbarch *gdbarch, { void **bpt_p; + /* If a non-software-single-step breakpoint is inserted at the same + location as our next_pc, no need to insert an additional + raw breakpoint. */ + + if (non_sss_software_breakpoint_inserted_here_p (aspace, next_pc)) + return; + if (single_step_breakpoints[0] == NULL) { bpt_p = &single_step_breakpoints[0]; @@ -15189,22 +15209,18 @@ single_step_breakpoints_inserted (void) void remove_single_step_breakpoints (void) { - gdb_assert (single_step_breakpoints[0] != NULL); - - /* See insert_single_step_breakpoint for more about this deprecated - call. */ - deprecated_remove_raw_breakpoint (single_step_gdbarch[0], - single_step_breakpoints[0]); - single_step_gdbarch[0] = NULL; - single_step_breakpoints[0] = NULL; + int i; - if (single_step_breakpoints[1] != NULL) - { - deprecated_remove_raw_breakpoint (single_step_gdbarch[1], - single_step_breakpoints[1]); - single_step_gdbarch[1] = NULL; - single_step_breakpoints[1] = NULL; - } + for (i = 0; i < 2; i++) + if (single_step_breakpoints[i] != NULL) + { + /* See insert_single_step_breakpoint for more about + this deprecated call. */ + deprecated_remove_raw_breakpoint (single_step_gdbarch[i], + single_step_breakpoints[i]); + single_step_gdbarch[i] = NULL; + single_step_breakpoints[i] = NULL; + } } /* Delete software single step breakpoints without removing them from