From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id C97423858D20 for ; Fri, 20 Jan 2023 16:59:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C97423858D20 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=foss.arm.com Authentication-Results: sourceware.org; spf=none smtp.mailfrom=foss.arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id EC087113E; Fri, 20 Jan 2023 09:00:32 -0800 (PST) Received: from [10.57.37.229] (unknown [10.57.37.229]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 993E93F67D; Fri, 20 Jan 2023 08:59:50 -0800 (PST) Message-ID: <2adfc900-284d-7345-5e46-c64d76e9a221@foss.arm.com> Date: Fri, 20 Jan 2023 16:59:50 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: [PATCH][GCC] arm: Add support for new frame unwinding instruction "0xb5". Content-Language: en-GB To: Srinath Parvathaneni , gcc-patches@gcc.gnu.org Cc: richard.earnshaw@arm.com, kyrylo.tkachov@arm.com References: From: Richard Earnshaw In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3495.6 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,NICE_REPLY_A,SPF_HELO_NONE,SPF_NONE,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: On 10/11/2022 10:37, Srinath Parvathaneni via Gcc-patches wrote: > Hi, > > This patch adds support for Arm frame unwinding instruction "0xb5" [1]. When > an exception is taken and "0xb5" instruction is encounter during runtime > stack-unwinding, we use effective vsp as modifier in pointer authentication. > On completion of stack unwinding if "0xb5" instruction is not encountered > then CFA will be used as modifier in pointer authentication. > > [1] https://github.com/ARM-software/abi-aa/releases/download/2022Q3/ehabi32.pdf > > Regression tested on arm-none-eabi target and found no regressions. > > Ok for master? > > Regards, > Srinath. > > gcc/ChangeLog: > > 2022-11-09 Srinath Parvathaneni > > * libgcc/config/arm/pr-support.c (__gnu_unwind_execute): Decode opcode > "0xb5". > > > ############### Attachment also inlined for ease of reply ############### > > > diff --git a/libgcc/config/arm/pr-support.c b/libgcc/config/arm/pr-support.c > index e48854587c667a959aa66ccc4982231f63333ecc..73e4942a39b34a83c2da85def6b13e82ec501552 100644 > --- a/libgcc/config/arm/pr-support.c > +++ b/libgcc/config/arm/pr-support.c > @@ -107,7 +107,9 @@ __gnu_unwind_execute (_Unwind_Context * context, __gnu_unwind_state * uws) > _uw op; > int set_pc; > int set_pac = 0; > + int set_pac_sp = 0; > _uw reg; > + _uw sp; > > set_pc = 0; > for (;;) > @@ -124,10 +126,11 @@ __gnu_unwind_execute (_Unwind_Context * context, __gnu_unwind_state * uws) > #if defined(TARGET_HAVE_PACBTI) > if (set_pac) > { > - _uw sp; > _uw lr; > _uw pac; > - _Unwind_VRS_Get (context, _UVRSC_CORE, R_SP, _UVRSD_UINT32, &sp); > + if (!set_pac_sp) > + _Unwind_VRS_Get (context, _UVRSC_CORE, R_SP, _UVRSD_UINT32, > + &sp); > _Unwind_VRS_Get (context, _UVRSC_CORE, R_LR, _UVRSD_UINT32, &lr); > _Unwind_VRS_Get (context, _UVRSC_PAC, R_IP, > _UVRSD_UINT32, &pac); > @@ -259,7 +262,19 @@ __gnu_unwind_execute (_Unwind_Context * context, __gnu_unwind_state * uws) > continue; > } > > - if ((op & 0xfc) == 0xb4) /* Obsolete FPA. */ > + /* Use current VSP as modifier in PAC validation. */ > + if (op == 0xb5) > + { > + if (set_pac) > + _Unwind_VRS_Get (context, _UVRSC_CORE, R_SP, _UVRSD_UINT32, > + &sp); > + else > + return _URC_FAILURE; I don't think you need to worry about the case when set_pac is false; in fact, I don't think you need to even test set_pac here. It's harmless if this opcode appears and then we never do the authentication, so just record the SP value at this point. > + set_pac_sp = 1; > + continue; > + } > + > + if ((op & 0xfd) == 0xb6) /* Obsolete FPA. */ No, this is logically impossible (0xfd is binary 1111_1101, while 0xb6 is binary 1011_110 and thus bit 2 will never be set after the mask). But you don't need to change the condition here at all, because we've already taken out the case you're worried about immediately above (and ended that block with a 'continue'). > return _URC_FAILURE; > > /* op & 0xf8 == 0xb8. */ > > > R.