From 5514706f739978e0897d01ec9fa25fa61861c263 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Tue, 7 Apr 2020 11:01:38 -0400 Subject: [PATCH] gdb: don't require displaced step copy_insn to be implemented in prepare/finish are The gdbarch verification currently requires that the gdbarch displaced_step_prepare and displaced_step_finish methods are provided if displaced_step_copy_insn is (and not provided if it isn't). displaced_step_copy_insn is used by the multiple_displaced_buffer_manager class, but a gdbarch could very well decide to implement prepare and finish without that, and without requiring copy_insn. Remove the dependency in the gdbarch verification between prepare/finish and copy_insn. Now, prepare and finish must be both provided or both not-provided. Since multiple_displaced_buffer_manager uses the `copy_insn` and `fixup` gdbarch methods, it now asserts that they are present for the thread architecture in its `prepare` method. --- gdb/displaced-stepping.c | 16 +++++++++++++--- gdb/gdbarch.c | 16 ++++++++++++---- gdb/gdbarch.h | 2 ++ gdb/gdbarch.sh | 4 ++-- gdb/infrun.c | 7 +++---- 5 files changed, 32 insertions(+), 13 deletions(-) diff --git a/gdb/displaced-stepping.c b/gdb/displaced-stepping.c index 5ada163225..3fafeb2bf9 100644 --- a/gdb/displaced-stepping.c +++ b/gdb/displaced-stepping.c @@ -13,14 +13,25 @@ displaced_step_copy_insn_closure::~displaced_step_copy_insn_closure() = default; displaced_step_prepare_status multiple_displaced_buffer_manager::prepare (thread_info *thread) { + gdb_assert (thread != nullptr); + + gdbarch *arch = thread->arch (); + + /* This class requires that the arch implements both copy_insn and fixup. */ + gdb_assert (gdbarch_displaced_step_copy_insn_p (arch)); + gdb_assert (gdbarch_displaced_step_fixup_p (arch)); + + /* It's invalid to `prepare` a thread that already has a displaced step in + progress. */ gdb_assert (!thread->displaced_step_state.in_progress ()); - displaced_step_buffer_state *buffer = nullptr; - /* Sanity check. */ + /* Sanity check: no buffer is currently assigned to this thread. */ for (displaced_step_buffer_state &buf : m_buffers) gdb_assert (buf.m_current_thread != thread); /* Search for an unused buffer. */ + displaced_step_buffer_state *buffer = nullptr; + for (displaced_step_buffer_state &candidate : m_buffers) { if (candidate.m_current_thread == nullptr) @@ -33,7 +44,6 @@ multiple_displaced_buffer_manager::prepare (thread_info *thread) if (buffer == nullptr) return DISPLACED_STEP_PREPARE_STATUS_UNAVAILABLE; - gdbarch *arch = thread->arch (); if (debug_displaced) fprintf_unfiltered (gdb_stdlog, "displaced: selected buffer at %s\n", diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index e7d94a3b47..597e10735c 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -443,7 +443,6 @@ gdbarch_alloc (const struct gdbarch_info *info, gdbarch->skip_permanent_breakpoint = default_skip_permanent_breakpoint; gdbarch->displaced_step_hw_singlestep = default_displaced_step_hw_singlestep; gdbarch->displaced_step_fixup = NULL; - gdbarch->displaced_step_prepare = NULL; gdbarch->displaced_step_finish = NULL; gdbarch->relocate_instruction = NULL; gdbarch->has_shared_address_space = default_has_shared_address_space; @@ -655,9 +654,8 @@ verify_gdbarch (struct gdbarch *gdbarch) /* Skip verify of displaced_step_copy_insn, has predicate. */ /* Skip verify of displaced_step_hw_singlestep, invalid_p == 0 */ /* Skip verify of displaced_step_fixup, has predicate. */ - if ((! gdbarch->displaced_step_prepare) != (! gdbarch->displaced_step_copy_insn)) - log.puts ("\n\tdisplaced_step_prepare"); - if ((! gdbarch->displaced_step_finish) != (! gdbarch->displaced_step_copy_insn)) + /* Skip verify of displaced_step_prepare, has predicate. */ + if ((! gdbarch->displaced_step_finish) != (! gdbarch->displaced_step_prepare)) log.puts ("\n\tdisplaced_step_finish"); /* Skip verify of relocate_instruction, has predicate. */ /* Skip verify of overlay_update, has predicate. */ @@ -927,6 +925,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file) fprintf_unfiltered (file, "gdbarch_dump: displaced_step_hw_singlestep = <%s>\n", host_address_to_string (gdbarch->displaced_step_hw_singlestep)); + fprintf_unfiltered (file, + "gdbarch_dump: gdbarch_displaced_step_prepare_p() = %d\n", + gdbarch_displaced_step_prepare_p (gdbarch)); fprintf_unfiltered (file, "gdbarch_dump: displaced_step_prepare = <%s>\n", host_address_to_string (gdbarch->displaced_step_prepare)); @@ -3992,6 +3993,13 @@ set_gdbarch_displaced_step_fixup (struct gdbarch *gdbarch, gdbarch->displaced_step_fixup = displaced_step_fixup; } +int +gdbarch_displaced_step_prepare_p (struct gdbarch *gdbarch) +{ + gdb_assert (gdbarch != NULL); + return gdbarch->displaced_step_prepare != NULL; +} + displaced_step_prepare_status gdbarch_displaced_step_prepare (struct gdbarch *gdbarch, thread_info *thread) { diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h index a870304167..b6652ac565 100644 --- a/gdb/gdbarch.h +++ b/gdb/gdbarch.h @@ -1079,6 +1079,8 @@ extern void set_gdbarch_displaced_step_fixup (struct gdbarch *gdbarch, gdbarch_d m;CORE_ADDR;displaced_step_location;thread_info *;thread;;NULL;;(! gdbarch->displaced_step_location) != (! gdbarch->displaced_step_copy_insn) m;CORE_ADDR;displaced_step_release_location;CORE_ADDR;addr;;NULL;;(! gdbarch->displaced_step_location) != (! gdbarch->displaced_step_copy_insn) */ +extern int gdbarch_displaced_step_prepare_p (struct gdbarch *gdbarch); + typedef displaced_step_prepare_status (gdbarch_displaced_step_prepare_ftype) (struct gdbarch *gdbarch, thread_info *thread); extern displaced_step_prepare_status gdbarch_displaced_step_prepare (struct gdbarch *gdbarch, thread_info *thread); extern void set_gdbarch_displaced_step_prepare (struct gdbarch *gdbarch, gdbarch_displaced_step_prepare_ftype *displaced_step_prepare); diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh index 10332ea282..7d61a510af 100755 --- a/gdb/gdbarch.sh +++ b/gdb/gdbarch.sh @@ -824,8 +824,8 @@ M;void;displaced_step_fixup;struct displaced_step_copy_insn_closure *closure, CO # see the comments in infrun.c. #m;CORE_ADDR;displaced_step_location;thread_info *;thread;;NULL;;(! gdbarch->displaced_step_location) != (! gdbarch->displaced_step_copy_insn) #m;CORE_ADDR;displaced_step_release_location;CORE_ADDR;addr;;NULL;;(! gdbarch->displaced_step_location) != (! gdbarch->displaced_step_copy_insn) -m;displaced_step_prepare_status;displaced_step_prepare;thread_info *thread;thread;;NULL;;(! gdbarch->displaced_step_prepare) != (! gdbarch->displaced_step_copy_insn) -m;displaced_step_finish_status;displaced_step_finish;thread_info *thread, gdb_signal sig;thread, sig;;NULL;;(! gdbarch->displaced_step_finish) != (! gdbarch->displaced_step_copy_insn) +M;displaced_step_prepare_status;displaced_step_prepare;thread_info *thread;thread +m;displaced_step_finish_status;displaced_step_finish;thread_info *thread, gdb_signal sig;thread, sig;;NULL;;(! gdbarch->displaced_step_finish) != (! gdbarch->displaced_step_prepare) # Relocate an instruction to execute at a different address. OLDLOC # is the address in the inferior memory where the instruction to diff --git a/gdb/infrun.c b/gdb/infrun.c index 7a1f9351e8..90d5b2717e 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1609,10 +1609,9 @@ show_can_use_displaced_stepping (struct ui_file *file, int from_tty, static bool gdbarch_supports_displaced_stepping (gdbarch *arch) { - /* Only check for the presence of copy_insn. Other required methods - are checked by the gdbarch validation to be provided if copy_insn is - provided. */ - return gdbarch_displaced_step_copy_insn_p (arch); + /* Only check for the presence of `prepare`. `finish` is required by the + gdbarch verification to be provided if `prepare` is provided. */ + return gdbarch_displaced_step_prepare_p (arch); } /* Return non-zero if displaced stepping can/should be used to step -- 2.34.1