From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id C5B633858CDB for ; Fri, 1 Dec 2023 16:21:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C5B633858CDB Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C5B633858CDB Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701447678; cv=none; b=WtmL9sthhGKqSaRgE/BE84RYOU/YYTZJ8nUV0TWHdIu2OeKsvuG3FR0MPqZOiRsAavlnlhTf+MD/9DfEzoge384B6Kc4oUkskDvH9rs9p4Ojf/zHVKX8Dp59xJSGX9auHjUYHhzxAefk+xJjCapfD6rn+2NPlmlAGFMA7bfI68A= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701447678; c=relaxed/simple; bh=pcGTG/hITgZ4G/2Wf0EGDARNORlV+7Gi30DllxRlOv8=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=HfkrKC/JZ12U35JmOdRsukGhuNY2gCcIn7vZtMpxU5fBFKebK8bGUoXTin1UR8KKJY98oIgp4Bji9BoKmHrIuZV0HG2HiRG8nqACj5rFVxYEuspD+CBXM+OmEHP5x1iur/D8KQH5OHWzU58hBJnOOjKdRS0kJV08uSClLVitgyk= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1701447676; bh=pcGTG/hITgZ4G/2Wf0EGDARNORlV+7Gi30DllxRlOv8=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=uwk51zfMOuQZQn/Fp7F/6tudXIigEaRcndY7tRLWxPsnMluYkFeDz8HnVgv+mIq9X txpe+ebav4ePqBVqGYLrWaD+qMHhF1HiwW7AbZU3GS8xFDbrzCjmnq9zAo4kIuZRZB 0nKpmiZpA/sLsxNPcGnsKgjBG6GbuLBVoQf7iPGg= Received: from [172.16.0.146] (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id A99171E091; Fri, 1 Dec 2023 11:21:14 -0500 (EST) Message-ID: <0674fb57-acde-4195-918f-5aa4090b3fa8@simark.ca> Date: Fri, 1 Dec 2023 11:21:13 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 2/2] gdb: add missing regcache_map_entry array null terminators in aarch64-linux-tdep.c Content-Language: fr To: Luis Machado , Simon Marchi , gdb-patches@sourceware.org Cc: John Baldwin References: <20231130212057.722990-1-simon.marchi@efficios.com> <20231130212057.722990-3-simon.marchi@efficios.com> <62a27fad-acaf-4d94-aa97-23cd751f4f2c@arm.com> From: Simon Marchi In-Reply-To: <62a27fad-acaf-4d94-aa97-23cd751f4f2c@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 12/1/23 04:12, Luis Machado wrote: > Hi Simon, > > On 11/30/23 21:20, Simon Marchi wrote: >> Fix two spots in aarch64-linux-tdep.c that build regcache_map_entry >> arrays without a null terminator. The null terminators are needed for >> regcache::transfer_regset to work properly. >> >> Some shower thoughts: I'm wondering: if a caller uses >> supply_regset/collect_regset with a regcache_map_entry array that >> describes exactly X bytes, and passes a buffer of X bytes, should a null >> terminator really be needed? regcache::transfer_regset could be written >> in a way that it exits as soon as the remaining buffer size reaches 0. >> Right now, even when it has consumed exactly the X bytes of the buffer, >> transfer_regset needs to read the following regcache_map_entry's count >> (expected to be 0) to realize it's done. If it exited its outer loop >> when `offs == size`, it would remove the need for the null terminator in >> this case. >> >> Shower thought #2: we could also bypass that by using array_view to pass >> regcache_map_entry arrays, removing the need for null terminators >> altogether, but that's a bigger change. >> >> Change-Id: I3224a314e1360b319438f32de8c81e70ab42e105 >> --- >> gdb/aarch64-linux-tdep.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c >> index cd99b33fed25..5be887a9c817 100644 >> --- a/gdb/aarch64-linux-tdep.c >> +++ b/gdb/aarch64-linux-tdep.c >> @@ -1497,7 +1497,9 @@ aarch64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch, >> /* Create this on the fly in order to handle the ZA register size. */ >> const struct regcache_map_entry za_regmap[] = >> { >> - { 1, tdep->sme_za_regnum, (int) std::pow (sve_vl_from_vq (tdep->sme_svq), 2) } >> + { 1, tdep->sme_za_regnum, >> + (int) std::pow (sve_vl_from_vq (tdep->sme_svq), 2) }, >> + { 0 } >> }; >> >> const struct regset aarch64_linux_za_regset = >> @@ -1518,7 +1520,8 @@ aarch64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch, >> { >> const struct regcache_map_entry zt_regmap[] = >> { >> - { 1, tdep->sme2_zt0_regnum, AARCH64_SME2_ZT0_SIZE } >> + { 1, tdep->sme2_zt0_regnum, AARCH64_SME2_ZT0_SIZE }, >> + { 0 } >> }; >> >> /* We set the register set size to REGSET_VARIABLE_SIZE here because > > Yeah, that was a silly mistake. Glad it was spotted. > > I failed to spot the missing terminators for these two regcache_map_entry's, > as I see them in other regcache_map_entry's. > > This is OK, thanks! > > Approved-By: Luis Machado Thanks, pushed both patches. Simon