gdb: add gdbarch::displaced_step_buffer_length

The gdbarch::max_insn_length field is used mostly to support displaced
stepping; it controls the size of the buffers allocated for the
displaced-step instruction, and is also used when first copying the
instruction, and later, when fixing up the instruction, in order to
read in and parse the instruction being stepped.

However, it has started to be used in other places in GDB, for
example, it's used in the Python disassembler API, and it is used on
amd64 as part of branch-tracing instruction classification.

The problem is that the value assigned to max_insn_length is not
always the maximum instruction length, but sometimes is a multiple of
that length, as required to support displaced stepping, see rs600,
ARM, and AArch64 for examples of this.

It seems to me that we are overloading the meaning of the
max_insn_length field, and I think that could potentially lead to
confusion.

I propose that we add a new gdbarch field,
gdbarch::displaced_step_buffer_length, this new field will do
exactly what it says on the tin; represent the required displaced step
buffer size.  The max_insn_length field can then do exactly what it
claims to do; represent the maximum length of a single instruction.

As some architectures (e.g. i386, and amd64) only require their
displaced step buffers to be a single instruction in size, I propose
that the default for displaced_step_buffer_length will be the
value of max_insn_length.  Architectures than need more buffer space
can then override this default as needed.

I've updated all architectures to setup the new field if appropriate,
and I've audited all calls to gdbarch_max_insn_length and switched to
gdbarch_displaced_step_buffer_length where appropriate.

There should be no user visible changes after this commit.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
index 0000b498f8959b44ed20404c834a4ad77fcea8d4..b183a3c9a380da866c41b31321c28f0e0bdd467f 100644 (file)
--- a/gdb/aarch64-linux-tdep.c
+++ b/gdb/aarch64-linux-tdep.c
@@ -2240,7 +2240,9 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_get_syscall_number (gdbarch, aarch64_linux_get_syscall_number);
 
   /* Displaced stepping.  */
-  set_gdbarch_max_insn_length (gdbarch, 4 * AARCH64_DISPLACED_MODIFIED_INSNS);
+  set_gdbarch_max_insn_length (gdbarch, 4);
+  set_gdbarch_displaced_step_buffer_length
+    (gdbarch, 4 * AARCH64_DISPLACED_MODIFIED_INSNS);
   set_gdbarch_displaced_step_copy_insn (gdbarch,
                                        aarch64_displaced_step_copy_insn);
   set_gdbarch_displaced_step_fixup (gdbarch, aarch64_displaced_step_fixup);

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 70d77452e93dc383c71707f36b3cefc1e130a418..883f8be296b9a61e4b0564fd3b8b45b402f9a303 100644 (file)
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -10662,7 +10662,9 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   /* Note: for displaced stepping, this includes the breakpoint, and one word
      of additional scratch space.  This setting isn't used for anything beside
      displaced stepping at present.  */
-  set_gdbarch_max_insn_length (gdbarch, 4 * ARM_DISPLACED_MODIFIED_INSNS);
+  set_gdbarch_displaced_step_buffer_length
+    (gdbarch, 4 * ARM_DISPLACED_MODIFIED_INSNS);
+  set_gdbarch_max_insn_length (gdbarch, 4);
 
   /* This should be low enough for everything.  */
   tdep->lowest_pc = 0x20;

diff --git a/gdb/displaced-stepping.c b/gdb/displaced-stepping.c
index 06b32a80f6a0ec54284aff2ddd456ea1e2625ecf..9f98ea8c35bd4dcf698e2775b5dab2194c99a3fb 100644 (file)
--- a/gdb/displaced-stepping.c
+++ b/gdb/displaced-stepping.c
@@ -55,7 +55,7 @@ displaced_step_buffers::prepare (thread_info *thread, CORE_ADDR &displaced_pc)
   regcache *regcache = get_thread_regcache (thread);
   const address_space *aspace = regcache->aspace ();
   gdbarch *arch = regcache->arch ();
-  ULONGEST len = gdbarch_max_insn_length (arch);
+  ULONGEST len = gdbarch_displaced_step_buffer_length (arch);
 
   /* Search for an unused buffer.  */
   displaced_step_buffer *buffer = nullptr;
