From 21edc42f4e1ec6fe8cfce171232bab27ad4af372 Mon Sep 17 00:00:00 2001 From: Yao Qi Date: Mon, 25 Apr 2016 09:16:21 +0100 Subject: [PATCH] Force to insert software single step breakpoint GDB doesn't insert software single step breakpoint if the instruction branches to itself, so that the program can't stop after command "si". (gdb) b 32 Breakpoint 2 at 0x8680: file git/gdb/testsuite/gdb.base/branch-to-self.c, line 32. (gdb) c Continuing. Breakpoint 2, main () at gdb/git/gdb/testsuite/gdb.base/branch-to-self.c:32 32 asm (".Lhere: " BRANCH_INSN " .Lhere"); /* loop-line */ (gdb) si infrun: clear_proceed_status_thread (Thread 3991.3991) infrun: proceed (addr=0xffffffff, signal=GDB_SIGNAL_DEFAULT) infrun: step-over queue now empty infrun: resuming [Thread 3991.3991] for step-over infrun: skipping breakpoint: stepping past insn at: 0x8680 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sending packet: $Z0,8678,4#f3...Packet received: OK infrun: skipping breakpoint: stepping past insn at: 0x8680 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Sending packet: $Z0,b6fe86c8,4#82...Packet received: OK infrun: resume (step=1, signal=GDB_SIGNAL_0), trap_expected=1, current thread [Thread 3991.3991] at 0x868 breakpoint.c:should_be_inserted thinks the breakpoint shouldn't be inserted, which is wrong. This patch restrict the condition that only skip the non-single-step breakpoints if they are inserted at the place we are stepping over, however we don't want to skip single-step breakpoint if its thread is the thread we are stepping over, so in this patch, I add a thread num in 'struct step_over_info' to record the thread we're stepping over. gdb: 2016-04-25 Yao Qi * breakpoint.c (should_be_inserted): Return 0 if the location's owner is not single step breakpoint or single step breakpoint's thread isn't the thread which is stepping past a breakpoint. * gdbarch.sh (software_single_step): Update comments. * gdbarch.h: Regenerated. * infrun.c (struct step_over_info) : New field. (set_step_over_info): New argument 'thread'. Callers updated. (clear_step_over_info): Set field thread to -1. (thread_is_stepping_over_breakpoint): New function. * infrun.h (thread_is_stepping_over_breakpoint): Declaration. --- gdb/ChangeLog | 13 +++++++++++++ gdb/breakpoint.c | 15 +++++++++++++-- gdb/gdbarch.h | 7 ++++++- gdb/gdbarch.sh | 7 ++++++- gdb/infrun.c | 24 ++++++++++++++++++++---- gdb/infrun.h | 4 ++++ 6 files changed, 62 insertions(+), 8 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 91db2e38cb..0b08605449 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,16 @@ +2016-04-25 Yao Qi + + * breakpoint.c (should_be_inserted): Return 0 if the location's + owner is not single step breakpoint or single step breakpoint's + thread isn't the thread which is stepping past a breakpoint. + * gdbarch.sh (software_single_step): Update comments. + * gdbarch.h: Regenerated. + * infrun.c (struct step_over_info) : New field. + (set_step_over_info): New argument 'thread'. Callers updated. + (clear_step_over_info): Set field thread to -1. + (thread_is_stepping_over_breakpoint): New function. + * infrun.h (thread_is_stepping_over_breakpoint): Declaration. + 2016-04-22 Edjunior Barbosa Machado * ppc-linux-nat.c (ppc_linux_read_description): Use PPC_FEATURE_HAS_VSX diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index f99a7ab46d..a39a15c7f3 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2219,11 +2219,22 @@ should_be_inserted (struct bp_location *bl) return 0; /* Don't insert a breakpoint if we're trying to step past its - location. */ + location, except if the breakpoint is a single-step breakpoint, + and the breakpoint's thread is the thread which is stepping past + a breakpoint. */ if ((bl->loc_type == bp_loc_software_breakpoint || bl->loc_type == bp_loc_hardware_breakpoint) && stepping_past_instruction_at (bl->pspace->aspace, - bl->address)) + bl->address) + /* The single-step breakpoint may be inserted at the location + we're trying to step if the instruction branches to itself. + However, the instruction won't be executed at all and it may + break the semantics of the instruction, for example, the + instruction is a conditional branch or updates some flags. + We can't fix it unless GDB is able to emulate the instruction + or switch to displaced stepping. */ + && !(bl->owner->type == bp_single_step + && thread_is_stepping_over_breakpoint (bl->owner->thread))) { if (debug_infrun) { diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h index 252fc4bad8..859ba857cb 100644 --- a/gdb/gdbarch.h +++ b/gdb/gdbarch.h @@ -650,7 +650,12 @@ extern void set_gdbarch_addr_bits_remove (struct gdbarch *gdbarch, gdbarch_addr_ target can single step. If not, then implement single step using breakpoints. A return value of 1 means that the software_single_step breakpoints - were inserted; 0 means they were not. */ + were inserted; 0 means they were not. Multiple breakpoints may be + inserted for some instructions such as conditional branch. However, + each implementation must always evaluate the condition and only put + the breakpoint at the branch destination if the condition is true, so + that we ensure forward progress when stepping past a conditional + branch to self. */ extern int gdbarch_software_single_step_p (struct gdbarch *gdbarch); diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh index 37f59b7b93..c8787c2556 100755 --- a/gdb/gdbarch.sh +++ b/gdb/gdbarch.sh @@ -609,7 +609,12 @@ m:CORE_ADDR:addr_bits_remove:CORE_ADDR addr:addr::core_addr_identity::0 # target can single step. If not, then implement single step using breakpoints. # # A return value of 1 means that the software_single_step breakpoints -# were inserted; 0 means they were not. +# were inserted; 0 means they were not. Multiple breakpoints may be +# inserted for some instructions such as conditional branch. However, +# each implementation must always evaluate the condition and only put +# the breakpoint at the branch destination if the condition is true, so +# that we ensure forward progress when stepping past a conditional +# branch to self. F:int:software_single_step:struct frame_info *frame:frame # Return non-zero if the processor is executing a delay slot and a diff --git a/gdb/infrun.c b/gdb/infrun.c index 696105d063..cfb1d06724 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1296,6 +1296,9 @@ struct step_over_info /* The instruction being stepped over triggers a nonsteppable watchpoint. If true, we'll skip inserting watchpoints. */ int nonsteppable_watchpoint_p; + + /* The thread's global number. */ + int thread; }; /* The step-over info of the location that is being stepped over. @@ -1329,11 +1332,13 @@ static struct step_over_info step_over_info; static void set_step_over_info (struct address_space *aspace, CORE_ADDR address, - int nonsteppable_watchpoint_p) + int nonsteppable_watchpoint_p, + int thread) { step_over_info.aspace = aspace; step_over_info.address = address; step_over_info.nonsteppable_watchpoint_p = nonsteppable_watchpoint_p; + step_over_info.thread = thread; } /* Called when we're not longer stepping over a breakpoint / an @@ -1348,6 +1353,7 @@ clear_step_over_info (void) step_over_info.aspace = NULL; step_over_info.address = 0; step_over_info.nonsteppable_watchpoint_p = 0; + step_over_info.thread = -1; } /* See infrun.h. */ @@ -1364,6 +1370,15 @@ stepping_past_instruction_at (struct address_space *aspace, /* See infrun.h. */ +int +thread_is_stepping_over_breakpoint (int thread) +{ + return (step_over_info.thread != -1 + && thread == step_over_info.thread); +} + +/* See infrun.h. */ + int stepping_past_nonsteppable_watchpoint (void) { @@ -2579,7 +2594,7 @@ resume (enum gdb_signal sig) stop_all_threads (); set_step_over_info (get_regcache_aspace (regcache), - regcache_read_pc (regcache), 0); + regcache_read_pc (regcache), 0, tp->global_num); step = maybe_software_singlestep (gdbarch, pc); @@ -7750,10 +7765,11 @@ keep_going_pass_signal (struct execution_control_state *ecs) && (remove_wps || !use_displaced_stepping (ecs->event_thread))) { set_step_over_info (get_regcache_aspace (regcache), - regcache_read_pc (regcache), remove_wps); + regcache_read_pc (regcache), remove_wps, + ecs->event_thread->global_num); } else if (remove_wps) - set_step_over_info (NULL, 0, remove_wps); + set_step_over_info (NULL, 0, remove_wps, -1); /* If we now need to do an in-line step-over, we need to stop all other threads. Note this must be done before diff --git a/gdb/infrun.h b/gdb/infrun.h index 61d3b2071f..e79bf2d83d 100644 --- a/gdb/infrun.h +++ b/gdb/infrun.h @@ -133,6 +133,10 @@ extern void insert_step_resume_breakpoint_at_sal (struct gdbarch *, extern int stepping_past_instruction_at (struct address_space *aspace, CORE_ADDR address); +/* Returns true if thread whose thread number is THREAD is stepping + over a breakpoint. */ +extern int thread_is_stepping_over_breakpoint (int thread); + /* Returns true if we're trying to step past an instruction that triggers a non-steppable watchpoint. */ extern int stepping_past_nonsteppable_watchpoint (void); -- 2.34.1