Re: [rfc] Fix infrun.c fall-out from the software single-step logic change

Ulrich Weigand
2007-06-21T00:11:13Z

> this fixes one outstanding issue after the ppc software single-step patch.

Here's a slightly modified version of that patch; it fixes some errors in
the comment wording, and also removes the SOFTWARE_SINGLE_STEP target macro
(the last one remaining!).

Tested on powerpc64-linux.

Bye,
Ulrich

ChangeLog:

* infrun.c (adjust_pc_after_break): Do not assume software single-step
is always active if SOFTWARE_SINGLE_STEP_P is true.
(resume): Use gdbarch_software_single_step[_p] instead of
SOFTWARE_SINGLE_STEP[_P].
(handle_inferior_event): Do not check for SOFTWARE_SINGLE_STEP_P.

* gdbarch.sh (software_single_step): Remove target macro.
* gdbarch.h, gdbarch.c: Regenerate.

diff -urNp gdb-orig/gdb/gdbarch.c gdb-head/gdb/gdbarch.c
--- gdb-orig/gdb/gdbarch.c 2007-06-21 00:56:50.032443000 +0200
+++ gdb-head/gdb/gdbarch.c 2007-06-21 00:57:56.924903103 +0200
@@ -976,21 +976,9 @@ gdbarch_dump (struct gdbarch *current_gd
fprintf_unfiltered (file,
"gdbarch_dump: smash_text_address = <0x%lx>\n",
(long) current_gdbarch->smash_text_address);
-#ifdef SOFTWARE_SINGLE_STEP_P
- fprintf_unfiltered (file,
- "gdbarch_dump: %s # %s\n",
- "SOFTWARE_SINGLE_STEP_P()",
- XSTRING (SOFTWARE_SINGLE_STEP_P ()));
-#endif
fprintf_unfiltered (file,
"gdbarch_dump: gdbarch_software_single_step_p() = %d\n",
gdbarch_software_single_step_p (current_gdbarch));
-#ifdef SOFTWARE_SINGLE_STEP
- fprintf_unfiltered (file,
- "gdbarch_dump: %s # %s\n",
- "SOFTWARE_SINGLE_STEP(frame)",
- XSTRING (SOFTWARE_SINGLE_STEP (frame)));
-#endif
fprintf_unfiltered (file,
"gdbarch_dump: software_single_step = <0x%lx>\n",
(long) current_gdbarch->software_single_step);
diff -urNp gdb-orig/gdb/gdbarch.h gdb-head/gdb/gdbarch.h
--- gdb-orig/gdb/gdbarch.h 2007-06-21 00:56:50.074437000 +0200
+++ gdb-head/gdb/gdbarch.h 2007-06-21 00:57:56.967896904 +0200
@@ -526,30 +526,11 @@ extern void set_gdbarch_smash_text_addre
A return value of 1 means that the software_single_step breakpoints
were inserted; 0 means they were not. */

-#if defined (SOFTWARE_SINGLE_STEP)
-/* Legacy for systems yet to multi-arch SOFTWARE_SINGLE_STEP */
-#if !defined (SOFTWARE_SINGLE_STEP_P)
-#define SOFTWARE_SINGLE_STEP_P() (1)
-#endif
-#endif
-
extern int gdbarch_software_single_step_p (struct gdbarch *gdbarch);
-#if !defined (GDB_TM_FILE) && defined (SOFTWARE_SINGLE_STEP_P)
-#error "Non multi-arch definition of SOFTWARE_SINGLE_STEP"
-#endif
-#if !defined (SOFTWARE_SINGLE_STEP_P)
-#define SOFTWARE_SINGLE_STEP_P() (gdbarch_software_single_step_p (current_gdbarch))
-#endif

typedef int (gdbarch_software_single_step_ftype) (struct frame_info *frame);
extern int gdbarch_software_single_step (struct gdbarch *gdbarch, struct frame_info *frame);
extern void set_gdbarch_software_single_step (struct gdbarch *gdbarch, gdbarch_software_single_step_ftype *software_single_step);
-#if !defined (GDB_TM_FILE) && defined (SOFTWARE_SINGLE_STEP)
-#error "Non multi-arch definition of SOFTWARE_SINGLE_STEP"
-#endif
-#if !defined (SOFTWARE_SINGLE_STEP)
-#define SOFTWARE_SINGLE_STEP(frame) (gdbarch_software_single_step (current_gdbarch, frame))
-#endif

/* Return non-zero if the processor is executing a delay slot and a
further single-step is needed before the instruction finishes. */
diff -urNp gdb-orig/gdb/gdbarch.sh gdb-head/gdb/gdbarch.sh
--- gdb-orig/gdb/gdbarch.sh 2007-06-21 00:56:50.082436000 +0200
+++ gdb-head/gdb/gdbarch.sh 2007-06-21 00:57:57.012890417 +0200
@@ -582,7 +582,7 @@ f::CORE_ADDR:smash_text_address:CORE_ADD
#
# A return value of 1 means that the software_single_step breakpoints
# were inserted; 0 means they were not.
-F:=:int:software_single_step:struct frame_info *frame:frame
+F::int:software_single_step:struct frame_info *frame:frame

# Return non-zero if the processor is executing a delay slot and a
# further single-step is needed before the instruction finishes.
diff -urNp gdb-orig/gdb/infrun.c gdb-head/gdb/infrun.c
--- gdb-orig/gdb/infrun.c 2007-06-21 00:56:50.132429000 +0200
+++ gdb-head/gdb/infrun.c 2007-06-21 00:57:57.060883498 +0200
@@ -537,10 +537,10 @@ how to step past a permanent breakpoint
a command like `return' or `jump' to continue execution."));
}

