From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 37643 invoked by alias); 21 Jun 2018 13:52:16 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 37606 invoked by uid 89); 21 Jun 2018 13:52:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.7 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3 autolearn=ham version=3.3.2 spammy= X-HELO: sessmg22.ericsson.net Received: from sessmg22.ericsson.net (HELO sessmg22.ericsson.net) (193.180.251.58) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 21 Jun 2018 13:52:13 +0000 Received: from ESESBMB501.ericsson.se (Unknown_Domain [153.88.183.114]) by sessmg22.ericsson.net (Symantec Mail Security) with SMTP id C1.6C.31169.A8DAB2B5; Thu, 21 Jun 2018 15:52:10 +0200 (CEST) Received: from ESESBMB501.ericsson.se (153.88.183.168) by ESESBMB501.ericsson.se (153.88.183.168) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3; Thu, 21 Jun 2018 15:51:44 +0200 Received: from NAM04-SN1-obe.outbound.protection.outlook.com (153.88.183.157) by ESESBMB501.ericsson.se (153.88.183.168) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3 via Frontend Transport; Thu, 21 Jun 2018 15:51:43 +0200 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=simon.marchi@ericsson.com; Received: from [142.133.60.192] (192.75.88.130) by BN7PR15MB2386.namprd15.prod.outlook.com (2603:10b6:406:8c::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.863.16; Thu, 21 Jun 2018 13:51:41 +0000 Subject: Re: [PATCH v2 1/3] Use unsigned ints in regcache_map_entry From: Simon Marchi To: Alan Hayward , CC: References: <20180621093802.79342-1-alan.hayward@arm.com> <20180621093802.79342-2-alan.hayward@arm.com> <4e636367-f19b-3aa8-6491-42d4ea5b024b@ericsson.com> Message-ID: <3c8db027-f24e-91cb-b7cc-25fb8cae0067@ericsson.com> Date: Thu, 21 Jun 2018 13:52:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <4e636367-f19b-3aa8-6491-42d4ea5b024b@ericsson.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-ClientProxiedBy: SN4PR0601CA0004.namprd06.prod.outlook.com (2603:10b6:803:2f::14) To BN7PR15MB2386.namprd15.prod.outlook.com (2603:10b6:406:8c::24) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 27e2b0b9-b96e-4ce3-8308-08d5d77e1e64 X-MS-TrafficTypeDiagnostic: BN7PR15MB2386: X-Exchange-Antispam-Report-Test: UriScan:(37575265505322); X-MS-Exchange-SenderADCheck: 1 X-Forefront-PRVS: 07106EF9B9 Received-SPF: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Jun 2018 13:51:41.2610 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 27e2b0b9-b96e-4ce3-8308-08d5d77e1e64 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 92e84ceb-fbfd-47ab-be52-080c6b87953f X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN7PR15MB2386 X-OriginatorOrg: ericsson.com X-IsSubscribed: yes X-SW-Source: 2018-06/txt/msg00516.txt.bz2 On 2018-06-21 09:27 AM, Simon Marchi wrote: > On 2018-06-21 05:38 AM, Alan Hayward wrote: >> All current uses of regcache_map_entry use static hard coded values. >> Update transfer_regset which uses those values. > > Can you explain what we gain from this patch? In the previous discussion, > I mentioned that the parameters LEN and OFFSET in the regcache methods > (e.g. read_part) could be come unsigned, which would allow us to remove > the "offset >= 0 && len >= 0" assertions. In turn, they won't be > needed in your raw_collect_part/raw_supply_part. But I don't see > exactly what the current patch brings (though it's not incorrect). > > Simon > This is what I would suggest: >From 22032e0f89b74daaa8186223508eb9d788772962 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 21 Jun 2018 09:42:05 -0400 Subject: [PATCH] Make some regcache method parameters unsigned The parameters LEN and OFFSET in some of regcache's methods can only have values >= 0, so we can make them unsigned. Doing so allows us to remove some asserts in read_part and write_part. Also, when we have these two assertions ... offset <= m_descr->sizeof_register[regnum] offset + len <= m_descr->sizeof_register[regnum] ... if (offset + len) is smaller than the register size, then offset is necessarily smaller than the register size. So we can remove the first one. gdb/ChangeLog: * common/common-regcache.h (struct reg_buffer_common) : Make OFFSET unsigned. * regcache.h (class reg_buffer) : Likewise. (class readable_regcache) : Make OFFSET and LEN unsigned. (class regcache) : Likewise. * regcache.c (readable_regcache::read_part): Make OFFSET and LEN unsigned, remove assertions. (regcache::write_part): Likewise. (readable_regcache::raw_read_part): Make OFFSET and LEN unsigned. (regcache::raw_write_part): Likewise. (readable_regcache::cooked_read_part): Likewise. (regcache::cooked_write_part): Likewise. (reg_buffer::raw_compare): Make OFFSET unsigned. gdb/gdbserver/ChangeLog: * regcache.h (struct regcache) : Make OFFSET unsigned. * regcache.c (regcache::raw_compare): Likewise. --- gdb/common/common-regcache.h | 3 ++- gdb/gdbserver/regcache.c | 2 +- gdb/gdbserver/regcache.h | 3 ++- gdb/regcache.c | 27 +++++++++++++-------------- gdb/regcache.h | 25 ++++++++++++++----------- 5 files changed, 32 insertions(+), 28 deletions(-) diff --git a/gdb/common/common-regcache.h b/gdb/common/common-regcache.h index 18080b2..7973254 100644 --- a/gdb/common/common-regcache.h +++ b/gdb/common/common-regcache.h @@ -79,7 +79,8 @@ struct reg_buffer_common /* Compare the contents of the register stored in the regcache (ignoring the first OFFSET bytes) to the contents of BUF (without any offset). Returns true if the same. */ - virtual bool raw_compare (int regnum, const void *buf, int offset) const = 0; + virtual bool raw_compare (int regnum, const void *buf, + unsigned int offset) const = 0; }; #endif /* COMMON_REGCACHE_H */ diff --git a/gdb/gdbserver/regcache.c b/gdb/gdbserver/regcache.c index 735ce7b..864333b 100644 --- a/gdb/gdbserver/regcache.c +++ b/gdb/gdbserver/regcache.c @@ -506,7 +506,7 @@ regcache::get_register_status (int regnum) const /* See common/common-regcache.h. */ bool -regcache::raw_compare (int regnum, const void *buf, int offset) const +regcache::raw_compare (int regnum, const void *buf, unsigned int offset) const { gdb_assert (buf != NULL); diff --git a/gdb/gdbserver/regcache.h b/gdb/gdbserver/regcache.h index b4c4c20..69cbeda 100644 --- a/gdb/gdbserver/regcache.h +++ b/gdb/gdbserver/regcache.h @@ -56,7 +56,8 @@ struct regcache : public reg_buffer_common void raw_collect (int regnum, void *buf) const override; /* See common/common-regcache.h. */ - bool raw_compare (int regnum, const void *buf, int offset) const override; + bool raw_compare (int regnum, const void *buf, + unsigned int offset) const override; }; struct regcache *init_register_cache (struct regcache *regcache, diff --git a/gdb/regcache.c b/gdb/regcache.c index 25436bb..919f9a1 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -779,15 +779,14 @@ regcache::cooked_write (int regnum, const gdb_byte *buf) operation. */ enum register_status -readable_regcache::read_part (int regnum, int offset, int len, void *in, - bool is_raw) +readable_regcache::read_part (int regnum, unsigned int offset, unsigned int len, + void *in, bool is_raw) { struct gdbarch *gdbarch = arch (); gdb_byte *reg = (gdb_byte *) alloca (register_size (gdbarch, regnum)); gdb_assert (in != NULL); - gdb_assert (offset >= 0 && offset <= m_descr->sizeof_register[regnum]); - gdb_assert (len >= 0 && offset + len <= m_descr->sizeof_register[regnum]); + gdb_assert (offset + len <= m_descr->sizeof_register[regnum]); /* Something to do? */ if (offset + len == 0) return REG_VALID; @@ -808,15 +807,14 @@ readable_regcache::read_part (int regnum, int offset, int len, void *in, } enum register_status -regcache::write_part (int regnum, int offset, int len, - const void *out, bool is_raw) +regcache::write_part (int regnum, unsigned int offset, unsigned int len, + const void *out, bool is_raw) { struct gdbarch *gdbarch = arch (); gdb_byte *reg = (gdb_byte *) alloca (register_size (gdbarch, regnum)); gdb_assert (out != NULL); - gdb_assert (offset >= 0 && offset <= m_descr->sizeof_register[regnum]); - gdb_assert (len >= 0 && offset + len <= m_descr->sizeof_register[regnum]); + gdb_assert (offset + len <= m_descr->sizeof_register[regnum]); /* Something to do? */ if (offset + len == 0) return REG_VALID; @@ -845,7 +843,8 @@ regcache::write_part (int regnum, int offset, int len, } enum register_status -readable_regcache::raw_read_part (int regnum, int offset, int len, gdb_byte *buf) +readable_regcache::raw_read_part (int regnum, unsigned int offset, + unsigned int len, gdb_byte *buf) { assert_regnum (regnum); return read_part (regnum, offset, len, buf, true); @@ -854,7 +853,7 @@ readable_regcache::raw_read_part (int regnum, int offset, int len, gdb_byte *buf /* See regcache.h. */ void -regcache::raw_write_part (int regnum, int offset, int len, +regcache::raw_write_part (int regnum, unsigned int offset, unsigned int len, const gdb_byte *buf) { assert_regnum (regnum); @@ -862,15 +861,15 @@ regcache::raw_write_part (int regnum, int offset, int len, } enum register_status -readable_regcache::cooked_read_part (int regnum, int offset, int len, - gdb_byte *buf) +readable_regcache::cooked_read_part (int regnum, unsigned int offset, + unsigned int len, gdb_byte *buf) { gdb_assert (regnum >= 0 && regnum < m_descr->nr_cooked_registers); return read_part (regnum, offset, len, buf, false); } void -regcache::cooked_write_part (int regnum, int offset, int len, +regcache::cooked_write_part (int regnum, unsigned int offset, unsigned int len, const gdb_byte *buf) { gdb_assert (regnum >= 0 && regnum < m_descr->nr_cooked_registers); @@ -1077,7 +1076,7 @@ regcache::collect_regset (const struct regset *regset, /* See common/common-regcache.h. */ bool -reg_buffer::raw_compare (int regnum, const void *buf, int offset) const +reg_buffer::raw_compare (int regnum, const void *buf, unsigned int offset) const { gdb_assert (buf != NULL); assert_regnum (regnum); diff --git a/gdb/regcache.h b/gdb/regcache.h index 74ac858..0bf7f1b 100644 --- a/gdb/regcache.h +++ b/gdb/regcache.h @@ -188,7 +188,8 @@ public: virtual ~reg_buffer () = default; /* See common/common-regcache.h. */ - bool raw_compare (int regnum, const void *buf, int offset) const override; + bool raw_compare (int regnum, const void *buf, + unsigned int offset) const override; protected: /* Assert on the range of REGNUM. */ @@ -232,8 +233,8 @@ public: enum register_status raw_read (int regnum, T *val); /* Partial transfer of raw registers. Return the status of the register. */ - enum register_status raw_read_part (int regnum, int offset, int len, - gdb_byte *buf); + enum register_status raw_read_part (int regnum, unsigned int offset, + unsigned int len, gdb_byte *buf); /* Make certain that the register REGNUM is up-to-date. */ virtual void raw_update (int regnum) = 0; @@ -245,16 +246,16 @@ public: enum register_status cooked_read (int regnum, T *val); /* Partial transfer of a cooked register. */ - enum register_status cooked_read_part (int regnum, int offset, int len, - gdb_byte *buf); + enum register_status cooked_read_part (int regnum, unsigned int offset, + unsigned int len, gdb_byte *buf); /* Read register REGNUM from the regcache and return a new value. This will call mark_value_bytes_unavailable as appropriate. */ struct value *cooked_read_value (int regnum); protected: - enum register_status read_part (int regnum, int offset, int len, void *in, - bool is_raw); + enum register_status read_part (int regnum, unsigned int offset, + unsigned int len, void *in, bool is_raw); }; /* Buffer of registers, can be read and written. */ @@ -311,11 +312,12 @@ public: /* Partial transfer of raw registers. Perform read, modify, write style operations. */ - void raw_write_part (int regnum, int offset, int len, const gdb_byte *buf); + void raw_write_part (int regnum, unsigned int offset, unsigned int len, + const gdb_byte *buf); /* Partial transfer of a cooked register. Perform read, modify, write style operations. */ - void cooked_write_part (int regnum, int offset, int len, + void cooked_write_part (int regnum, unsigned int offset, unsigned int len, const gdb_byte *buf); void supply_regset (const struct regset *regset, @@ -355,8 +357,9 @@ private: int regnum, const void *in_buf, void *out_buf, size_t size) const; - enum register_status write_part (int regnum, int offset, int len, - const void *out, bool is_raw); + enum register_status write_part (int regnum, unsigned int offset, + unsigned int len, const void *out, + bool is_raw); /* The address space of this register cache (for registers where it -- 2.7.4