From patchwork Thu Aug 17 08:49:58 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Hayward X-Patchwork-Id: 22181 Received: (qmail 23965 invoked by alias); 17 Aug 2017 08:50:12 -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 23651 invoked by uid 89); 17 Aug 2017 08:50:05 -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-eopbgr50077.outbound.protection.outlook.com (HELO EUR03-VE1-obe.outbound.protection.outlook.com) (40.107.5.77) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 17 Aug 2017 08:50:03 +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:49:58 +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:49:58 +0000 From: Alan Hayward To: "gdb-patches@sourceware.org" CC: nd Subject: [PATCH 7/7]: Regcache: Refactor raw_set_cached_value Date: Thu, 17 Aug 2017 08:49:58 +0000 Message-ID: <280BC89C-5892-4097-9FEC-F5003D1BBE3C@arm.com> authentication-results: spf=none (sender IP is ) smtp.mailfrom=Alan.Hayward@arm.com; x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM3PR08MB0101; 6:VmLELxOpbFVm5pbsAVnIFDgLfMmVi2YyWRT8+K/L/A+dgp0XWvYAlHJJsXU+f+3CnNQ8JAputNJUVNmX1iN9qBGdaIT3KAmkakuvTwFOF3/n8UtpPmo4zTl489XtCA7ksKsrDMgVa38rnQAym9D6mmpNkC+ba0HEav4Qg6GXCJ2+mzv/BQdF/vBDyZHMjN81E+GitdXof3eTrQnmlpZsLn8hy2wEmraKKnZpWruTpxqVNKx2BjbBp5r97KCuUuJ5usceyhQ95Bqt7uFyUQ0rYgS0aOoknuh6MM7yxdEPuheoNx5SkCu0/PQiWBZTIimGXNGCjBAqiSrRm4LmLwZNMA==; 5:uFJdZg7XGVU2IdteIas2VcXiC+K3Nvc3LeE2mVrnTh5aN3wDx78d69nAjs1gBRMAeDNJ687mP+IJTPTQvGXoWXa6RGwVM4fh8LYxKJuJI0RIrkaQcqEFN3H0toA5RjbvF6+gdbAHTnGeCDaYLMkL1g==; 24:kzoGpZPLy2g57Pn9y8uyeIGn5QkZoYZog4MyAC2vD8/4e/RmsQ+Yan3CXMbWfsV6O/euqsfb2sfn0zItGZ2a7jmmHteAJn9peClXxagAzRs=; 7:JQNLL9OgQM1HIKJh139OlXMDxd2lvU1IEkeVPmbBvvgnr1peIl33fdRrK4LhbUMi6ZuJ90aAioL6gbospIImW8I4k2GHB2D+UXHAs66TJJcdPGH2nOCquBJZ7GYb0Rqba0q0fbPsNxe3oHPx+ApFp4XdLzWATMTUFIahmCN/CFjPfv2xwXOUgRFoDYazAvGRCqU/AoP3hXKXWMXGCayOJPIM97IF57mRjC2CSp3tOv8= x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-correlation-id: 5bc66fcf-f7b7-452b-1ffa-08d4e54cf0bf 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)(25786009)(7736002)(53936002)(3660700001)(33656002)(3846002)(305945005)(6512007)(36756003)(102836003)(101416001)(97736004)(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: MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Aug 2017 08:49:58.3696 (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 I was going to remove raw_set_cached_value. However, I think it can be used to make the code clearer. I'm renaming it to _reg instead of _value, because it doesn't use value structs. I'm making it a internal method, and adding a _get version. In regcache, wherever the code directly reads or writes to the internal cache, this is replaced with a call to the new functions. This makes it obvious where regcache is writing to the cache. On the downside, this adds extra function calls. The alternative to this patch would be to remove raw_set_cached_value, and keep the rest of the code unchanged. Tested on a --enable-targets=all build with board files unix, native-gdbserver and unittest.exp. 2017-08-16 Alan Hayward * gdbarch-selftests.c (regcache_test::raw_write): Call super method. * regcache.c (regcache::raw_read): Use raw_get_cached_reg. (target_regcache::raw_read): Move code to raw_get_cached_reg. (regcache::raw_set_cached_reg): Rename function. (regcache::raw_get_cached_reg): New function. (regcache::raw_write): Use raw_set_cached_reg. (target_regcache::raw_write): Use raw_set_cached_reg. (regcache::raw_supply): Likewise (regcache::raw_collect): Use raw_get_cached_reg. * regcache.h: (regcache_raw_set_cached_value): Remove. diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c index a6251d2a9f87ccff310f90eb5c27bc4199844f22..dbb0f810f951a87a56115d7fb1c392fc7eb0f9c1 100644 --- a/gdb/gdbarch-selftests.c +++ b/gdb/gdbarch-selftests.c @@ -40,7 +40,8 @@ public: void raw_write (int regnum, const gdb_byte *buf) override { - raw_set_cached_value (regnum, buf); + /* Bypass writing to the target. */ + regcache::raw_write (regnum, buf); } }; diff --git a/gdb/regcache.h b/gdb/regcache.h index f4408a562f84bb2d0dcd2792c646c0f98deee716..f45ab71d92018087e3fe94e3a7a69a87ecaf2e70 100644 --- a/gdb/regcache.h +++ b/gdb/regcache.h @@ -79,14 +79,6 @@ extern void regcache_raw_write_unsigned (struct regcache *regcache, extern LONGEST regcache_raw_get_signed (struct regcache *regcache, int regnum); -/* Set a raw register's value in the regcache's buffer. Unlike - regcache_raw_write, this is not write-through. The intention is - allowing to change the buffer contents of a read-only regcache - allocated with regcache_xmalloc. */ - -extern void regcache_raw_set_cached_value - (struct regcache *regcache, int regnum, const gdb_byte *buf); - /* Partial transfer of raw registers. These perform read, modify, write style operations. The read variant returns the status of the register. */ @@ -301,8 +293,6 @@ public: virtual enum register_status get_register_status (int regnum) const; - void raw_set_cached_value (int regnum, const gdb_byte *buf); - void invalidate (int regnum); enum register_status raw_read_part (int regnum, int offset, int len, @@ -339,6 +329,10 @@ protected: gdb_byte *register_buffer (int regnum) const; + /* Get/Set the cached contents of the regcache. */ + void raw_set_cached_reg (int regnum, const gdb_byte *buf); + enum register_status raw_get_cached_reg (int regnum, gdb_byte *buf) const; + struct regcache_descr *m_descr; /* The address space of this register cache (for registers where it diff --git a/gdb/regcache.c b/gdb/regcache.c index 9d04c7be904cfb17ac8fd871e9d7678df1694dfd..1fcb4d21332385ba094c23963a3b9db078a7ae3d 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -604,8 +604,7 @@ regcache_raw_read (struct regcache *regcache, int regnum, gdb_byte *buf) enum register_status regcache::raw_read (int regnum, gdb_byte *buf) { - raw_collect (regnum, buf); - return (enum register_status) m_register_status[regnum]; + return raw_get_cached_reg (regnum, buf); } enum register_status @@ -616,13 +615,7 @@ target_regcache::raw_read (int regnum, gdb_byte *buf) /* Read register value from the target into the regcache. */ raw_update (regnum); - if (m_register_status[regnum] != REG_VALID) - memset (buf, 0, m_descr->sizeof_register[regnum]); - else - memcpy (buf, register_buffer (regnum), - m_descr->sizeof_register[regnum]); - - return (enum register_status) m_register_status[regnum]; + return raw_get_cached_reg (regnum, buf); } enum register_status @@ -852,21 +845,42 @@ regcache_cooked_write_unsigned (struct regcache *regcache, int regnum, regcache->cooked_write (regnum, val); } -/* See regcache.h. */ - void -regcache_raw_set_cached_value (struct regcache *regcache, int regnum, - const gdb_byte *buf) +regcache::raw_set_cached_reg (int regnum, const gdb_byte *buf) { - regcache->raw_set_cached_value (regnum, buf); + gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers); + gdb_assert (!m_readonly_p); + + if (buf) + { + memcpy (register_buffer (regnum), buf, m_descr->sizeof_register[regnum]); + m_register_status[regnum] = REG_VALID; + } + else + { + /* This memset not strictly necessary, but better than garbage + in case the register value manages to escape somewhere (due + to a bug, no less). */ + memset (register_buffer (regnum), 0, m_descr->sizeof_register[regnum]); + m_register_status[regnum] = REG_UNAVAILABLE; + } + + gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers); } -void -regcache::raw_set_cached_value (int regnum, const gdb_byte *buf) +enum register_status +regcache::raw_get_cached_reg (int regnum, gdb_byte *buf) const { - memcpy (register_buffer (regnum), buf, - m_descr->sizeof_register[regnum]); - m_register_status[regnum] = REG_VALID; + gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers); + gdb_assert (buf != NULL); + + if (m_register_status[regnum] != REG_VALID) + memset (buf, 0, m_descr->sizeof_register[regnum]); + else + memcpy (buf, register_buffer (regnum), + m_descr->sizeof_register[regnum]); + + return (enum register_status) m_register_status[regnum]; } void @@ -880,16 +894,14 @@ regcache_raw_write (struct regcache *regcache, int regnum, void regcache::raw_write (int regnum, const gdb_byte *buf) { - gdb_assert (buf != NULL); gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers); - gdb_assert (!m_readonly_p); /* On the sparc, writing %g0 is a no-op, so we don't even want to change the registers array if something writes to this register. */ if (gdbarch_cannot_store_register (arch (), regnum)) return; - raw_set_cached_value (regnum, buf); + raw_set_cached_reg (regnum, buf); } void @@ -913,7 +925,7 @@ target_regcache::raw_write (int regnum, const gdb_byte *buf) return; target_prepare_to_store (this); - raw_set_cached_value (regnum, buf); + raw_set_cached_reg (regnum, buf); /* Register a cleanup function for invalidating the register after it is written, in case of a failure. */ @@ -1072,28 +1084,7 @@ regcache_raw_supply (struct regcache *regcache, int regnum, const void *buf) void regcache::raw_supply (int regnum, const void *buf) { - void *regbuf; - size_t size; - - gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers); - gdb_assert (!m_readonly_p); - - regbuf = register_buffer (regnum); - size = m_descr->sizeof_register[regnum]; - - if (buf) - { - memcpy (regbuf, buf, size); - m_register_status[regnum] = REG_VALID; - } - else - { - /* This memset not strictly necessary, but better than garbage - in case the register value manages to escape somewhere (due - to a bug, no less). */ - memset (regbuf, 0, size); - m_register_status[regnum] = REG_UNAVAILABLE; - } + raw_set_cached_reg (regnum, (const gdb_byte*) buf); } /* Supply register REGNUM to REGCACHE. Value to supply is an integer stored at @@ -1153,15 +1144,7 @@ regcache_raw_collect (const struct regcache *regcache, int regnum, void *buf) void regcache::raw_collect (int regnum, void *buf) const { - const void *regbuf; - size_t size; - - gdb_assert (buf != NULL); - gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers); - - regbuf = register_buffer (regnum); - size = m_descr->sizeof_register[regnum]; - memcpy (buf, regbuf, size); + raw_get_cached_reg (regnum, (gdb_byte*) buf); } /* Transfer a single or all registers belonging to a certain register