[PATCH 06/12] gdb: introduce status enum for displaced step prepare/finish
Simon Marchi
simon.marchi@efficios.com
Tue Nov 10 21:46:08 GMT 2020
This is a preparatory patch to reduce the size of the diff of the
upcoming main patch. It introduces enum types for the return values of
displaced step "prepare" and "finish" operations. I find that this
expresses better the intention of the code, rather than returning
arbitrary integer values (-1, 0 and 1) which are difficult to remember.
That makes the code easier to read.
I put the new enum types in a new displaced-stepping.h file, because I
introduce that file in a later patch anyway. Putting it there avoids
having to move it later.
There is one change in behavior for displaced_step_finish: it currently
returns 0 if the thread wasn't doing a displaced step and 1 if the
thread was doing a displaced step which was executed successfully. It
turns out that this distinction is not needed by any caller, so I've
merged these two cases into "_OK", rather than adding an extra
enumerator.
gdb/ChangeLog:
* infrun.c (displaced_step_prepare_throw): Change return type to
displaced_step_prepare_status.
(displaced_step_prepare): Likewise.
(displaced_step_finish): Change return type to
displaced_step_finish_status.
(resume_1): Adjust.
(stop_all_threads): Adjust.
* displaced-stepping.h: New file.
Change-Id: I5c8fe07212cd398d5b486b5936d9d0807acd3788
---
gdb/displaced-stepping.h | 46 +++++++++++++++++++++++++
gdb/infrun.c | 72 +++++++++++++++++++++++-----------------
2 files changed, 88 insertions(+), 30 deletions(-)
create mode 100644 gdb/displaced-stepping.h
diff --git a/gdb/displaced-stepping.h b/gdb/displaced-stepping.h
new file mode 100644
index 00000000000..9c7e85c3769
--- /dev/null
+++ b/gdb/displaced-stepping.h
@@ -0,0 +1,46 @@
+/* Displaced stepping related things.
+
+ Copyright (C) 2020 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef DISPLACED_STEPPING_H
+#define DISPLACED_STEPPING_H
+
+enum displaced_step_prepare_status
+{
+ /* A displaced stepping buffer was successfully allocated and prepared. */
+ DISPLACED_STEP_PREPARE_STATUS_OK,
+
+ /* Something bad happened. */
+ DISPLACED_STEP_PREPARE_STATUS_ERROR,
+
+ /* Not enough resources are available at this time, try again later. */
+ DISPLACED_STEP_PREPARE_STATUS_UNAVAILABLE,
+};
+
+enum displaced_step_finish_status
+{
+ /* Either the instruction was stepped and fixed up, or the specified thread
+ wasn't executing a displaced step (in which case there's nothing to
+ finish). */
+ DISPLACED_STEP_FINISH_STATUS_OK,
+
+ /* The thread was executing a displaced step, but didn't complete it. */
+ DISPLACED_STEP_FINISH_STATUS_NOT_EXECUTED,
+};
+
+#endif /* DISPLACED_STEPPING_H */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index cdf198bb307..ed54c4331d7 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -19,6 +19,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "defs.h"
+#include "displaced-stepping.h"
#include "infrun.h"
#include <ctype.h>
#include "symtab.h"
@@ -1652,11 +1653,13 @@ displaced_step_dump_bytes (const gdb_byte *buf, size_t len)
Comments in the code for 'random signals' in handle_inferior_event
explain how we handle this case instead.
- Returns 1 if preparing was successful -- this thread is going to be
- stepped now; 0 if displaced stepping this thread got queued; or -1
- if this instruction can't be displaced stepped. */
+ Returns DISPLACED_STEP_PREPARE_STATUS_OK if preparing was successful -- this
+ thread is going to be stepped now; DISPLACED_STEP_PREPARE_STATUS_UNAVAILABLE
+ if displaced stepping this thread got queued; or
+ DISPLACED_STEP_PREPARE_STATUS_ERROR if this instruction can't be displaced
+ stepped. */
-static int
+static displaced_step_prepare_status
displaced_step_prepare_throw (thread_info *tp)
{
regcache *regcache = get_thread_regcache (tp);
@@ -1694,7 +1697,7 @@ displaced_step_prepare_throw (thread_info *tp)
target_pid_to_str (tp->ptid).c_str ());
global_thread_step_over_chain_enqueue (tp);
- return 0;
+ return DISPLACED_STEP_PREPARE_STATUS_UNAVAILABLE;
}
else
displaced_debug_printf ("stepping %s now",
@@ -1725,7 +1728,7 @@ displaced_step_prepare_throw (thread_info *tp)
displaced_debug_printf ("breakpoint set in scratch pad. "
"Stepping over breakpoint in-line instead.");
- return -1;
+ return DISPLACED_STEP_PREPARE_STATUS_ERROR;
}
/* Save the original contents of the copy area. */
@@ -1749,7 +1752,7 @@ displaced_step_prepare_throw (thread_info *tp)
/* The architecture doesn't know how or want to displaced step
this instruction or instruction sequence. Fallback to
stepping over the breakpoint in-line. */
- return -1;
+ return DISPLACED_STEP_PREPARE_STATUS_ERROR;
}
/* Save the information we need to fix things up if the step
@@ -1770,20 +1773,21 @@ displaced_step_prepare_throw (thread_info *tp)
displaced_debug_printf ("displaced pc to %s", paddress (gdbarch, copy));
- return 1;
+ return DISPLACED_STEP_PREPARE_STATUS_OK;
}
/* Wrapper for displaced_step_prepare_throw that disabled further
attempts at displaced stepping if we get a memory error. */
-static int
+static displaced_step_prepare_status
displaced_step_prepare (thread_info *thread)
{
- int prepared = -1;
+ displaced_step_prepare_status status
+ = DISPLACED_STEP_PREPARE_STATUS_ERROR;
try
{
- prepared = displaced_step_prepare_throw (thread);
+ status = displaced_step_prepare_throw (thread);
}
catch (const gdb_exception_error &ex)
{
@@ -1810,7 +1814,7 @@ displaced_step_prepare (thread_info *thread)
displaced_state->failed_before = 1;
}
- return prepared;
+ return status;
}
static void
@@ -1840,22 +1844,24 @@ displaced_step_restore (struct displaced_step_inferior_state *displaced,
displaced->step_copy));
}
-/* If we displaced stepped an instruction successfully, adjust
- registers and memory to yield the same effect the instruction would
- have had if we had executed it at its original address, and return
- 1. If the instruction didn't complete, relocate the PC and return
- -1. If the thread wasn't displaced stepping, return 0. */
+/* If we displaced stepped an instruction successfully, adjust registers and
+ memory to yield the same effect the instruction would have had if we had
+ executed it at its original address, and return
+ DISPLACED_STEP_PREPARE_STATUS_OK. If the instruction didn't complete,
+ relocate the PC and return DISPLACED_STEP_PREPARE_STATUS_NOT_EXECUTED.
-static int
+ If the thread wasn't displaced stepping, return
+ DISPLACED_STEP_PREPARE_STATUS_OK as well.. */
+
+static displaced_step_finish_status
displaced_step_finish (thread_info *event_thread, enum gdb_signal signal)
{
struct displaced_step_inferior_state *displaced
= get_displaced_stepping_state (event_thread->inf);
- int ret;
/* Was this event for the thread we displaced? */
if (displaced->step_thread != event_thread)
- return 0;
+ return DISPLACED_STEP_FINISH_STATUS_OK;
/* Fixup may need to read memory/registers. Switch to the thread
that we're fixing up. Also, target_stopped_by_watchpoint checks
@@ -1879,7 +1885,8 @@ displaced_step_finish (thread_info *event_thread, enum gdb_signal signal)
displaced->step_original,
displaced->step_copy,
get_thread_regcache (displaced->step_thread));
- ret = 1;
+
+ return DISPLACED_STEP_FINISH_STATUS_OK;
}
else
{
@@ -1890,10 +1897,9 @@ displaced_step_finish (thread_info *event_thread, enum gdb_signal signal)
pc = displaced->step_original + (pc - displaced->step_copy);
regcache_write_pc (regcache, pc);
- ret = -1;
- }
- return ret;
+ return DISPLACED_STEP_FINISH_STATUS_NOT_EXECUTED;
+ }
}
/* Data to be passed around while handling an event. This data is
@@ -2417,16 +2423,17 @@ resume_1 (enum gdb_signal sig)
&& sig == GDB_SIGNAL_0
&& !current_inferior ()->waiting_for_vfork_done)
{
- int prepared = displaced_step_prepare (tp);
+ displaced_step_prepare_status prepare_status
+ = displaced_step_prepare (tp);
- if (prepared == 0)
+ if (prepare_status == DISPLACED_STEP_PREPARE_STATUS_UNAVAILABLE)
{
infrun_debug_printf ("Got placed in step-over queue");
tp->control.trap_expected = 0;
return;
}
- else if (prepared < 0)
+ else if (prepare_status == DISPLACED_STEP_PREPARE_STATUS_ERROR)
{
/* Fallback to stepping over the breakpoint in-line. */
@@ -2440,7 +2447,7 @@ resume_1 (enum gdb_signal sig)
insert_breakpoints ();
}
- else if (prepared > 0)
+ else if (prepare_status == DISPLACED_STEP_PREPARE_STATUS_OK)
{
/* Update pc to reflect the new address from which we will
execute instructions due to displaced stepping. */
@@ -2448,6 +2455,9 @@ resume_1 (enum gdb_signal sig)
step = gdbarch_displaced_step_hw_singlestep (gdbarch);
}
+ else
+ gdb_assert_not_reached (_("Invalid displaced_step_prepare_status "
+ "value."));
}
/* Do we need to do it the hard way, w/temp breakpoints? */
@@ -4822,7 +4832,8 @@ stop_all_threads (void)
t->suspend.waitstatus.kind = TARGET_WAITKIND_IGNORE;
t->suspend.waitstatus_pending_p = 0;
- if (displaced_step_finish (t, GDB_SIGNAL_0) < 0)
+ if (displaced_step_finish (t, GDB_SIGNAL_0)
+ == DISPLACED_STEP_FINISH_STATUS_NOT_EXECUTED)
{
/* Add it back to the step-over queue. */
infrun_debug_printf
@@ -4850,7 +4861,8 @@ stop_all_threads (void)
sig = (event.ws.kind == TARGET_WAITKIND_STOPPED
? event.ws.value.sig : GDB_SIGNAL_0);
- if (displaced_step_finish (t, sig) < 0)
+ if (displaced_step_finish (t, sig)
+ == DISPLACED_STEP_FINISH_STATUS_NOT_EXECUTED)
{
/* Add it back to the step-over queue. */
t->control.trap_expected = 0;
--
2.28.0
More information about the Gdb-patches
mailing list