From patchwork Wed Jan 22 15:14:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 37466 Received: (qmail 76442 invoked by alias); 22 Jan 2020 15:14:38 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 76328 invoked by uid 89); 22 Jan 2020 15:14:30 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.1 spammy=17439 X-HELO: mail.efficios.com Received: from mail.efficios.com (HELO mail.efficios.com) (167.114.26.124) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 22 Jan 2020 15:14:28 +0000 Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id B301A25A1AC for ; Wed, 22 Jan 2020 10:14:26 -0500 (EST) Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id 7pRY6DPpnE4C; Wed, 22 Jan 2020 10:14:26 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 0F9E825A1AA; Wed, 22 Jan 2020 10:14:26 -0500 (EST) DKIM-Filter: OpenDKIM Filter v2.10.3 mail.efficios.com 0F9E825A1AA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficios.com; s=default; t=1579706066; bh=AuxMO43k+84T9v3e7xpyUXV85T4rDUwazNv0l56N4iA=; h=From:To:Date:Message-Id:MIME-Version; b=Whp6SZopObkxgBW/AnmdRWMY1k2h8kMsqENQkNYAAadpxspwGntJrMQ6JpmiNcphI QA4FpsWM5aoma8WMPhaJA0RBmQoGhcNeDdShhWXPLA+htqf2yKn+i31PdRkttHa6h5 e5GSA0vEWKr6Y+QMhJ2ScnUece/NTIhkiwwDXIl+JfxRGYuy9JmIIObdpXA8JZSi/5 6dm6W45Bj/6A1jUkF7pAo38XFVn7I+1tHrnZUtOH7eDevOx7NbFkIREH6dBKJC7vY9 XSwRpXCTnbDLmFhFUzRMxvgeiq2elwuhpt+ErW8Tvd1pu0Ua6WV/SURhgiCwdXpGHi FvCb+NfvPGfeg== Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id fdTsN5uGRIcE; Wed, 22 Jan 2020 10:14:25 -0500 (EST) Received: from smarchi-efficios.internal.efficios.com (192-222-181-218.qc.cable.ebox.net [192.222.181.218]) by mail.efficios.com (Postfix) with ESMTPSA id E0B6825A1A9; Wed, 22 Jan 2020 10:14:25 -0500 (EST) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH 2/2] gdb: make gdbarch_displaced_step_copy_insn return an std::unique_ptr Date: Wed, 22 Jan 2020 10:14:10 -0500 Message-Id: <20200122151410.30012-2-simon.marchi@efficios.com> In-Reply-To: <20200122151410.30012-1-simon.marchi@efficios.com> References: <20200122151410.30012-1-simon.marchi@efficios.com> MIME-Version: 1.0 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. * 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/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 ++-- 14 files changed, 32 insertions(+), 32 deletions(-) diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c index 1631178f8c26..aa768004bf78 100644 --- a/gdb/aarch64-tdep.c +++ b/gdb/aarch64-tdep.c @@ -2966,7 +2966,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) @@ -3020,7 +3020,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 732f78b5ba50..fc397967cd8e 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 f5ec40f37e8c..35ddfbaa32df 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 4c6d13222216..33ef0c3cea7e 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 363e67161c84..b3ae04fb431f 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 cc8569f5c959..011c2d3306e4 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -3930,7 +3930,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 9f32ac23aba6..de031cc8c16d 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 58f6e1ebdca1..9357636a1f5f 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 6f702b59e7f8..537f8c46876a 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 f120438081de..ff7f4baa4496 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 14bab37205f8..41faf515868d 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 1fee65730dbc..ef52c3e2faff 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1652,7 +1652,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 @@ -1744,9 +1743,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 @@ -1758,7 +1757,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 ccffc39508a6..58f828f41dc6 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 e01505549e67..780752338efb 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