From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id 72B3D3858C5E for ; Thu, 23 Feb 2023 16:27:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 72B3D3858C5E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: from r6.localdomain (82-217-174-174.cable.dynamic.v4.ziggo.nl [82.217.174.174]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 72AB230067D6; Thu, 23 Feb 2023 17:27:15 +0100 (CET) Received: by r6.localdomain (Postfix, from userid 1000) id 3941F34025A; Thu, 23 Feb 2023 17:27:15 +0100 (CET) Message-ID: <19848b9e2577638165448d4cbe5107e0b9df19b7.camel@klomp.org> Subject: Re: [PATCH 2/4] libdw, aarch64: Implement DW_CFA_AARCH64_negate_ra_state CFI instruction From: Mark Wielaard To: German Gomez , elfutils-devel@sourceware.org Date: Thu, 23 Feb 2023 17:27:15 +0100 In-Reply-To: <20220425140311.95231-3-german.gomez@arm.com> References: <20220425140311.95231-1-german.gomez@arm.com> <20220425140311.95231-3-german.gomez@arm.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) MIME-Version: 1.0 X-Spam-Status: No, score=-3035.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,RCVD_IN_BARRACUDACENTRAL,SPF_HELO_NONE,SPF_PASS,TXREP 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: Hi German, On Mon, 2022-04-25 at 14:03 +0000, German Gomez via Elfutils-devel wrote: > Implement DW_CFA_AARCH64_negate_ra_state in accordance with the DWARF > AARCH64 ABI [1]. >=20 > Followup commits will use the value of this register to remove the PAC > from return addresses. >=20 > [1] https://github.com/ARM-software/abi-aa/blob/main/aadwarf64/aadwarf64.= rst#44call-frame-instructions >=20 > Signed-off-by: German Gomez This looks good, but two comments below. > --- > libdw/cfi.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) >=20 > diff --git a/libdw/cfi.c b/libdw/cfi.c > index a73fb03f..f985b4d8 100644 > --- a/libdw/cfi.c > +++ b/libdw/cfi.c > @@ -125,6 +125,15 @@ execute_cfi (Dwarf_CFI *cache, > fs->regs[regno].value =3D (r_value); \ > } while (0) > =20 > + /* The AARCH64 DWARF ABI states that register 34 (ra_sign_state) must > + be initialized to 0. So do it before executing the CFI. */ > + if (cache->e_machine =3D=3D EM_AARCH64) > + { > + if (unlikely (! enough_registers (DW_AARCH64_RA_SIGN_STATE, &fs, &= result))) > + goto out; > + fs->regs[DW_AARCH64_RA_SIGN_STATE].value =3D 0; > + } Right. I thought this would be better expressed as part of the abi_cfi (see aarch64_abi_cfi in backends/aarch64_cfi.c). But that would require a DW_CFA_val_expression which we don't allow for abi_cfi. So this is probably the best way to do it. > while (program < end) > { > uint8_t opcode =3D *program++; > @@ -355,7 +364,10 @@ execute_cfi (Dwarf_CFI *cache, > { > /* Toggles the return address state, indicating whether > the return address is encrypted or not on > - aarch64. XXX not handled yet. */ > + aarch64. */ > + if (unlikely (! enough_registers (DW_AARCH64_RA_SIGN_STATE, &fs, = &result))) > + goto out; > + fs->regs[DW_AARCH64_RA_SIGN_STATE].value ^=3D 0x1; > } > else > { Looks good. Lets also move the DW_AARCH64_RA_SIGN_STATE definition into cfi.h (from libdw.h in the previous patch). Thanks, Mark