@@ -243,7 +243,7 @@ displaced_step_buffers::finish (gdbarch *arch, thread_info *thread,
      below.  */
   thread->inf->displaced_step_state.unavailable = false;
 
-  ULONGEST len = gdbarch_max_insn_length (arch);
+  ULONGEST len = gdbarch_displaced_step_buffer_length (arch);
 
   /* Restore memory of the buffer.  */
   write_memory_ptid (thread->ptid, buffer->addr,
@@ -302,7 +302,7 @@ displaced_step_buffers::restore_in_ptid (ptid_t ptid)
 
       regcache *regcache = get_thread_regcache (buffer.current_thread);
       gdbarch *arch = regcache->arch ();
-      ULONGEST len = gdbarch_max_insn_length (arch);
+      ULONGEST len = gdbarch_displaced_step_buffer_length (arch);
 
       write_memory_ptid (ptid, buffer.addr, buffer.saved_copy.data (), len);
 

diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
index ddb97f60315f515487dea21fc4c58087a4f395b5..76d12a15317ffb892a52046b74b1bab85379eda8 100644 (file)
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -1039,8 +1039,8 @@ extern void set_gdbarch_max_insn_length (struct gdbarch *gdbarch, ULONGEST max_i
    see the comments in infrun.c.
 
    The TO area is only guaranteed to have space for
-   gdbarch_max_insn_length (arch) bytes, so this function must not
-   write more bytes than that to that area.
+   gdbarch_displaced_step_buffer_length (arch) octets, so this
+   function must not write more octets than that to this area.
 
    If you do not provide this function, GDB assumes that the
    architecture does not support displaced stepping.
@@ -1122,6 +1122,14 @@ typedef void (gdbarch_displaced_step_restore_all_in_ptid_ftype) (inferior *paren
 extern void gdbarch_displaced_step_restore_all_in_ptid (struct gdbarch *gdbarch, inferior *parent_inf, ptid_t child_ptid);
 extern void set_gdbarch_displaced_step_restore_all_in_ptid (struct gdbarch *gdbarch, gdbarch_displaced_step_restore_all_in_ptid_ftype *displaced_step_restore_all_in_ptid);
 
+/* The maximum length in octets required for a displaced-step instruction
+   buffer.  By default this will be the same as gdbarch::max_insn_length,
+   but should be overridden for architectures that might expand a
+   displaced-step instruction to multiple replacement instructions. */
+
+extern ULONGEST gdbarch_displaced_step_buffer_length (struct gdbarch *gdbarch);
+extern void set_gdbarch_displaced_step_buffer_length (struct gdbarch *gdbarch, ULONGEST displaced_step_buffer_length);
+
 /* Relocate an instruction to execute at a different address.  OLDLOC
    is the address in the inferior memory where the instruction to
    relocate is currently at.  On input, TO points to the destination

diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 84f6a481885249a83185350243e1615481a32d06..b4763aa6bf4fe26a5bfe537a3cf363c265f33af8 100644 (file)
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -192,6 +192,7 @@ struct gdbarch
   gdbarch_displaced_step_finish_ftype *displaced_step_finish = NULL;
   gdbarch_displaced_step_copy_insn_closure_by_addr_ftype *displaced_step_copy_insn_closure_by_addr = nullptr;
   gdbarch_displaced_step_restore_all_in_ptid_ftype *displaced_step_restore_all_in_ptid = nullptr;
+  ULONGEST displaced_step_buffer_length = 0;
   gdbarch_relocate_instruction_ftype *relocate_instruction = NULL;
   gdbarch_overlay_update_ftype *overlay_update = nullptr;
   gdbarch_core_read_description_ftype *core_read_description = nullptr;
@@ -451,6 +452,10 @@ verify_gdbarch (struct gdbarch *gdbarch)
     log.puts ("\n\tdisplaced_step_finish");
   /* Skip verify of displaced_step_copy_insn_closure_by_addr, has predicate.  */
   /* Skip verify of displaced_step_restore_all_in_ptid, invalid_p == 0 */
+  if (gdbarch->displaced_step_buffer_length == 0)
+    gdbarch->displaced_step_buffer_length = gdbarch->max_insn_length;
+  if (gdbarch->displaced_step_buffer_length < gdbarch->max_insn_length)
+    log.puts ("\n\tdisplaced_step_buffer_length");
   /* Skip verify of relocate_instruction, has predicate.  */
   /* Skip verify of overlay_update, has predicate.  */
   /* Skip verify of core_read_description, has predicate.  */
@@ -1109,6 +1114,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
   gdb_printf (file,
              "gdbarch_dump: displaced_step_restore_all_in_ptid = <%s>\n",
              host_address_to_string (gdbarch->displaced_step_restore_all_in_ptid));
+  gdb_printf (file,
+             "gdbarch_dump: displaced_step_buffer_length = %s\n",
+             plongest (gdbarch->displaced_step_buffer_length));
   gdb_printf (file,
              "gdbarch_dump: gdbarch_relocate_instruction_p() = %d\n",
              gdbarch_relocate_instruction_p (gdbarch));
@@ -4157,6 +4165,24 @@ set_gdbarch_displaced_step_restore_all_in_ptid (struct gdbarch *gdbarch,
   gdbarch->displaced_step_restore_all_in_ptid = displaced_step_restore_all_in_ptid;
 }
 
+ULONGEST
+gdbarch_displaced_step_buffer_length (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  /* Check variable is valid.  */
+  gdb_assert (!(gdbarch->displaced_step_buffer_length < gdbarch->max_insn_length));
+  if (gdbarch_debug >= 2)
+    gdb_printf (gdb_stdlog, "gdbarch_displaced_step_buffer_length called\n");
+  return gdbarch->displaced_step_buffer_length;
+}
+
+void
+set_gdbarch_displaced_step_buffer_length (struct gdbarch *gdbarch,
+                                         ULONGEST displaced_step_buffer_length)
+{
+  gdbarch->displaced_step_buffer_length = displaced_step_buffer_length;
+}
+
 bool
 gdbarch_relocate_instruction_p (struct gdbarch *gdbarch)
 {

diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
index 6cdf15b391026efa84b2d7a7afc640514c30f146..92c501d2a722caef59545ec42c05217b6791f065 100644 (file)
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -1735,8 +1735,8 @@ For a general explanation of displaced stepping and how GDB uses it,
 see the comments in infrun.c.
 
 The TO area is only guaranteed to have space for
-gdbarch_max_insn_length (arch) bytes, so this function must not
-write more bytes than that to that area.
+gdbarch_displaced_step_buffer_length (arch) octets, so this
+function must not write more octets than that to this area.
 
 If you do not provide this function, GDB assumes that the
 architecture does not support displaced stepping.
@@ -1844,6 +1844,20 @@ contents of all displaced step buffers in the child's address space.
     invalid=False,
 )
 
+Value(
+    comment="""
+The maximum length in octets required for a displaced-step instruction
+buffer.  By default this will be the same as gdbarch::max_insn_length,
+but should be overridden for architectures that might expand a
+displaced-step instruction to multiple replacement instructions.
+""",
+    type="ULONGEST",
+    name="displaced_step_buffer_length",
+    predefault="0",
+    postdefault="gdbarch->max_insn_length",
+    invalid="gdbarch->displaced_step_buffer_length < gdbarch->max_insn_length",
+)
+
 Method(
     comment="""
 Relocate an instruction to execute at a different address.  OLDLOC

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index e6ce13a1c672d4f326a6d5df9609d03d38f61ff2..3eaa5f3358484a7837f86c2b74d164a3c8fe6984 100644 (file)
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -2603,7 +2603,7 @@ linux_displaced_step_prepare (gdbarch *arch, thread_info *thread,
         at DISP_STEP_BUF_ADDR.  They are all of size BUF_LEN.  */
       CORE_ADDR disp_step_buf_addr
        = linux_displaced_step_location (thread->inf->gdbarch);
-      int buf_len = gdbarch_max_insn_length (arch);
+      int buf_len = gdbarch_displaced_step_buffer_length (arch);
 
       linux_gdbarch_data *gdbarch_data = get_linux_gdbarch_data (arch);
       gdb_assert (gdbarch_data->num_disp_step_buffers > 0);

diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 6ba56527a22f9c627cb0494908fd752d46b1fa18..9859a7d8b85a8cb8e7f578e157adc8b96b1bd0be 100644 (file)
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -889,7 +889,8 @@ ppc_displaced_step_copy_insn (struct gdbarch *gdbarch,
                              CORE_ADDR from, CORE_ADDR to,
                              struct regcache *regs)
 {
-  size_t len = gdbarch_max_insn_length (gdbarch);
+  size_t len = gdbarch_displaced_step_buffer_length (gdbarch);
+  gdb_assert (len > PPC_INSN_SIZE);
   std::unique_ptr<ppc_displaced_step_copy_insn_closure> closure
     (new ppc_displaced_step_copy_insn_closure (len));
   gdb_byte *buf = closure->buf.data ();
@@ -8363,8 +8364,9 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_displaced_step_finish (gdbarch, ppc_displaced_step_finish);
   set_gdbarch_displaced_step_restore_all_in_ptid
     (gdbarch, ppc_displaced_step_restore_all_in_ptid);
+  set_gdbarch_displaced_step_buffer_length (gdbarch, 2 * PPC_INSN_SIZE);
 
-  set_gdbarch_max_insn_length (gdbarch, 2 * PPC_INSN_SIZE);
+  set_gdbarch_max_insn_length (gdbarch, PPC_INSN_SIZE);
 
   /* Hook in ABI-specific overrides, if they have been registered.  */
   info.target_desc = tdesc;