- if (SOFTWARE_SINGLE_STEP_P () && step)
+ if (step && gdbarch_software_single_step_p (current_gdbarch))
{
/* Do it the hard way, w/temp breakpoints */
- if (SOFTWARE_SINGLE_STEP (get_current_frame ()))
+ if (gdbarch_software_single_step (current_gdbarch, get_current_frame ()))
{
/* ...and don't ask hardware to do it. */
step = 0;
@@ -1184,53 +1184,34 @@ adjust_pc_after_break (struct execution_
breakpoint_pc = read_pc_pid (ecs->ptid) - gdbarch_decr_pc_after_break
(current_gdbarch);

- if (SOFTWARE_SINGLE_STEP_P ())
- {
- /* When using software single-step, a SIGTRAP can only indicate
- an inserted breakpoint. This actually makes things
- easier. */
- if (singlestep_breakpoints_inserted_p)
- /* When software single stepping, the instruction at [prev_pc]
- is never a breakpoint, but the instruction following
- [prev_pc] (in program execution order) always is. Assume
- that following instruction was reached and hence a software
- breakpoint was hit. */
- write_pc_pid (breakpoint_pc, ecs->ptid);
- else if (software_breakpoint_inserted_here_p (breakpoint_pc))
- /* The inferior was free running (i.e., no single-step
- breakpoints inserted) and it hit a software breakpoint. */
+ /* Check whether there actually is a software breakpoint inserted
+ at that location. */
+ if (software_breakpoint_inserted_here_p (breakpoint_pc))
+ {
+ /* When using hardware single-step, a SIGTRAP is reported for both
+ a completed single-step and a software breakpoint. Need to
+ differentiate between the two, as the latter needs adjusting
+ but the former does not.
+
+ The SIGTRAP can be due to a completed hardware single-step only if
+ - we didn't insert software single-step breakpoints
+ - the thread to be examined is still the current thread
+ - this thread is currently being stepped
+
+ If any of these events did not occur, we must have stopped due
+ to hitting a software breakpoint, and have to back up to the
+ breakpoint address.
+
+ As a special case, we could have hardware single-stepped a
+ software breakpoint. In this case (prev_pc == breakpoint_pc),
+ we also need to back up to the breakpoint address. */
+
+ if (singlestep_breakpoints_inserted_p
+ || !ptid_equal (ecs->ptid, inferior_ptid)
+ || !currently_stepping (ecs)
+ || prev_pc == breakpoint_pc)
write_pc_pid (breakpoint_pc, ecs->ptid);
}
- else
- {
- /* When using hardware single-step, a SIGTRAP is reported for
- both a completed single-step and a software breakpoint. Need
- to differentiate between the two as the latter needs
- adjusting but the former does not.
-
- When the thread to be examined does not match the current thread
- context we can't use currently_stepping, so assume no
- single-stepping in this case. */
- if (ptid_equal (ecs->ptid, inferior_ptid) && currently_stepping (ecs))
- {
- if (prev_pc == breakpoint_pc
- && software_breakpoint_inserted_here_p (breakpoint_pc))
- /* Hardware single-stepped a software breakpoint (as
- occures when the inferior is resumed with PC pointing
- at not-yet-hit software breakpoint). Since the
- breakpoint really is executed, the inferior needs to be
- backed up to the breakpoint address. */
- write_pc_pid (breakpoint_pc, ecs->ptid);
- }
- else
- {
- if (software_breakpoint_inserted_here_p (breakpoint_pc))
- /* The inferior was free running (i.e., no hardware
- single-step and no possibility of a false SIGTRAP) and
- hit a software breakpoint. */
- write_pc_pid (breakpoint_pc, ecs->ptid);
- }
- }
}

/* Given an execution control state that has been freshly filled in
@@ -1373,7 +1354,7 @@ handle_inferior_event (struct execution_
(LONGEST) ecs->ws.value.integer));
gdb_flush (gdb_stdout);
target_mourn_inferior ();
- singlestep_breakpoints_inserted_p = 0; /* SOFTWARE_SINGLE_STEP_P() */
+ singlestep_breakpoints_inserted_p = 0;
stop_print_frame = 0;
stop_stepping (ecs);
return;
@@ -1393,7 +1374,7 @@ handle_inferior_event (struct execution_
target_mourn_inferior ();

print_stop_reason (SIGNAL_EXITED, stop_signal);
- singlestep_breakpoints_inserted_p = 0; /* SOFTWARE_SINGLE_STEP_P() */
+ singlestep_breakpoints_inserted_p = 0;
stop_stepping (ecs);
return;

@@ -1549,8 +1530,7 @@ handle_inferior_event (struct execution_

if (stepping_past_singlestep_breakpoint)
{
- gdb_assert (SOFTWARE_SINGLE_STEP_P ()
- && singlestep_breakpoints_inserted_p);
+ gdb_assert (singlestep_breakpoints_inserted_p);
gdb_assert (ptid_equal (singlestep_ptid, ecs->ptid));
gdb_assert (!ptid_equal (singlestep_ptid, saved_singlestep_ptid));

@@ -1599,7 +1579,7 @@ handle_inferior_event (struct execution_
if (!breakpoint_thread_match (stop_pc, ecs->ptid))
thread_hop_needed = 1;
}
- else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+ else if (singlestep_breakpoints_inserted_p)
{
/* We have not context switched yet, so this should be true
no matter which thread hit the singlestep breakpoint. */
@@ -1670,7 +1650,7 @@ handle_inferior_event (struct execution_
/* Saw a breakpoint, but it was hit by the wrong thread.
Just continue. */

- if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+ if (singlestep_breakpoints_inserted_p)
{
/* Pull the single step breakpoints out of the target. */
remove_single_step_breakpoints ();
@@ -1719,7 +1699,7 @@ handle_inferior_event (struct execution_
return;
}
}
- else if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+ else if (singlestep_breakpoints_inserted_p)
{
sw_single_step_trap_p = 1;
ecs->random_signal = 0;
@@ -1741,7 +1721,7 @@ handle_inferior_event (struct execution_
deprecated_context_hook (pid_to_thread_id (ecs->ptid));
}

- if (SOFTWARE_SINGLE_STEP_P () && singlestep_breakpoints_inserted_p)
+ if (singlestep_breakpoints_inserted_p)
{
/* Pull the single step breakpoints out of the target. */
remove_single_step_breakpoints ();