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 D58DC3858439 for ; Tue, 19 Jul 2022 12:01:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D58DC3858439 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 10672139F; Tue, 19 Jul 2022 05:01:12 -0700 (PDT) Received: from [10.2.78.52] (unknown [10.2.78.52]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 00F1D3F766; Tue, 19 Jul 2022 05:01:10 -0700 (PDT) Message-ID: <316845be-66f0-f154-2cbf-bac5ce39dbf2@foss.arm.com> Date: Tue, 19 Jul 2022 13:01:09 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH][Binutils][GAS] arm: Add cfi expression support for ra_auth_code Content-Language: en-GB To: "Victor L. Do Nascimento" Cc: binutils@sourceware.org, richard.earnshaw@arm.com References: <20220718160129.15652-1-victor.donascimento@arm.com> <137e4b3a-0d13-1fbe-8e44-56cf5a3447d2@foss.arm.com> From: Richard Earnshaw In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3496.8 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 X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Jul 2022 12:01:13 -0000 On 19/07/2022 12:28, Victor L. Do Nascimento wrote: > Richard Earnshaw writes: > >> On 18/07/2022 17:01, Victor Do Nascimento via Binutils wrote: >>> Hi all, >>> This patch extends assembler support for the use of ra_auth_code register, >>> particularly in the context of CFI directives, mapping the register to >>> the correct DWARF-2 register number. >>> Tested for arm-none-eabi. >>> Thanks, >>> Victor. >>> gas/Changelog: >>> * gas/config/tc-arm.c (tc_arm_regname_to_dw2regnum): Add >>> REG_TYPE_PSEUDO handling. >>> * gas/testsuite/gas/arm/cfi-pacbti-m-readelf.d: New. >>> * gas/testsuite/gas/arm/cfi-pacbti-m.s: New. >>> --- >>> gas/config/tc-arm.c | 9 ++++++ >>> gas/testsuite/gas/arm/cfi-pacbti-m-readelf.d | 31 ++++++++++++++++++++ >>> gas/testsuite/gas/arm/cfi-pacbti-m.s | 22 ++++++++++++++ >>> 3 files changed, 62 insertions(+) >>> create mode 100644 gas/testsuite/gas/arm/cfi-pacbti-m-readelf.d >>> create mode 100644 gas/testsuite/gas/arm/cfi-pacbti-m.s >>> diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c >>> index 2e6d175482e..bf912ccf2d5 100644 >>> --- a/gas/config/tc-arm.c >>> +++ b/gas/config/tc-arm.c >>> @@ -27933,6 +27933,15 @@ tc_arm_regname_to_dw2regnum (char *regname) >>> if (reg != FAIL) >>> return reg + 256; >>> + reg = arm_reg_parse (®name, REG_TYPE_PSEUDO); >>> + switch (reg) >>> + { >>> + case 12: >>> + return 143; >> >> Sorry, this doesn't look right. The comment where the pseudos are defined says: >> >> /* DWARF ABI defines RA_AUTH_CODE to 143. It also reserves 134-142 for future >> expansion. RA_AUTH_CODE here is given the value 143 % 134 to make it easy >> for tc_arm_regname_to_dw2regnum to translate to DWARF reg number using >> 134 + reg_number should the range 134 to 142 be used for more pseudo regs >> in the future. This also helps fit RA_AUTH_CODE into a bitmask. */ >> >> though I'll note that 143 % 134 is 9, not 12. >> >> So really, the code here should be adding back the base for the range that it >> covers rather than just handling a special case. >> >> And, of course, either the comment needs fixing, or the assigned internal number >> needs fixing. >> >> R. > > Thanks for catchig this stale comment. I should have updated this when I > re-numbered RA_AUT_CODE. > > The numerical mismatch between internal numbering of the pseudo-register > arising from the modulo operation described above and the physical reg > it mapped onto made the handling of mixed register lists (e.g. via > s_arm_unwind_save_mixed) seem unnecessarily complex at the time of > implementation. A pragmatic, albeit less elegant, solution was to have > RA_AUTH_CODE pseudo be numbered according to the restrictions imposed by > the PAC/PACBTI assembly instructions, which save the generated PAC code > in R12. > > If you've no objections, will update the comment and resubmit. Trying to make RA_AUTH_CODE sound like an alias of R12 sounds like an egregious hack to me. We shouldn't be doing that. And secondly, we shouldn't be using manifest constants in the body of the source code. R. > > V. > >>> + default: >>> + break; >>> + } >>> + >>> return FAIL; >>> } >>> diff --git a/gas/testsuite/gas/arm/cfi-pacbti-m-readelf.d >>> b/gas/testsuite/gas/arm/cfi-pacbti-m-readelf.d >>> new file mode 100644 >>> index 00000000000..997ea75f179 >>> --- /dev/null >>> +++ b/gas/testsuite/gas/arm/cfi-pacbti-m-readelf.d >>> @@ -0,0 +1,31 @@ >>> +#readelf: -wf >>> +#source: cfi-pacbti-m.s >>> +#name: Call Frame information for Armv8.1-M.Mainline PACBTI extension >>> +# This test is only valid on ELF based ports. >>> +#notarget: *-*-pe *-*-wince >>> +# VxWorks needs a special variant of this file. >>> +#skip: *-*-vxworks* >>> + >>> +Contents of the .eh_frame section: >>> + >>> + >>> +00000000 00000010 00000000 CIE >>> + Version: 1 >>> + Augmentation: "zR" >>> + Code alignment factor: 2 >>> + Data alignment factor: -4 >>> + Return address column: 14 >>> + Augmentation data: 1b >>> + DW_CFA_def_cfa: r13 ofs 0 >>> + >>> +00000014 00000020 00000018 FDE cie=00000000 pc=00000000..0000000c >>> + DW_CFA_advance_loc: 4 to 00000004 >>> + DW_CFA_register: r143 in r12 >>> + DW_CFA_advance_loc: 4 to 00000008 >>> + DW_CFA_def_cfa_offset: 8 >>> + DW_CFA_offset: r14 at cfa-8 >>> + DW_CFA_offset: r12 at cfa-4 >>> + DW_CFA_advance_loc: 4 to 0000000c >>> + DW_CFA_restore_extended: r143 >>> + DW_CFA_restore: r14 >>> + DW_CFA_def_cfa_offset: 0 >>> diff --git a/gas/testsuite/gas/arm/cfi-pacbti-m.s b/gas/testsuite/gas/arm/cfi-pacbti-m.s >>> new file mode 100644 >>> index 00000000000..515400d86f5 >>> --- /dev/null >>> +++ b/gas/testsuite/gas/arm/cfi-pacbti-m.s >>> @@ -0,0 +1,22 @@ >>> + .arch armv8.1-m.main >>> + .arch_extension pacbti >>> + .eabi_attribute Tag_PAC_extension, 2 >>> + .eabi_attribute Tag_BTI_extension, 2 >>> + .eabi_attribute Tag_BTI_use, 1 >>> + .eabi_attribute Tag_PACRET_use, 1 >>> + .syntax unified >>> + .text >>> + .thumb >>> +.Lstart: >>> + .cfi_startproc >>> + pacbti ip, lr, sp >>> + .cfi_register ra_auth_code, ip >>> + push {ip, lr} >>> + .cfi_def_cfa_offset 8 >>> + .cfi_offset lr, -8 >>> + .cfi_offset ip, -4 >>> + pop {ip, lr} >>> + .cfi_restore 143 >>> + .cfi_restore 14 >>> + .cfi_def_cfa_offset 0 >>> + .cfi_endproc