From e8217e61f5952ccfdabb0c4ee0c237a363e9bd99 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Fri, 14 Feb 2020 15:29:08 -0500 Subject: [PATCH] gdb: make gdbarch_displaced_step_copy_insn return an std::unique_ptr This callback dynamically allocates a specialized displaced_step_closure, and gives the ownership of the object to its caller. So I think it would make sense for the callback to return an std::unique_ptr, this is what this patch implements. gdb/ChangeLog: * gdbarch.sh (displaced_step_copy_insn): Change return type to an std::unique_ptr. * gdbarch.c: Re-generate. * gdbarch.h: Re-generate. * infrun.c (displaced_step_prepare_throw): Adjust to std::unique_ptr change. * aarch64-tdep.c (aarch64_displaced_step_copy_insn): Change return type to std::unique_ptr. * aarch64-tdep.h (aarch64_displaced_step_copy_insn): Likewise. * amd64-tdep.c (amd64_displaced_step_copy_insn): Likewise. * amd64-tdep.h (amd64_displaced_step_copy_insn): Likewise. * arm-linux-tdep.c (arm_linux_displaced_step_copy_insn): Likewise. * i386-linux-tdep.c (i386_linux_displaced_step_copy_insn): Likewise. * i386-tdep.c (i386_displaced_step_copy_insn): Likewise. * i386-tdep.h (i386_displaced_step_copy_insn): Likewise. * rs6000-tdep.c (ppc_displaced_step_copy_insn): Likewise. * s390-tdep.c (s390_displaced_step_copy_insn): Likewise. --- gdb/ChangeLog | 20 ++++++++++++++++++++ gdb/aarch64-tdep.c | 4 ++-- gdb/aarch64-tdep.h | 2 +- gdb/amd64-tdep.c | 8 ++++---- gdb/amd64-tdep.h | 2 +- gdb/arm-linux-tdep.c | 11 ++++++----- gdb/gdbarch.c | 2 +- gdb/gdbarch.h | 4 ++-- gdb/gdbarch.sh | 2 +- gdb/i386-linux-tdep.c | 6 +++--- gdb/i386-tdep.c | 5 +++-- gdb/i386-tdep.h | 2 +- gdb/infrun.c | 8 +++----- gdb/rs6000-tdep.c | 4 ++-- gdb/s390-tdep.c | 4 ++-- 15 files changed, 52 insertions(+), 32 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index e3fa08165a..b71251c6a2 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,23 @@ +2020-02-14 Simon Marchi + + * gdbarch.sh (displaced_step_copy_insn): Change return type to an + std::unique_ptr. + * gdbarch.c: Re-generate. + * gdbarch.h: Re-generate. + * infrun.c (displaced_step_prepare_throw): Adjust to std::unique_ptr + change. + * aarch64-tdep.c (aarch64_displaced_step_copy_insn): Change return + type to std::unique_ptr. + * aarch64-tdep.h (aarch64_displaced_step_copy_insn): Likewise. + * amd64-tdep.c (amd64_displaced_step_copy_insn): Likewise. + * amd64-tdep.h (amd64_displaced_step_copy_insn): Likewise. + * arm-linux-tdep.c (arm_linux_displaced_step_copy_insn): Likewise. + * i386-linux-tdep.c (i386_linux_displaced_step_copy_insn): Likewise. + * i386-tdep.c (i386_displaced_step_copy_insn): Likewise. + * i386-tdep.h (i386_displaced_step_copy_insn): Likewise. + * rs6000-tdep.c (ppc_displaced_step_copy_insn): Likewise. + * s390-tdep.c (s390_displaced_step_copy_insn): Likewise. + 2020-02-14 Simon Marchi * infrun.c (get_displaced_step_closure_by_addr): Adjust to diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 1c44345e57..1bf6bfdf92 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -2999,7 +2999,7 @@ static const struct aarch64_insn_visitor visitor = /* Implement the "displaced_step_copy_insn" gdbarch method. */ -struct displaced_step_closure * +std::unique_ptr aarch64_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs) @@ -3053,7 +3053,7 @@ aarch64_displaced_step_copy_insn (struct gdbarch *gdbarch, dsc = NULL; } - return dsc.release (); + return dsc; } /* Implement the "displaced_step_fixup" gdbarch method. */ diff --git a/gdb/aarch64-tdep.h b/gdb/aarch64-tdep.h index 732f78b5ba..fc397967cd 100644 --- a/gdb/aarch64-tdep.h +++ b/gdb/aarch64-tdep.h @@ -106,7 +106,7 @@ const target_desc *aarch64_read_description (uint64_t vq, bool pauth_p); extern int aarch64_process_record (struct gdbarch *gdbarch, struct regcache *regcache, CORE_ADDR addr); -struct displaced_step_closure * +struct std::unique_ptr aarch64_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs); diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c index f5ec40f37e..35ddfbaa32 100644 --- a/gdb/amd64-tdep.c +++ b/gdb/amd64-tdep.c @@ -1465,7 +1465,7 @@ fixup_displaced_copy (struct gdbarch *gdbarch, } } -struct displaced_step_closure * +std::unique_ptr amd64_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs) @@ -1474,8 +1474,8 @@ amd64_displaced_step_copy_insn (struct gdbarch *gdbarch, /* Extra space for sentinels so fixup_{riprel,displaced_copy} don't have to continually watch for running off the end of the buffer. */ int fixup_sentinel_space = len; - amd64_displaced_step_closure *dsc - = new amd64_displaced_step_closure (len + fixup_sentinel_space); + std::unique_ptr dsc + (new amd64_displaced_step_closure (len + fixup_sentinel_space)); gdb_byte *buf = &dsc->insn_buf[0]; struct amd64_insn *details = &dsc->insn_details; @@ -1500,7 +1500,7 @@ amd64_displaced_step_copy_insn (struct gdbarch *gdbarch, /* Modify the insn to cope with the address where it will be executed from. In particular, handle any rip-relative addressing. */ - fixup_displaced_copy (gdbarch, dsc, from, to, regs); + fixup_displaced_copy (gdbarch, dsc.get (), from, to, regs); write_memory (to, buf, len); diff --git a/gdb/amd64-tdep.h b/gdb/amd64-tdep.h index 4c6d132222..33ef0c3cea 100644 --- a/gdb/amd64-tdep.h +++ b/gdb/amd64-tdep.h @@ -87,7 +87,7 @@ enum amd64_regnum #define AMD64_NUM_REGS (AMD64_GSBASE_REGNUM + 1) -extern struct displaced_step_closure *amd64_displaced_step_copy_insn +extern std::unique_ptr amd64_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs); extern void amd64_displaced_step_fixup (struct gdbarch *gdbarch, diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c index 363e67161c..b3ae04fb43 100644 --- a/gdb/arm-linux-tdep.c +++ b/gdb/arm-linux-tdep.c @@ -1103,12 +1103,13 @@ arm_catch_kernel_helper_return (struct gdbarch *gdbarch, CORE_ADDR from, the program has stepped into a Linux kernel helper routine (which must be handled as a special case). */ -static struct displaced_step_closure * +static std::unique_ptr arm_linux_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs) { - arm_displaced_step_closure *dsc = new arm_displaced_step_closure; + std::unique_ptr dsc + (new arm_displaced_step_closure); /* Detect when we enter an (inaccessible by GDB) Linux kernel helper, and stop at the return location. */ @@ -1118,17 +1119,17 @@ arm_linux_displaced_step_copy_insn (struct gdbarch *gdbarch, fprintf_unfiltered (gdb_stdlog, "displaced: detected kernel helper " "at %.8lx\n", (unsigned long) from); - arm_catch_kernel_helper_return (gdbarch, from, to, regs, dsc); + arm_catch_kernel_helper_return (gdbarch, from, to, regs, dsc.get ()); } else { /* Override the default handling of SVC instructions. */ dsc->u.svc.copy_svc_os = arm_linux_copy_svc; - arm_process_displaced_insn (gdbarch, from, to, regs, dsc); + arm_process_displaced_insn (gdbarch, from, to, regs, dsc.get ()); } - arm_displaced_init_closure (gdbarch, from, to, dsc); + arm_displaced_init_closure (gdbarch, from, to, dsc.get ()); return dsc; } diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index d763fc85e6..4e59b375f7 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -3936,7 +3936,7 @@ gdbarch_displaced_step_copy_insn_p (struct gdbarch *gdbarch) return gdbarch->displaced_step_copy_insn != NULL; } -struct displaced_step_closure * +std::unique_ptr gdbarch_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs) { gdb_assert (gdbarch != NULL); diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h index 800a4e8b16..8a3a07181c 100644 --- a/gdb/gdbarch.h +++ b/gdb/gdbarch.h @@ -1036,8 +1036,8 @@ extern void set_gdbarch_max_insn_length (struct gdbarch *gdbarch, ULONGEST max_i extern int gdbarch_displaced_step_copy_insn_p (struct gdbarch *gdbarch); -typedef struct displaced_step_closure * (gdbarch_displaced_step_copy_insn_ftype) (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs); -extern struct displaced_step_closure * gdbarch_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs); +typedef std::unique_ptr (gdbarch_displaced_step_copy_insn_ftype) (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs); +extern std::unique_ptr gdbarch_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs); extern void set_gdbarch_displaced_step_copy_insn (struct gdbarch *gdbarch, gdbarch_displaced_step_copy_insn_ftype *displaced_step_copy_insn); /* Return true if GDB should use hardware single-stepping to execute diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh index a86de96119..d4170c9822 100755 --- a/gdb/gdbarch.sh +++ b/gdb/gdbarch.sh @@ -816,7 +816,7 @@ V;ULONGEST;max_insn_length;;;0;0 # If the instruction cannot execute out of line, return NULL. The # core falls back to stepping past the instruction in-line instead in # that case. -M;struct displaced_step_closure *;displaced_step_copy_insn;CORE_ADDR from, CORE_ADDR to, struct regcache *regs;from, to, regs +M;std::unique_ptr;displaced_step_copy_insn;CORE_ADDR from, CORE_ADDR to, struct regcache *regs;from, to, regs # Return true if GDB should use hardware single-stepping to execute # the displaced instruction identified by CLOSURE. If false, diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c index f4a5f0a761..7170687f4d 100644 --- a/gdb/i386-linux-tdep.c +++ b/gdb/i386-linux-tdep.c @@ -797,12 +797,12 @@ i386_linux_iterate_over_regset_sections (struct gdbarch *gdbarch, which does not seem worth it. The same effect is achieved by patching that 'nop' instruction there instead. */ -static struct displaced_step_closure * +static std::unique_ptr i386_linux_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs) { - displaced_step_closure *closure_ + std::unique_ptr closure_ = i386_displaced_step_copy_insn (gdbarch, from, to, regs); if (i386_linux_get_syscall_number_from_regcache (regs) != -1) @@ -810,7 +810,7 @@ i386_linux_displaced_step_copy_insn (struct gdbarch *gdbarch, /* The closure returned by i386_displaced_step_copy_insn is simply a buffer with a copy of the instruction. */ i386_displaced_step_closure *closure - = (i386_displaced_step_closure *) closure_; + = (i386_displaced_step_closure *) closure_.get (); /* Fake nop. */ closure->buf[0] = 0x90; diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c index f71444f652..4b6f3d0ecd 100644 --- a/gdb/i386-tdep.c +++ b/gdb/i386-tdep.c @@ -798,13 +798,14 @@ i386_insn_is_jump (struct gdbarch *gdbarch, CORE_ADDR addr) /* Some kernels may run one past a syscall insn, so we have to cope. */ -struct displaced_step_closure * +std::unique_ptr i386_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs) { size_t len = gdbarch_max_insn_length (gdbarch); - i386_displaced_step_closure *closure = new i386_displaced_step_closure (len); + std::unique_ptr closure + (new i386_displaced_step_closure (len)); gdb_byte *buf = closure->buf.data (); read_memory (from, buf, len); diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h index 14bab37205..41faf51586 100644 --- a/gdb/i386-tdep.h +++ b/gdb/i386-tdep.h @@ -428,7 +428,7 @@ extern void typedef buf_displaced_step_closure i386_displaced_step_closure; -extern struct displaced_step_closure *i386_displaced_step_copy_insn +extern std::unique_ptr i386_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs); extern void i386_displaced_step_fixup (struct gdbarch *gdbarch, diff --git a/gdb/infrun.c b/gdb/infrun.c index e3e4bdb9b8..d9a6f73351 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1646,7 +1646,6 @@ displaced_step_prepare_throw (thread_info *tp) const address_space *aspace = regcache->aspace (); CORE_ADDR original, copy; ULONGEST len; - struct displaced_step_closure *closure; int status; /* We should never reach this function if the architecture does not @@ -1738,9 +1737,9 @@ displaced_step_prepare_throw (thread_info *tp) len); }; - closure = gdbarch_displaced_step_copy_insn (gdbarch, - original, copy, regcache); - if (closure == NULL) + displaced->step_closure + = gdbarch_displaced_step_copy_insn (gdbarch, original, copy, regcache); + if (displaced->step_closure == NULL) { /* The architecture doesn't know how or want to displaced step this instruction or instruction sequence. Fallback to @@ -1752,7 +1751,6 @@ displaced_step_prepare_throw (thread_info *tp) succeeds. */ displaced->step_thread = tp; displaced->step_gdbarch = gdbarch; - displaced->step_closure.reset (closure); displaced->step_original = original; displaced->step_copy = copy; diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c index 919bebc71b..010bbc9e01 100644 --- a/gdb/rs6000-tdep.c +++ b/gdb/rs6000-tdep.c @@ -855,7 +855,7 @@ typedef buf_displaced_step_closure ppc_displaced_step_closure; /* We can't displaced step atomic sequences. */ -static struct displaced_step_closure * +static std::unique_ptr ppc_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs) @@ -894,7 +894,7 @@ ppc_displaced_step_copy_insn (struct gdbarch *gdbarch, displaced_step_dump_bytes (gdb_stdlog, buf, len); } - return closure.release (); + return closure; } /* Fix up the state of registers and memory after having single-stepped diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c index 5f3cb7e81e..313d459cb3 100644 --- a/gdb/s390-tdep.c +++ b/gdb/s390-tdep.c @@ -425,7 +425,7 @@ typedef buf_displaced_step_closure s390_displaced_step_closure; /* Implementation of gdbarch_displaced_step_copy_insn. */ -static struct displaced_step_closure * +static std::unique_ptr s390_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs) @@ -477,7 +477,7 @@ s390_displaced_step_copy_insn (struct gdbarch *gdbarch, displaced_step_dump_bytes (gdb_stdlog, buf, len); } - return closure.release (); + return closure; } /* Fix up the state of registers and memory after having single-stepped -- 2.34.1