From patchwork Thu Aug 17 08:48:10 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 22178 Received: (qmail 23107 invoked by alias); 17 Aug 2017 08:49:29 -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 22185 invoked by uid 89); 17 Aug 2017 08:48:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LOTSOFHASH, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: EUR03-VE1-obe.outbound.protection.outlook.com Received: from mail-eopbgr50043.outbound.protection.outlook.com (HELO EUR03-VE1-obe.outbound.protection.outlook.com) (40.107.5.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 17 Aug 2017 08:48:15 +0000 Received: from AM3PR08MB0101.eurprd08.prod.outlook.com (10.160.211.19) by AM3PR08MB0101.eurprd08.prod.outlook.com (10.160.211.19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.1.1341.17; Thu, 17 Aug 2017 08:48:10 +0000 Received: from AM3PR08MB0101.eurprd08.prod.outlook.com ([fe80::b8e2:8809:e2b2:ae37]) by AM3PR08MB0101.eurprd08.prod.outlook.com ([fe80::b8e2:8809:e2b2:ae37%14]) with mapi id 15.01.1341.024; Thu, 17 Aug 2017 08:48:10 +0000 From: Alan Hayward To: "gdb-patches@sourceware.org" CC: nd Subject: [PATCH 4/7] Regcache: Refactor dup/cpy/save/restore Date: Thu, 17 Aug 2017 08:48:10 +0000 Message-ID: authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM3PR08MB0101; 6:AGAt8l2ZAaQZrfqx0iecj/CJFUm/dxgPIo8NYtuVFFN0FW5lbRWJuRqYz0tqOIAuh6r9gGYTZtkBZUIx4hsw93+CZQKD/uUu8b0UVNc5tsen7WDv983bTDlvBI8JfjXuCRSpXTqA0vRbsjys0W0UE3AD1i3Mk8jzB3GpQ/7tq9u/siqsTRYdsLIcB1/zxwd6cTu9qwjmKCVGBTLkksTHos5ubYx7TnD5CIha3oH3QKZycyHOVWe5w+RsW5czAuS+2CElJVNPbSooSeX8/GWjygkJcW87xoVjF0C9I1riPPPVkn278gfsYUnWKjFmbneex08d9CAqjYchGjjM13UF3w==; 5:OCYASsUBRvnK1KoBNcn/KHNfl4pgvVOIAki/MYDDE8Dlkfp4AKYviANEr05B4wCLtznzOkh2X3Er1gzQxd934jiueorOQtFCi8vib00a18QMc/4Ni+lXDgj2x7oket76/wfETOeEC7ZsnbswB2Tw2A==; 24:aOIvdr8o7uKBnJsKg+wFC6pEoNYttsVpmENBKJ5boC0bF8UROGjl1he67/2902Y0bFEno1+Wxd5RjDpGAHqu4bWv+20kG50y33pclCVpw4M=; 7:YrKM7o61FdYJPHfZ/wHobx+bWQylPgIqWvzqknhOuU6x+Y1z1j+PnKqLRB1ztqSAeJwY+Avf7FJZg47PuFFDRO5+tMApe+jnO6aKAM0RasuUj3A6AFpjuXPFdEo8gPgUO9mB4k0tysjfWVRj+As4vzmzrFrUwEfsyJriKYLiulXRosrm7DkOxZ6kvxL48vB2H9A9ySvj0CtYK9stDSW8HIbyRM5Ne8msCdku1UQ8ZrQ= x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-correlation-id: 2d649ca4-7ca4-4600-4b00-08d4e54cb04d x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(300000500095)(300135000095)(300000501095)(300135300095)(300000502095)(300135100095)(22001)(2017030254152)(300000503095)(300135400095)(48565401081)(2017052603031)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095); SRVR:AM3PR08MB0101; x-ms-traffictypediagnostic: AM3PR08MB0101: nodisclaimer: True x-exchange-antispam-report-test: UriScan:(180628864354917); x-microsoft-antispam-prvs: x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(601004)(2401047)(5005006)(8121501046)(93006095)(93001095)(100000703101)(100105400095)(10201501046)(3002001)(6055026)(6041248)(20161123562025)(20161123558100)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123564025)(20161123555025)(20161123560025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:AM3PR08MB0101; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:AM3PR08MB0101; x-forefront-prvs: 0402872DA1 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(39860400002)(199003)(377424004)(189002)(81166006)(68736007)(86362001)(3280700002)(575784001)(478600001)(189998001)(2501003)(5640700003)(99286003)(5660300001)(2906002)(6436002)(53946003)(25786009)(7736002)(53936002)(3660700001)(33656002)(3846002)(305945005)(6512007)(36756003)(102836003)(101416001)(97736004)(4743002)(8936002)(6116002)(66066001)(14454004)(50986999)(6916009)(2900100001)(8676002)(54356999)(6486002)(106356001)(105586002)(72206003)(6506006)(81156014)(2351001)(82746002)(83716003)(4326008)(110136004)(5250100002); DIR:OUT; SFP:1101; SCL:1; SRVR:AM3PR08MB0101; H:AM3PR08MB0101.eurprd08.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-ID: <8CE83C783A8EC4488499667CFBF650D3@eurprd08.prod.outlook.com> MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Aug 2017 08:48:10.3588 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM3PR08MB0101 X-IsSubscribed: yes This patch refactors and simplifies the methods for copying a regcache. In the existing code, save/restore/dup/cpy differed on whether the dest and source were readonly or not readonly. These changes change the dependency to whether the dest and source are a target_regcache or regcache. The functions are now: save () : This is deleted from target_regcache, ensuring the destination is always a detached regcache. The code does not care if the source is a target_regache or not. restore_to () : Replacement for restore. This is deleted from target_regcache. It ensures that the target is a regcache and the source is a target_regcache. dup () : There is a different regcache and target_regcache version of this. Both versions return a detached regcache. regcache_cpy () : Removed. The above methods give everything required. regcache(readonly_t, regcache*) : Removed. Use dup() instead. Tested on a --enable-targets=all build with board files unix, native-gdbserver and unittest.exp. 2017-08-16 Alan Hayward * frame.c (frame_save_as_regcache): Use save. (frame_pop): Use restore_to. * infcmd.c (get_return_value): Use dup. * infrun.c (save_infcall_suspend_state): Likewise. (restore_infcall_suspend_state): Use restore_to. * linux-fork.c (fork_load_infrun_state): Likewise. (fork_save_infrun_state): Use dup. * ppc-linux-tdep.c (ppu2spu_sniffer): Use save. * regcache.c (regcache::regcache): Remove function. (regcache_save): Remove function. (regcache::save): Remove checks. (regcache::restore_to): New version of... (regcache::restore): ...this. (regcache::cpy): Remove function. (regcache::dup): Copy from regcache. (target_regcache::dup): Likewise. (regcache_dup): Remove function. * spu-tdep.c (spu2ppu_sniffer): Use dup. diff --git a/gdb/frame.c b/gdb/frame.c index 7fd4b07a2e95f28b2eb6a18dea4d2071f0ece4e2..30cd8532e6dcb91510c527f0c3c524558a7a6058 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -1024,7 +1024,7 @@ frame_save_as_regcache (struct frame_info *this_frame) regcache *backup = new regcache (get_frame_arch (this_frame), aspace); struct cleanup *cleanups = make_cleanup_regcache_delete (backup); - regcache_save (backup, do_frame_register_read, this_frame); + backup->save (do_frame_register_read, this_frame); discard_cleanups (cleanups); return backup; } @@ -1072,9 +1072,8 @@ frame_pop (struct frame_info *this_frame) Unfortunately, they don't implement it. Their lack of a formal definition can lead to targets writing back bogus values (arguably a bug in the target code mind). */ - /* Now copy those saved registers into the current regcache. - Here, regcache_cpy() calls regcache_restore(). */ - regcache_cpy (get_current_regcache (), scratch); + /* Now copy those saved registers into the current regcache. */ + scratch->restore_to (get_current_regcache ()); do_cleanups (cleanups); /* We've made right mess of GDB's local state, just discard diff --git a/gdb/infcmd.c b/gdb/infcmd.c index defa7b0c48cd3a1a90786ba3d883a986247a3b5d..52da4ba0e68417b7b515d1382f3f107e9d262130 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -1602,8 +1602,8 @@ advance_command (char *arg, int from_tty) struct value * get_return_value (struct value *function, struct type *value_type) { - regcache stop_regs (regcache::readonly, *get_current_regcache ()); - struct gdbarch *gdbarch = stop_regs.arch (); + regcache *stop_regs = get_current_regcache ()->dup (); + struct gdbarch *gdbarch = stop_regs->arch (); struct value *value; value_type = check_typedef (value_type); @@ -1623,7 +1623,7 @@ get_return_value (struct value *function, struct type *value_type) case RETURN_VALUE_ABI_RETURNS_ADDRESS: case RETURN_VALUE_ABI_PRESERVES_ADDRESS: value = allocate_value (value_type); - gdbarch_return_value (gdbarch, function, value_type, &stop_regs, + gdbarch_return_value (gdbarch, function, value_type, stop_regs, value_contents_raw (value), NULL); break; case RETURN_VALUE_STRUCT_CONVENTION: @@ -1633,6 +1633,7 @@ get_return_value (struct value *function, struct type *value_type) internal_error (__FILE__, __LINE__, _("bad switch")); } + delete stop_regs; return value; } diff --git a/gdb/infrun.c b/gdb/infrun.c index 6510aec548f6495ca64f5b566ff3628734698113..485cdccf133a055382b569c9d20a6f2f76c4c0ba 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -8911,7 +8911,7 @@ save_infcall_suspend_state (void) inf_state->stop_pc = stop_pc; - inf_state->registers = regcache_dup (regcache); + inf_state->registers = regcache->dup (); return inf_state; } @@ -8922,7 +8922,7 @@ void restore_infcall_suspend_state (struct infcall_suspend_state *inf_state) { struct thread_info *tp = inferior_thread (); - struct regcache *regcache = get_current_regcache (); + target_regcache *regcache = get_current_regcache (); struct gdbarch *gdbarch = get_regcache_arch (regcache); tp->suspend = inf_state->thread_suspend; @@ -8942,7 +8942,7 @@ restore_infcall_suspend_state (struct infcall_suspend_state *inf_state) (and perhaps other times). */ if (target_has_execution) /* NB: The register write goes through to the target. */ - regcache_cpy (regcache, inf_state->registers); + inf_state->registers->restore_to (regcache); discard_infcall_suspend_state (inf_state); } diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c index 032ff62807b728be336bf0e5daf59e47615d4799..d307aa7162cdd2b8ac1536e4eb1a1936ed6766f5 100644 --- a/gdb/linux-fork.c +++ b/gdb/linux-fork.c @@ -264,7 +264,7 @@ fork_load_infrun_state (struct fork_info *fp) linux_nat_switch_fork (fp->ptid); if (fp->savedregs && fp->clobber_regs) - regcache_cpy (get_current_regcache (), fp->savedregs); + fp->savedregs->restore_to (get_current_regcache ()); registers_changed (); reinit_frame_cache (); @@ -297,7 +297,7 @@ fork_save_infrun_state (struct fork_info *fp, int clobber_regs) if (fp->savedregs) delete fp->savedregs; - fp->savedregs = regcache_dup (get_current_regcache ()); + fp->savedregs = get_current_regcache ()->dup (); fp->clobber_regs = clobber_regs; if (clobber_regs) diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c index 42aff2cd1bf3834268b0b58dcf00dac1c52add96..285ab74cbf656f501805663b4e1063f28d5269d0 100644 --- a/gdb/ppc-linux-tdep.c +++ b/gdb/ppc-linux-tdep.c @@ -1366,7 +1366,7 @@ ppu2spu_sniffer (const struct frame_unwind *self, struct address_space *aspace = get_frame_address_space (this_frame); regcache *backup = new regcache (data.gdbarch, aspace); struct cleanup *cleanups = make_cleanup_regcache_delete (backup); - regcache_save (backup, ppu2spu_unwind_register, &data); + backup->save (ppu2spu_unwind_register, &data); discard_cleanups (cleanups); cache->frame_id = frame_id_build (base, func); diff --git a/gdb/regcache.h b/gdb/regcache.h index 00b87db7d145205b74859c08ca5a9d852441a4aa..1437dac220e0364557e3c568f1c5223cec598dfd 100644 --- a/gdb/regcache.h +++ b/gdb/regcache.h @@ -209,20 +209,10 @@ extern struct type *register_type (struct gdbarch *gdbarch, int regnum); extern int register_size (struct gdbarch *gdbarch, int regnum); - -/* Save/restore a register cache. The set of registers saved / - restored into the DST regcache determined by the save_reggroup / - restore_reggroup respectively. COOKED_READ returns zero iff the - register's value can't be returned. */ - typedef enum register_status (regcache_cooked_read_ftype) (void *src, int regnum, gdb_byte *buf); -extern void regcache_save (struct regcache *dst, - regcache_cooked_read_ftype *cooked_read, - void *cooked_read_context); - enum regcache_dump_what { regcache_dump_none, regcache_dump_raw, @@ -249,12 +239,6 @@ public: : regcache (gdbarch, aspace_, true) {} - struct readonly_t {}; - static constexpr readonly_t readonly {}; - - /* Create a readonly regcache from a non-readonly regcache. */ - regcache (readonly_t, const regcache &src); - regcache (const regcache &) = delete; void operator= (const regcache &) = delete; @@ -271,8 +255,17 @@ public: return m_aspace; } + /* Duplicate self into a new regcache. */ + virtual regcache* dup (); + + /* Copy the register contents from a target_regcache to self. + All cooked registers are read and cached. */ void save (regcache_cooked_read_ftype *cooked_read, void *src); + /* Copy register contents to a target_regcache. All cached cooked registers + are also restored. */ + void restore_to (target_regcache *dst); + enum register_status cooked_read (int regnum, gdb_byte *buf); void cooked_write (int regnum, const gdb_byte *buf); @@ -348,8 +341,6 @@ protected: gdb_byte *register_buffer (int regnum) const; - void restore (struct regcache *src); - struct regcache_descr *m_descr; /* The address space of this register cache (for registers where it @@ -363,12 +354,10 @@ protected: /* Register cache status. */ signed char *m_register_status; - /* Is this a read-only cache? A read-only cache is used for saving - the target's register state (e.g, across an inferior function - call or just before forcing a function return). A read-only - cache can only be updated via the methods regcache_dup() and - regcache_cpy(). The actual contents are determined by the - reggroup_save and reggroup_restore methods. */ + + /* A read-only cache can not change it's register contents, except from + an target_regcache via the save () method. + A target_regcache cache can never be read-only. */ bool m_readonly_p; private: @@ -385,8 +374,6 @@ private: private: - friend void - regcache_cpy (struct regcache *dst, struct regcache *src); }; @@ -397,6 +384,16 @@ class target_regcache : public regcache { public: + target_regcache (const target_regcache &) = delete; + void operator= (const target_regcache &) = delete; + + /* Cannot be called on a target_regcache. */ + void save (regcache_cooked_read_ftype *cooked_read, void *src) = delete; + void restore_to (target_regcache *dst) = delete; + + /* Duplicate self into a new regcache. Result is not a target_regcache. */ + regcache* dup (); + /* Overridden regcache methods. These versions will pass the read/write through to the target. */ enum register_status raw_read (int regnum, gdb_byte *buf); @@ -434,14 +431,6 @@ private: friend void registers_changed_ptid (ptid_t ptid); }; -/* Duplicate the contents of a register cache to a read-only register - cache. The operation is pass-through. */ -extern struct regcache *regcache_dup (struct regcache *regcache); - -/* Writes to DEST will go through to the target. SRC is a read-only - register cache. */ -extern void regcache_cpy (struct regcache *dest, struct regcache *src); - extern void registers_changed (void); extern void registers_changed_ptid (ptid_t); diff --git a/gdb/regcache.c b/gdb/regcache.c index e7da5a622e861e1e6a1938f91a0dd9a39942a050..5405dba7a706910f0b6d20c77eef657e38695b34 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -226,13 +226,6 @@ do_cooked_read (void *src, int regnum, gdb_byte *buf) return regcache_cooked_read (regcache, regnum, buf); } -regcache::regcache (readonly_t, const regcache &src) - : regcache (src.arch (), src.aspace (), true) -{ - gdb_assert (!src.m_readonly_p); - save (do_cooked_read, (void *) &src); -} - gdbarch * regcache::arch () const { @@ -312,23 +305,11 @@ regcache::register_buffer (int regnum) const } void -regcache_save (struct regcache *regcache, - regcache_cooked_read_ftype *cooked_read, void *src) -{ - regcache->save (cooked_read, src); -} - -void -regcache::save (regcache_cooked_read_ftype *cooked_read, - void *src) +regcache::save (regcache_cooked_read_ftype *cooked_read, void *src) { struct gdbarch *gdbarch = m_descr->gdbarch; int regnum; - /* The DST should be `read-only', if it wasn't then the save would - end up trying to write the register values back out to the - target. */ - gdb_assert (m_readonly_p); /* Clear the dest. */ memset (m_registers, 0, m_descr->sizeof_cooked_registers); memset (m_register_status, 0, m_descr->sizeof_cooked_register_status); @@ -354,15 +335,14 @@ regcache::save (regcache_cooked_read_ftype *cooked_read, } void -regcache::restore (struct regcache *src) +regcache::restore_to (target_regcache *dst) { struct gdbarch *gdbarch = m_descr->gdbarch; int regnum; + gdb_assert (dst != NULL); + gdb_assert (dst->m_descr->gdbarch == m_descr->gdbarch); + gdb_assert (dst != this); - /* The dst had better not be read-only. If it is, the `restore' - doesn't make much sense. */ - gdb_assert (!m_readonly_p); - gdb_assert (src->m_readonly_p); /* Copy over any registers, being careful to only restore those that were both saved and need to be restored. The full [0 .. gdbarch_num_regs + gdbarch_num_pseudo_regs) range is checked since some architectures need @@ -371,27 +351,33 @@ regcache::restore (struct regcache *src) { if (gdbarch_register_reggroup_p (gdbarch, regnum, restore_reggroup)) { - if (src->m_register_status[regnum] == REG_VALID) - cooked_write (regnum, src->register_buffer (regnum)); + if (m_register_status[regnum] == REG_VALID) + dst->cooked_write (regnum, register_buffer (regnum)); } } } -void -regcache_cpy (struct regcache *dst, struct regcache *src) +/* Duplicate detached regcache to a detached regcache. */ +regcache* +regcache::dup () { - gdb_assert (src != NULL && dst != NULL); - gdb_assert (src->m_descr->gdbarch == dst->m_descr->gdbarch); - gdb_assert (src != dst); - gdb_assert (src->m_readonly_p && !dst->m_readonly_p); + regcache *new_regcache = new regcache (arch (), aspace ()); + + memcpy (new_regcache->m_registers, m_registers, + m_descr->sizeof_cooked_registers); + memcpy (new_regcache->m_register_status, m_register_status, + m_descr->sizeof_cooked_register_status); - dst->restore (src); + return new_regcache; } -struct regcache * -regcache_dup (struct regcache *src) +/* Duplicate a target_regcache to a detached regcache. */ +regcache* +target_regcache::dup () { - return new regcache (regcache::readonly, *src); + regcache *new_regcache = new regcache (arch (), aspace ()); + new_regcache->save (do_cooked_read, (void *) this); + return new_regcache; } enum register_status diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c index 1ac763da7ede9d4b9f4bff1e8d71551b0c776fa1..59ed3851a62df324a61ca3c604f22548a1630f7d 100644 --- a/gdb/spu-tdep.c +++ b/gdb/spu-tdep.c @@ -1275,7 +1275,7 @@ spu2ppu_sniffer (const struct frame_unwind *self, { struct regcache *regcache; regcache = get_thread_arch_regcache (inferior_ptid, target_gdbarch ()); - cache->regcache = regcache_dup (regcache); + cache->regcache = regcache->dup (); *this_prologue_cache = cache; return 1; }