[v3,3/4] aarch64 DWARF: add new CFI directive for PAuth_LR

Message ID 20250113112206.1071596-4-matthieu.longo@arm.com
State New
Headers
Series aarch64: add DWARF and SFrame support for new CFI directive used for PAuth_LR |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Matthieu Longo Jan. 13, 2025, 11:22 a.m. UTC
  This patch adds a new CFI directive (cfi_negate_ra_state_with_pc) which
set an additional bit in the RA state to inform that RA was signed with
SP but also PC as an additional diversifier.

RA state | Description
0b00     | Return address not signed (default if no cfi_negate_ra_state*)
0b01     | Return address signed with SP (cfi_negate_ra_state)
0b10     | Invalid state
0b11     | Return address signed with SP+PC (cfi_negate_ra_state_with_pc)
---
 bfd/elf-eh-frame.c |  1 +
 binutils/dwarf.c   |  5 +++++
 gas/dw2gencfi.c    | 10 ++++++++++
 gas/scfidw2gen.c   |  1 +
 include/dwarf2.def |  2 ++
 5 files changed, 19 insertions(+)
  

Comments

Indu Bhagat Jan. 13, 2025, 11:29 p.m. UTC | #1
On 1/13/25 3:22 AM, Matthieu Longo wrote:
> This patch adds a new CFI directive (cfi_negate_ra_state_with_pc) which
> set an additional bit in the RA state to inform that RA was signed with
> SP but also PC as an additional diversifier.
> 
> RA state | Description
> 0b00     | Return address not signed (default if no cfi_negate_ra_state*)
> 0b01     | Return address signed with SP (cfi_negate_ra_state)
> 0b10     | Invalid state
> 0b11     | Return address signed with SP+PC (cfi_negate_ra_state_with_pc)
> ---
>   bfd/elf-eh-frame.c |  1 +
>   binutils/dwarf.c   |  5 +++++
>   gas/dw2gencfi.c    | 10 ++++++++++
>   gas/scfidw2gen.c   |  1 +
>   include/dwarf2.def |  2 ++
>   5 files changed, 19 insertions(+)
> 

Comments below. The changes look OK to me for the scfi*; I cannot 
approve the other components.

Thanks

