[v3,3/4] aarch64 DWARF: add new CFI directive 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
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
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)
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)
>
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
@@ -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;
@@ -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]);
@@ -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;
@@ -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 },
@@ -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)