From: Yao Qi Date: Mon, 25 Apr 2016 08:16:21 +0000 (+0100) Subject: Force to insert software single step breakpoint X-Git-Tag: binutils-2_27~660 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=21edc42f4e1ec6fe8cfce171232bab27ad4af372;p=external%2Fbinutils.git 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. --- diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 91db2e3..0b08605 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 f99a7ab..a39a15c 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 252fc4b..859ba85 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 37f59b7..c8787c2 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 696105d..cfb1d06 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. */ @@ -1365,6 +1371,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) { return step_over_info.nonsteppable_watchpoint_p; @@ -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 61d3b20..e79bf2d 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);