> diff --git a/bfd/elf-eh-frame.c b/bfd/elf-eh-frame.c
> index d903e27a676..b6f5078bb33 100644
> --- a/bfd/elf-eh-frame.c
> +++ b/bfd/elf-eh-frame.c
> @@ -359,6 +359,7 @@ skip_cfa_op (bfd_byte **iter, bfd_byte *end, unsigned int encoded_ptr_width)
>       case DW_CFA_remember_state:
>       case DW_CFA_restore_state:
>       case DW_CFA_GNU_window_save:
> +    case DW_CFA_AARCH64_negate_ra_state_with_pc:
>         /* No arguments.  */
>         return true;
>   
> diff --git a/binutils/dwarf.c b/binutils/dwarf.c
> index 626efb2eb9a..8e004cea839 100644
> --- a/binutils/dwarf.c
> +++ b/binutils/dwarf.c
> @@ -10408,6 +10408,11 @@ display_debug_frames (struct dwarf_section *section,
>   	      fc->pc_begin += ofs;
>   	      break;
>   
> +	    case DW_CFA_AARCH64_negate_ra_state_with_pc:
> +	      if (! do_debug_frames_interp)
> +		printf ("  DW_CFA_AARCH64_negate_ra_state_with_pc\n");
> +	      break;
> +
>   	    case DW_CFA_GNU_window_save:
>   	      if (! do_debug_frames_interp)
>   		printf ("  %s\n", DW_CFA_GNU_window_save_name[is_aarch64]);
> diff --git a/gas/dw2gencfi.c b/gas/dw2gencfi.c
> index fbeb697af09..c984d8326d0 100644
> --- a/gas/dw2gencfi.c
> +++ b/gas/dw2gencfi.c
> @@ -714,6 +714,7 @@ const pseudo_typeS cfi_pseudo_table[] =
>       { "cfi_restore_state", dot_cfi, DW_CFA_restore_state },
>       { "cfi_window_save", dot_cfi, DW_CFA_GNU_window_save },
>       { "cfi_negate_ra_state", dot_cfi, DW_CFA_AARCH64_negate_ra_state },
> +    { "cfi_negate_ra_state_with_pc", dot_cfi, DW_CFA_AARCH64_negate_ra_state_with_pc },

Also keep the cfi_pseudo_table[] for the case when "#else /* 
TARGET_USE_CFIPOP */" in the file in sync ?

>       { "cfi_escape", dot_cfi_escape, 0 },
>       { "cfi_signal_frame", dot_cfi, CFI_signal_frame },
>       { "cfi_personality", dot_cfi_personality, 0 },
> @@ -914,6 +915,10 @@ dot_cfi (int arg)
>         cfi_add_CFA_insn (DW_CFA_GNU_window_save);
>         break;
>   
> +    case DW_CFA_AARCH64_negate_ra_state_with_pc:
> +      cfi_add_CFA_insn (DW_CFA_AARCH64_negate_ra_state_with_pc);
> +      break;
> +
>       case CFI_signal_frame:
>         frchain_now->frch_cfi_data->cur_fde_data->signal_frame = 1;
>         break;
> @@ -1754,6 +1759,10 @@ output_cfi_insn (struct cfi_insn_data *insn)
>         out_one (DW_CFA_GNU_window_save);
>         break;
>   
> +    case DW_CFA_AARCH64_negate_ra_state_with_pc:
> +      out_one (DW_CFA_AARCH64_negate_ra_state_with_pc);
> +      break;
> +
>       case CFI_escape:
>         {
>   	struct cfi_escape_data *e;
> @@ -2212,6 +2221,7 @@ cfi_change_reg_numbers (struct cfi_insn_data *insn, segT ccseg)
>   	case DW_CFA_remember_state:
>   	case DW_CFA_restore_state:
>   	case DW_CFA_GNU_window_save:
> +	case DW_CFA_AARCH64_negate_ra_state_with_pc:
>   	case CFI_escape:
>   	case CFI_label:
>   	  break;
> diff --git a/gas/scfidw2gen.c b/gas/scfidw2gen.c
> index 7463207e170..9b3ad4b13e0 100644
> --- a/gas/scfidw2gen.c
> +++ b/gas/scfidw2gen.c
> @@ -113,6 +113,7 @@ const pseudo_typeS scfi_pseudo_table[] =
>       { "cfi_restore_state", dot_scfi_ignore, 0 },
>       { "cfi_window_save", dot_scfi_ignore, 0 },
>       { "cfi_negate_ra_state", dot_scfi_ignore, 0 },
> +    { "cfi_negate_ra_state_with_pc", dot_scfi_ignore, 0 },
>       { "cfi_escape", dot_scfi_ignore, 0 },
>       { "cfi_personality", dot_scfi_ignore, 0 },
>       { "cfi_personality_id", dot_scfi_ignore, 0 },
> diff --git a/include/dwarf2.def b/include/dwarf2.def
> index 63cb35560e7..477b2ca20c0 100644
> --- a/include/dwarf2.def
> +++ b/include/dwarf2.def
> @@ -785,6 +785,8 @@ DW_CFA (DW_CFA_hi_user, 0x3f)
>   
>   /* SGI/MIPS specific.  */
>   DW_CFA (DW_CFA_MIPS_advance_loc8, 0x1d)
> +/* AArch64 extensions. */

Nit: dot, space, space, end of comment

> +DW_CFA (DW_CFA_AARCH64_negate_ra_state_with_pc, 0x2c)
>   /* GNU extensions.
>      NOTE: DW_CFA_GNU_window_save is multiplexed on Sparc and AArch64.  */
>   DW_CFA (DW_CFA_GNU_window_save, 0x2d)
  
Matthieu Longo Jan. 14, 2025, 11 a.m. UTC | #2
On 2025-01-13 23:29, Indu Bhagat wrote:
> On 1/13/25 3:22 AM, Matthieu Longo wrote:
>> This patch adds a new CFI directive (cfi_negate_ra_state_with_pc) which
>> set an additional bit in the RA state to inform that RA was signed with
>> SP but also PC as an additional diversifier.
>>
>> RA state | Description
>> 0b00     | Return address not signed (default if no cfi_negate_ra_state*)
>> 0b01     | Return address signed with SP (cfi_negate_ra_state)
>> 0b10     | Invalid state
>> 0b11     | Return address signed with SP+PC (cfi_negate_ra_state_with_pc)
>> ---
>>   bfd/elf-eh-frame.c |  1 +
>>   binutils/dwarf.c   |  5 +++++
>>   gas/dw2gencfi.c    | 10 ++++++++++
>>   gas/scfidw2gen.c   |  1 +
>>   include/dwarf2.def |  2 ++
>>   5 files changed, 19 insertions(+)
>>
> 
> Comments below. The changes look OK to me for the scfi*; I cannot 
> approve the other components.
> 
> Thanks

Jan Beulich also approved the changes in v2: 
https://inbox.sourceware.org/binutils/ddbf42f2-2b19-4faa-84d1-5f2a77b31a50@suse.com/

Thanks.

>> diff --git a/bfd/elf-eh-frame.c b/bfd/elf-eh-frame.c
>> index d903e27a676..b6f5078bb33 100644
>> --- a/bfd/elf-eh-frame.c
>> +++ b/bfd/elf-eh-frame.c
>> @@ -359,6 +359,7 @@ skip_cfa_op (bfd_byte **iter, bfd_byte *end, 
>> unsigned int encoded_ptr_width)
>>       case DW_CFA_remember_state:
>>       case DW_CFA_restore_state:
>>       case DW_CFA_GNU_window_save:
>> +    case DW_CFA_AARCH64_negate_ra_state_with_pc:
>>         /* No arguments.  */
>>         return true;
>> diff --git a/binutils/dwarf.c b/binutils/dwarf.c
>> index 626efb2eb9a..8e004cea839 100644
>> --- a/binutils/dwarf.c
>> +++ b/binutils/dwarf.c
>> @@ -10408,6 +10408,11 @@ display_debug_frames (struct dwarf_section 
>> *section,
>>             fc->pc_begin += ofs;
>>             break;
>> +        case DW_CFA_AARCH64_negate_ra_state_with_pc:
>> +          if (! do_debug_frames_interp)
>> +        printf ("  DW_CFA_AARCH64_negate_ra_state_with_pc\n");
>> +          break;
>> +
>>           case DW_CFA_GNU_window_save:
>>             if (! do_debug_frames_interp)
>>           printf ("  %s\n", DW_CFA_GNU_window_save_name[is_aarch64]);
>> diff --git a/gas/dw2gencfi.c b/gas/dw2gencfi.c
>> index fbeb697af09..c984d8326d0 100644
>> --- a/gas/dw2gencfi.c
>> +++ b/gas/dw2gencfi.c
>> @@ -714,6 +714,7 @@ const pseudo_typeS cfi_pseudo_table[] =
>>       { "cfi_restore_state", dot_cfi, DW_CFA_restore_state },
>>       { "cfi_window_save", dot_cfi, DW_CFA_GNU_window_save },
>>       { "cfi_negate_ra_state", dot_cfi, DW_CFA_AARCH64_negate_ra_state },
>> +    { "cfi_negate_ra_state_with_pc", dot_cfi, 
>> DW_CFA_AARCH64_negate_ra_state_with_pc },
> 
> Also keep the cfi_pseudo_table[] for the case when "#else /* 
> TARGET_USE_CFIPOP */" in the file in sync ?

Fixed. Thanks for catching this. cfi_negate_ra_state was also absent, so 
I added it along cfi_negate_ra_state_with_pc.

>>       { "cfi_escape", dot_cfi_escape, 0 },
>>       { "cfi_signal_frame", dot_cfi, CFI_signal_frame },
>>       { "cfi_personality", dot_cfi_personality, 0 },
>> @@ -914,6 +915,10 @@ dot_cfi (int arg)
>>         cfi_add_CFA_insn (DW_CFA_GNU_window_save);
>>         break;
>> +    case DW_CFA_AARCH64_negate_ra_state_with_pc:
>> +      cfi_add_CFA_insn (DW_CFA_AARCH64_negate_ra_state_with_pc);
>> +      break;
>> +
>>       case CFI_signal_frame:
>>         frchain_now->frch_cfi_data->cur_fde_data->signal_frame = 1;
>>         break;
>> @@ -1754,6 +1759,10 @@ output_cfi_insn (struct cfi_insn_data *insn)
>>         out_one (DW_CFA_GNU_window_save);
>>         break;
>> +    case DW_CFA_AARCH64_negate_ra_state_with_pc:
>> +      out_one (DW_CFA_AARCH64_negate_ra_state_with_pc);
>> +      break;
>> +
>>       case CFI_escape:
>>         {
>>       struct cfi_escape_data *e;
>> @@ -2212,6 +2221,7 @@ cfi_change_reg_numbers (struct cfi_insn_data 
>> *insn, segT ccseg)
>>       case DW_CFA_remember_state:
>>       case DW_CFA_restore_state:
>>       case DW_CFA_GNU_window_save:
>> +    case DW_CFA_AARCH64_negate_ra_state_with_pc:
>>       case CFI_escape:
>>       case CFI_label:
>>         break;
>> diff --git a/gas/scfidw2gen.c b/gas/scfidw2gen.c
>> index 7463207e170..9b3ad4b13e0 100644
>> --- a/gas/scfidw2gen.c
>> +++ b/gas/scfidw2gen.c
>> @@ -113,6 +113,7 @@ const pseudo_typeS scfi_pseudo_table[] =
>>       { "cfi_restore_state", dot_scfi_ignore, 0 },
>>       { "cfi_window_save", dot_scfi_ignore, 0 },
>>       { "cfi_negate_ra_state", dot_scfi_ignore, 0 },
>> +    { "cfi_negate_ra_state_with_pc", dot_scfi_ignore, 0 },
>>       { "cfi_escape", dot_scfi_ignore, 0 },
>>       { "cfi_personality", dot_scfi_ignore, 0 },
>>       { "cfi_personality_id", dot_scfi_ignore, 0 },
>> diff --git a/include/dwarf2.def b/include/dwarf2.def
>> index 63cb35560e7..477b2ca20c0 100644
>> --- a/include/dwarf2.def
>> +++ b/include/dwarf2.def
>> @@ -785,6 +785,8 @@ DW_CFA (DW_CFA_hi_user, 0x3f)
>>   /* SGI/MIPS specific.  */
>>   DW_CFA (DW_CFA_MIPS_advance_loc8, 0x1d)
>> +/* AArch64 extensions. */
> 
> Nit: dot, space, space, end of comment

Fixed.

>> +DW_CFA (DW_CFA_AARCH64_negate_ra_state_with_pc, 0x2c)
>>   /* GNU extensions.
>>      NOTE: DW_CFA_GNU_window_save is multiplexed on Sparc and 
>> AArch64.  */
>>   DW_CFA (DW_CFA_GNU_window_save, 0x2d)
>
  
Thiago Jung Bauermann Feb. 2, 2025, 9:21 p.m. UTC | #3
Hello,

Matthieu Longo <matthieu.longo@arm.com> writes:

> diff --git a/include/dwarf2.def b/include/dwarf2.def
> index 63cb35560e7..477b2ca20c0 100644
> --- a/include/dwarf2.def
> +++ b/include/dwarf2.def
> @@ -785,6 +785,8 @@ DW_CFA (DW_CFA_hi_user, 0x3f)
>
>  /* SGI/MIPS specific.  */
>  DW_CFA (DW_CFA_MIPS_advance_loc8, 0x1d)
> +/* AArch64 extensions. */
> +DW_CFA (DW_CFA_AARCH64_negate_ra_state_with_pc, 0x2c)
>  /* GNU extensions.
>     NOTE: DW_CFA_GNU_window_save is multiplexed on Sparc and AArch64.  */
>  DW_CFA (DW_CFA_GNU_window_save, 0x2d)

Would it be possible to send this part of the patch to GCC as well?

Currently building GCC for arm-eabi using a combined source tree is
failing with:

  CC       elf-eh-frame.lo
../../combined-tree-src/bfd/elf-eh-frame.c: In function ‘skip_cfa_op’:
../../combined-tree-src/bfd/elf-eh-frame.c:362:10: error: ‘DW_CFA_AARCH64_negate_ra_state_with_pc’ undeclared (first use in this function); did you mean ‘DW_CFA_AARCH64_negate_ra_state’?
  362 |     case DW_CFA_AARCH64_negate_ra_state_with_pc:
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |          DW_CFA_AARCH64_negate_ra_state
../../combined-tree-src/bfd/elf-eh-frame.c:362:10: note: each undeclared identifier is reported only once for each function it appears in
make[4]: *** [Makefile:1762: elf-eh-frame.lo] Error 1

When I apply the change above the build succeeds.

--
Thiago
  

Patch

diff --git a/bfd/elf-eh-frame.c b/bfd/elf-eh-frame.c
index d903e27a676..b6f5078bb33 100644
--- a/bfd/elf-eh-frame.c
+++ b/bfd/elf-eh-frame.c
@@ -359,6 +359,7 @@  skip_cfa_op (bfd_byte **iter, bfd_byte *end, unsigned int encoded_ptr_width)
     case DW_CFA_remember_state:
     case DW_CFA_restore_state:
     case DW_CFA_GNU_window_save:
+    case DW_CFA_AARCH64_negate_ra_state_with_pc:
       /* No arguments.  */
       return true;
 
diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index 626efb2eb9a..8e004cea839 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -10408,6 +10408,11 @@  display_debug_frames (struct dwarf_section *section,
 	      fc->pc_begin += ofs;
 	      break;
 
+	    case DW_CFA_AARCH64_negate_ra_state_with_pc:
+	      if (! do_debug_frames_interp)
+		printf ("  DW_CFA_AARCH64_negate_ra_state_with_pc\n");
+	      break;
+
 	    case DW_CFA_GNU_window_save:
 	      if (! do_debug_frames_interp)
 		printf ("  %s\n", DW_CFA_GNU_window_save_name[is_aarch64]);
diff --git a/gas/dw2gencfi.c b/gas/dw2gencfi.c
index fbeb697af09..c984d8326d0 100644
--- a/gas/dw2gencfi.c
+++ b/gas/dw2gencfi.c
@@ -714,6 +714,7 @@  const pseudo_typeS cfi_pseudo_table[] =
     { "cfi_restore_state", dot_cfi, DW_CFA_restore_state },
     { "cfi_window_save", dot_cfi, DW_CFA_GNU_window_save },
     { "cfi_negate_ra_state", dot_cfi, DW_CFA_AARCH64_negate_ra_state },
+    { "cfi_negate_ra_state_with_pc", dot_cfi, DW_CFA_AARCH64_negate_ra_state_with_pc },
     { "cfi_escape", dot_cfi_escape, 0 },
     { "cfi_signal_frame", dot_cfi, CFI_signal_frame },
     { "cfi_personality", dot_cfi_personality, 0 },
@@ -914,6 +915,10 @@  dot_cfi (int arg)
       cfi_add_CFA_insn (DW_CFA_GNU_window_save);
       break;
 
+    case DW_CFA_AARCH64_negate_ra_state_with_pc:
+      cfi_add_CFA_insn (DW_CFA_AARCH64_negate_ra_state_with_pc);
+      break;
+
     case CFI_signal_frame:
       frchain_now->frch_cfi_data->cur_fde_data->signal_frame = 1;
       break;
@@ -1754,6 +1759,10 @@  output_cfi_insn (struct cfi_insn_data *insn)
       out_one (DW_CFA_GNU_window_save);
       break;
 
+    case DW_CFA_AARCH64_negate_ra_state_with_pc:
+      out_one (DW_CFA_AARCH64_negate_ra_state_with_pc);
+      break;
+
     case CFI_escape:
       {
 	struct cfi_escape_data *e;
@@ -2212,6 +2221,7 @@  cfi_change_reg_numbers (struct cfi_insn_data *insn, segT ccseg)
 	case DW_CFA_remember_state:
 	case DW_CFA_restore_state:
 	case DW_CFA_GNU_window_save:
+	case DW_CFA_AARCH64_negate_ra_state_with_pc:
 	case CFI_escape:
 	case CFI_label:
 	  break;
diff --git a/gas/scfidw2gen.c b/gas/scfidw2gen.c
index 7463207e170..9b3ad4b13e0 100644
--- a/gas/scfidw2gen.c
+++ b/gas/scfidw2gen.c
@@ -113,6 +113,7 @@  const pseudo_typeS scfi_pseudo_table[] =
     { "cfi_restore_state", dot_scfi_ignore, 0 },
     { "cfi_window_save", dot_scfi_ignore, 0 },
     { "cfi_negate_ra_state", dot_scfi_ignore, 0 },
+    { "cfi_negate_ra_state_with_pc", dot_scfi_ignore, 0 },
     { "cfi_escape", dot_scfi_ignore, 0 },
     { "cfi_personality", dot_scfi_ignore, 0 },
     { "cfi_personality_id", dot_scfi_ignore, 0 },
diff --git a/include/dwarf2.def b/include/dwarf2.def
index 63cb35560e7..477b2ca20c0 100644
--- a/include/dwarf2.def
+++ b/include/dwarf2.def
@@ -785,6 +785,8 @@  DW_CFA (DW_CFA_hi_user, 0x3f)
 
 /* SGI/MIPS specific.  */
 DW_CFA (DW_CFA_MIPS_advance_loc8, 0x1d)
+/* AArch64 extensions. */
+DW_CFA (DW_CFA_AARCH64_negate_ra_state_with_pc, 0x2c)
 /* GNU extensions.
    NOTE: DW_CFA_GNU_window_save is multiplexed on Sparc and AArch64.  */
 DW_CFA (DW_CFA_GNU_window_save, 0x2d)