From 0fdd06aff3439fb5769f485f465f15cc2c6f7841 Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Fri, 30 May 2014 11:27:19 -0700 Subject: [PATCH] User breakpoint ignored if software-single-step at same location 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 skipping the removal of the software single-step raw breakpoints if at least one non-software-single-step breakpoint is still inserted at that location. Additionally, this patch also avoids inserting the single-step breakpoint when possible, mostly to avoid issues with targets not supporting multiple breakpoints at same address. For instance, some remote stubs might not support multiple Z0 packets at the same address. gdb/ChangeLog: PR breakpoints/17000 * 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 the target does not support target-side breakpoint condition evaluation, a 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. Do not remove the raw breakpoint is a user software breakpoint is still inserted at the same location. Tested on ppc-aix with AdaCore's testsuite. Tested on x86_64-linux with the official testsuite. Also tested on x86_64-linux through Pedro's branch enabling software single-stepping on that platform. --- gdb/breakpoint.c | 73 ++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 20 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 676c7b8..bddbca0 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,20 @@ insert_single_step_breakpoint (struct gdbarch *gdbarch, { void **bpt_p; + /* Unless the target supports target-side evaluation of breakpoint + conditions, there is no need to insert an additional raw breakpoint + if a non-software-single-step breakpoint is already inserted + at that location. + + For targets supporting target-side evaluation of breakpoint + conditions, the issue is that those breakpoints cannot be + relied on to trigger during our software-single-step operation. + So we have to insert our own to make sure. */ + + if (!target_supports_evaluation_of_breakpoint_conditions () + && 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 +15216,28 @@ 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) + { + struct bp_target_info *bp_tgt = single_step_breakpoints[i]; + CORE_ADDR pc = bp_tgt->placed_address; + struct address_space *aspace = bp_tgt->placed_address_space; + + /* Only remove the raw breakpoint if there are no other + non-software-single-step breakpoints still inserted + at this location. Otherwise, we would be effectively + disabling those breakpoints. */ + if (!non_sss_software_breakpoint_inserted_here_p (aspace, pc)) + { + /* See insert_single_step_breakpoint for more about + this deprecated call. */ + deprecated_remove_raw_breakpoint (single_step_gdbarch[i], bp_tgt); + } + single_step_gdbarch[i] = NULL; + single_step_breakpoints[i] = NULL; + } } /* Delete software single step breakpoints without removing them from -- 1.9.1