From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id wNC8EeEpfWPNeRsAWB0awg (envelope-from ) for ; Tue, 22 Nov 2022 14:58:25 -0500 Received: by simark.ca (Postfix, from userid 112) id 380491E124; Tue, 22 Nov 2022 14:58:25 -0500 (EST) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=ae9XKXtV; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A, RCVD_IN_DNSWL_MED,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id F319B1E0CB for ; Tue, 22 Nov 2022 14:58:23 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B177B3858421 for ; Tue, 22 Nov 2022 19:58:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B177B3858421 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1669147102; bh=o0T9bRhHQVoUdiHUOdeEFUwFqN9sk7eRuP3GoDF3Pzs=; h=Date:Subject:To:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=ae9XKXtVBbi991XrY68I14Ev8VGgYeZ65Furbx5S9/b2HyfJlNI/IwN2+x9Kd8HPe mYW+2cuNvBkeIZQWJK0h7Sq/8/UdKpI78hIip/vWUxsmcN2iW/1my2iVoAmlX2XHf7 C7xJe9nYF5g6xKuk1jY0fh/u0kcoaGjfwHfHAAME= Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id BC92B3858C1F for ; Tue, 22 Nov 2022 19:56:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org BC92B3858C1F Received: from [172.16.0.64] (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 1E1C01E0CB; Tue, 22 Nov 2022 14:56:09 -0500 (EST) Message-ID: Date: Tue, 22 Nov 2022 14:56:08 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.1 Subject: Re: [PATCH 1/5] regcache: Add collect/supply_regset variants that accept a register base. Content-Language: fr To: John Baldwin , gdb-patches@sourceware.org References: <20220708005816.9408-1-jhb@FreeBSD.org> <20220708005816.9408-2-jhb@FreeBSD.org> In-Reply-To: <20220708005816.9408-2-jhb@FreeBSD.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 7/7/22 20:58, John Baldwin wrote: > Some register sets described by an array of regcache_map_entry > structures do not have fixed register numbers in their associated > architecture but do describe a block of registers whose numbers are at > fixed offsets relative to some base register value. An example of > this are the TLS register sets for the ARM and AArch64 architectures. > > Currently OS-specific architectures create register maps and register > sets dynamically using the register base number. However, this > requires duplicating the code to create the register map and register > set. To reduce duplication, add variants of the collect_regset and > supply_regset regcache methods which accept a base register number. > For valid register map entries (i.e. not REGCACHE_MAP_SKIP), add this > base register number to the value from the map entry to determine the > final register number. The patch LGTM. I have some small comments, once addressed you can put my: Approved-By: Simon Marchi > --- > gdb/regcache.c | 28 +++++++++++++++++++++++++--- > gdb/regcache.h | 24 ++++++++++++++++++++++-- > 2 files changed, 47 insertions(+), 5 deletions(-) > > diff --git a/gdb/regcache.c b/gdb/regcache.c > index 037659ef8fa..1db3d972ef8 100644 > --- a/gdb/regcache.c > +++ b/gdb/regcache.c > @@ -1180,7 +1180,7 @@ regcache::transfer_regset_register (struct regcache *out_regcache, int regnum, > /* See regcache.h. */ > > void > -regcache::transfer_regset (const struct regset *regset, > +regcache::transfer_regset (const struct regset *regset, int regbase, > struct regcache *out_regcache, > int regnum, const gdb_byte *in_buf, > gdb_byte *out_buf, size_t size) const > @@ -1195,6 +1195,9 @@ regcache::transfer_regset (const struct regset *regset, > int regno = map->regno; > int slot_size = map->size; > > + if (regno != REGCACHE_MAP_SKIP) > + regno += regbase; > + > if (slot_size == 0 && regno != REGCACHE_MAP_SKIP) > slot_size = m_descr->sizeof_register[regno]; > > @@ -1242,7 +1245,18 @@ void > regcache::supply_regset (const struct regset *regset, > int regnum, const void *buf, size_t size) > { > - transfer_regset (regset, this, regnum, (const gdb_byte *) buf, nullptr, size); > + transfer_regset (regset, 0, this, regnum, (const gdb_byte *) buf, nullptr, > + size); > +} Can the old regcache::{supply,collect}_regset (without regbase) become trivial wrappers around the new ones (with regbase)? I would put the implementation directly in the .h if doing that. > + > +/* See regcache.h. */ > + > +void > +regcache::supply_regset (const struct regset *regset, int regbase, > + int regnum, const void *buf, size_t size) > +{ > + transfer_regset (regset, regbase, this, regnum, (const gdb_byte *) buf, > + nullptr, size); > } > > /* Collect register REGNUM from REGCACHE to BUF, using the register > @@ -1261,11 +1275,19 @@ void > regcache::collect_regset (const struct regset *regset, > int regnum, void *buf, size_t size) const > { > - transfer_regset (regset, nullptr, regnum, nullptr, (gdb_byte *) buf, size); > + transfer_regset (regset, 0, nullptr, regnum, nullptr, (gdb_byte *) buf, size); > } > > /* See regcache.h */ > > +void > +regcache::collect_regset (const struct regset *regset, int regbase, > + int regnum, void *buf, size_t size) const > +{ > + transfer_regset (regset, regbase, nullptr, regnum, nullptr, (gdb_byte *) buf, > + size); > +} > + > bool > regcache_map_supplies (const struct regcache_map_entry *map, int regnum, > struct gdbarch *gdbarch, size_t size) > diff --git a/gdb/regcache.h b/gdb/regcache.h > index 1dbba5ce9af..f01127b20fb 100644 > --- a/gdb/regcache.h > +++ b/gdb/regcache.h > @@ -150,6 +150,14 @@ extern void regcache_collect_regset (const struct regset *regset, > int regnum, void *buf, size_t size); > > > +extern void regcache_supply_regset (const struct regset *regset, int regbase, > + struct regcache *regcache, > + int regnum, const void *buf, > + size_t size); > +extern void regcache_collect_regset (const struct regset *regset, int regbase, > + const struct regcache *regcache, > + int regnum, void *buf, size_t size); These don't have a definition, I guess they are not needed. (Could we make a patch that removes regcache_supply_regset and regcache_collect_regset, since they just forward to the regcache methods?) > + belonging to the regset, otherwise just the register numbered > + REGNUM. The REGSET's 'regmap' field must point to an array of > + 'struct regcache_map_entry'. The valid register numbers in each > + entry in 'struct regcache_map_entry' are offset by REGBASE. */ > + > + void supply_regset (const struct regset *regset, int regbase, > + int regnum, const void *buf, size_t size); > + > + void collect_regset (const struct regset *regset, int regbase, int regnum, > + void *buf, size_t size) const; > + > void supply_regset (const struct regset *regset, > int regnum, const void *buf, size_t size); > > - > void collect_regset (const struct regset *regset, int regnum, > void *buf, size_t size) const; Can you document the last two, just by saying something like "Same as the above, but with REGBASE == 0"? Simon