From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1726) id C23E0385841A; Mon, 13 Mar 2023 22:01:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C23E0385841A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1678744900; bh=GsDCD2ahPdYX0ysfVFxLCgPlcLa2hj1RtE42b/qoIjg=; h=From:To:Subject:Date:From; b=WUVOVJnFHx52E/DQq3lSNwl53LVKCIhfViGinaQa3GFxc+dw9ZHHmMbL5F4aO5iun v16WD61KXmLl7N7PviT6QR+PR026aBpvMSbQGiQLkC49wLLxqdyE5XlolYKNRLmV+H hLw9cbAvGB1bA7mqmPcN8/cyPX91bQqEYNvKjQn0= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Andrew Burgess To: gdb-cvs@sourceware.org Subject: [binutils-gdb] gdb: add gdbarch::displaced_step_buffer_length X-Act-Checkin: binutils-gdb X-Git-Author: Andrew Burgess X-Git-Refname: refs/heads/master X-Git-Oldrev: 564cddf8edc75c1b043fcab93cc28861e0d48fa2 X-Git-Newrev: deb65a3cd86462cb19d10a867ee474b3f4cf7012 Message-Id: <20230313220140.C23E0385841A@sourceware.org> Date: Mon, 13 Mar 2023 22:01:40 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Ddeb65a3cd864= 62cb19d10a867ee474b3f4cf7012 commit deb65a3cd86462cb19d10a867ee474b3f4cf7012 Author: Andrew Burgess Date: Thu Feb 23 11:45:11 2023 +0000 gdb: add gdbarch::displaced_step_buffer_length =20 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. =20 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. =20 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. =20 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. =20 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. =20 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. =20 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. =20 There should be no user visible changes after this commit. =20 Approved-By: Simon Marchi Diff: --- gdb/aarch64-linux-tdep.c | 4 +++- gdb/arm-tdep.c | 4 +++- gdb/displaced-stepping.c | 6 +++--- gdb/gdbarch-gen.h | 12 ++++++++++-- gdb/gdbarch.c | 26 ++++++++++++++++++++++++++ gdb/gdbarch_components.py | 18 ++++++++++++++++-- gdb/linux-tdep.c | 2 +- gdb/rs6000-tdep.c | 6 ++++-- 8 files changed, 66 insertions(+), 12 deletions(-) diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c index 0000b498f89..b183a3c9a38 100644 --- a/gdb/aarch64-linux-tdep.c +++ b/gdb/aarch64-linux-tdep.c @@ -2240,7 +2240,9 @@ aarch64_linux_init_abi (struct gdbarch_info info, str= uct gdbarch *gdbarch) set_gdbarch_get_syscall_number (gdbarch, aarch64_linux_get_syscall_numbe= r); =20 /* Displaced stepping. */ - set_gdbarch_max_insn_length (gdbarch, 4 * AARCH64_DISPLACED_MODIFIED_INS= NS); + 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 70d77452e93..883f8be296b 100644 --- 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 w= ord of additional scratch space. This setting isn't used for anything be= side 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); =20 /* This should be low enough for everything. */ tdep->lowest_pc =3D 0x20; diff --git a/gdb/displaced-stepping.c b/gdb/displaced-stepping.c index 06b32a80f6a..9f98ea8c35b 100644 --- a/gdb/displaced-stepping.c +++ b/gdb/displaced-stepping.c @@ -55,7 +55,7 @@ displaced_step_buffers::prepare (thread_info *thread, COR= E_ADDR &displaced_pc) regcache *regcache =3D get_thread_regcache (thread); const address_space *aspace =3D regcache->aspace (); gdbarch *arch =3D regcache->arch (); - ULONGEST len =3D gdbarch_max_insn_length (arch); + ULONGEST len =3D gdbarch_displaced_step_buffer_length (arch); =20 /* Search for an unused buffer. */ displaced_step_buffer *buffer =3D nullptr; @@ -243,7 +243,7 @@ displaced_step_buffers::finish (gdbarch *arch, thread_i= nfo *thread, below. */ thread->inf->displaced_step_state.unavailable =3D false; =20 - ULONGEST len =3D gdbarch_max_insn_length (arch); + ULONGEST len =3D gdbarch_displaced_step_buffer_length (arch); =20 /* 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) =20 regcache *regcache =3D get_thread_regcache (buffer.current_thread); gdbarch *arch =3D regcache->arch (); - ULONGEST len =3D gdbarch_max_insn_length (arch); + ULONGEST len =3D gdbarch_displaced_step_buffer_length (arch); =20 write_memory_ptid (ptid, buffer.addr, buffer.saved_copy.data (), len= ); =20 diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index ddb97f60315..76d12a15317 100644 --- a/gdb/gdbarch-gen.h +++ b/gdb/gdbarch-gen.h @@ -1039,8 +1039,8 @@ extern void set_gdbarch_max_insn_length (struct gdbar= ch *gdbarch, ULONGEST max_i see the comments in infrun.c. =20 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. =20 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 *gd= barch, 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); =20 +/* 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 *gdba= rch); +extern void set_gdbarch_displaced_step_buffer_length (struct gdbarch *gdba= rch, 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 84f6a481885..b4763aa6bf4 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -192,6 +192,7 @@ struct gdbarch gdbarch_displaced_step_finish_ftype *displaced_step_finish =3D NULL; gdbarch_displaced_step_copy_insn_closure_by_addr_ftype *displaced_step_c= opy_insn_closure_by_addr =3D nullptr; gdbarch_displaced_step_restore_all_in_ptid_ftype *displaced_step_restore= _all_in_ptid =3D nullptr; + ULONGEST displaced_step_buffer_length =3D 0; gdbarch_relocate_instruction_ftype *relocate_instruction =3D NULL; gdbarch_overlay_update_ftype *overlay_update =3D nullptr; gdbarch_core_read_description_ftype *core_read_description =3D 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 predicat= e. */ /* Skip verify of displaced_step_restore_all_in_ptid, invalid_p =3D=3D 0= */ + if (gdbarch->displaced_step_buffer_length =3D=3D 0) + gdbarch->displaced_step_buffer_length =3D 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 =3D <%s>\n", host_address_to_string (gdbarch->displaced_step_restore_all_in_ptid= )); + gdb_printf (file, + "gdbarch_dump: displaced_step_buffer_length =3D %s\n", + plongest (gdbarch->displaced_step_buffer_length)); gdb_printf (file, "gdbarch_dump: gdbarch_relocate_instruction_p() =3D %d\n", gdbarch_relocate_instruction_p (gdbarch)); @@ -4157,6 +4165,24 @@ set_gdbarch_displaced_step_restore_all_in_ptid (stru= ct gdbarch *gdbarch, gdbarch->displaced_step_restore_all_in_ptid =3D displaced_step_restore_a= ll_in_ptid; } =20 +ULONGEST +gdbarch_displaced_step_buffer_length (struct gdbarch *gdbarch) +{ + gdb_assert (gdbarch !=3D NULL); + /* Check variable is valid. */ + gdb_assert (!(gdbarch->displaced_step_buffer_length < gdbarch->max_insn_= length)); + if (gdbarch_debug >=3D 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 =3D 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 6cdf15b3910..92c501d2a72 100644 --- a/gdb/gdbarch_components.py +++ b/gdb/gdbarch_components.py @@ -1735,8 +1735,8 @@ For a general explanation of displaced stepping and h= ow GDB uses it, see the comments in infrun.c. =20 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. =20 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=3DFalse, ) =20 +Value( + comment=3D""" +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=3D"ULONGEST", + name=3D"displaced_step_buffer_length", + predefault=3D"0", + postdefault=3D"gdbarch->max_insn_length", + invalid=3D"gdbarch->displaced_step_buffer_length < gdbarch->max_insn_l= ength", +) + Method( comment=3D""" Relocate an instruction to execute at a different address. OLDLOC diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c index e6ce13a1c67..3eaa5f33584 100644 --- a/gdb/linux-tdep.c +++ b/gdb/linux-tdep.c @@ -2603,7 +2603,7 @@ linux_displaced_step_prepare (gdbarch *arch, thread_i= nfo *thread, at DISP_STEP_BUF_ADDR. They are all of size BUF_LEN. */ CORE_ADDR disp_step_buf_addr =3D linux_displaced_step_location (thread->inf->gdbarch); - int buf_len =3D gdbarch_max_insn_length (arch); + int buf_len =3D gdbarch_displaced_step_buffer_length (arch); =20 linux_gdbarch_data *gdbarch_data =3D 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 6ba56527a22..9859a7d8b85 100644 --- 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 =3D gdbarch_max_insn_length (gdbarch); + size_t len =3D gdbarch_displaced_step_buffer_length (gdbarch); + gdb_assert (len > PPC_INSN_SIZE); std::unique_ptr closure (new ppc_displaced_step_copy_insn_closure (len)); gdb_byte *buf =3D 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); =20 - set_gdbarch_max_insn_length (gdbarch, 2 * PPC_INSN_SIZE); + set_gdbarch_max_insn_length (gdbarch, PPC_INSN_SIZE); =20 /* Hook in ABI-specific overrides, if they have been registered. */ info.target_desc =3D tdesc;