[PATCH][Binutils][GAS] arm: Add cfi expression support for ra_auth_code

[PATCH][Binutils][GAS] arm: Add cfi expression support for ra_auth_code

[Victor Do Nascimento]

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 (&regname, REG_TYPE_PSEUDO);
+  switch (reg)
+  {
+    case 12:
+      return 143;
+    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
-- 
2.36.1

Re: [PATCH][Binutils][GAS] arm: Add cfi expression support for ra_auth_code

[Richard Earnshaw]

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 (&regname, 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.

> +    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

Re: [PATCH][Binutils][GAS] arm: Add cfi expression support for ra_auth_code

[Victor L. Do Nascimento]

Richard Earnshaw <Richard.Earnshaw@foss.arm.com> 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 (&regname, 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.

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

Re: [PATCH][Binutils][GAS] arm: Add cfi expression support for ra_auth_code

[Richard Earnshaw]

On 19/07/2022 12:28, Victor L. Do Nascimento wrote:
> Richard Earnshaw <Richard.Earnshaw@foss.arm.com> 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 (&regname, 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