[PATCH v2 2/2] Enable arch-specific CFI directives and DWARF instructions only when required by the target (part 1)

[Indu Bhagat]

On 4/25/25 7:02 AM, Matthieu Longo wrote:
> On 2025-04-25 14:10, Jan Beulich wrote:
>> On 25.04.2025 13:04, Matthieu Longo wrote:
>>> --- a/gas/dw2gencfi.c
>>> +++ b/gas/dw2gencfi.c
>>> @@ -720,9 +720,16 @@ const pseudo_typeS cfi_pseudo_table[] =
>>>       { "cfi_same_value", dot_cfi, DW_CFA_same_value },
>>>       { "cfi_remember_state", dot_cfi, DW_CFA_remember_state },
>>>       { "cfi_restore_state", dot_cfi, DW_CFA_restore_state },
>>> -    { "cfi_window_save", dot_cfi, DW_CFA_GNU_window_save },
>>> +#if TC_AARCH64
>>> +    /* cfi_window_save is an alias of cfi_negate_ra_state which is 
>>> kept for
>>> +       backward-compatibility concerns.  */
>>> +    { "cfi_window_save", dot_cfi, DW_CFA_AARCH64_negate_ra_state },
>>
>> Why's this of concern only for Arm64?
>>
> 
> I am not sure that I understand what your question is here.
> cfi_window_save is only used by AArch64 and Sparc.
> 
>>> --- a/gas/gen-sframe.c
>>> +++ b/gas/gen-sframe.c
>>> @@ -1257,6 +1257,8 @@ sframe_xlate_do_restore (struct 
>>> sframe_xlate_ctx *xlate_ctx,
>>>     return SFRAME_XLATE_OK;
>>>   }
>>> +#if TC_AARCH64
>>> +
>>>   /* Translate DW_CFA_AARCH64_negate_ra_state into SFrame context.
>>>      Return SFRAME_XLATE_OK if success.  */
>>> @@ -1287,30 +1289,7 @@ 
>>> sframe_xlate_do_aarch64_negate_ra_state_with_pc (struct 
>>> sframe_xlate_ctx *xlate_
>>>     return SFRAME_XLATE_ERR_NOTREPRESENTED;  /* Not represented.  */
>>>   }
>>> -/* Translate DW_CFA_GNU_window_save into SFrame context.
>>> -   DW_CFA_GNU_window_save is a DWARF Sparc extension, but is 
>>> multiplexed with a
>>> -   directive of DWARF AArch64 extension: 
>>> DW_CFA_AARCH64_negate_ra_state.
>>> -   The AArch64 backend of GCC 14 and older versions was emitting 
>>> mistakenly the
>>> -   Sparc CFI directive (.cfi_window_save).  From GCC 15, the AArch64 
>>> backend
>>> -   only emits .cfi_negate_ra_state.  For backward compatibility, the 
>>> handler for
>>> -   .cfi_window_save needs to check whether the directive was used in 
>>> a AArch64
>>> -   ABI context or not.
>>> -   Return SFRAME_XLATE_OK if success.  */
>>> -
>>> -static int
>>> -sframe_xlate_do_gnu_window_save (struct sframe_xlate_ctx *xlate_ctx,
>>> -                 struct cfi_insn_data *cfi_insn)
>>> -{
>>> -  unsigned char abi_arch = sframe_get_abi_arch ();
>>> -
>>> -  /* Translate DW_CFA_AARCH64_negate_ra_state into SFrame context.  */
>>> -  if (abi_arch == SFRAME_ABI_AARCH64_ENDIAN_BIG
>>> -      || abi_arch == SFRAME_ABI_AARCH64_ENDIAN_LITTLE)
>>> -    return sframe_xlate_do_aarch64_negate_ra_state (xlate_ctx, 
>>> cfi_insn);
>>> -
>>> -  as_warn (_("skipping SFrame FDE; .cfi_window_save"));
>>> -  return SFRAME_XLATE_ERR_NOTREPRESENTED;  /* Not represented.  */
>>> -}
>>> +#endif /* TC_AARCH64 */
>>>   /* Handle DW_CFA_expression in .cfi_escape.
>>> @@ -1622,14 +1601,14 @@ sframe_do_cfi_insn (struct sframe_xlate_ctx 
>>> *xlate_ctx,
>>>       case DW_CFA_restore:
>>>         err = sframe_xlate_do_restore (xlate_ctx, cfi_insn);
>>>         break;
>>> -    /* DW_CFA_AARCH64_negate_ra_state is multiplexed with
>>> -       DW_CFA_GNU_window_save.  */
>>> -    case DW_CFA_GNU_window_save:
>>> -      err = sframe_xlate_do_gnu_window_save (xlate_ctx, cfi_insn);
>>> +#if TC_AARCH64
>>> +    case DW_CFA_AARCH64_negate_ra_state:
>>> +      err = sframe_xlate_do_aarch64_negate_ra_state (xlate_ctx, 
>>> cfi_insn);
>>>         break;
>>>       case DW_CFA_AARCH64_negate_ra_state_with_pc:
>>>         err = sframe_xlate_do_aarch64_negate_ra_state_with_pc 
>>> (xlate_ctx, cfi_insn);
>>>         break;
>>> +#endif /* TC_AARCH64 */
>>>       case DW_CFA_register:
>>>         err = sframe_xlate_do_register (xlate_ctx, cfi_insn);
>>>         break;
>>
>> This wants looking over by Indu (now Cc-ed).
> 
> FYI I removed DW_CFA_GNU_window_save following the suggestion of Jens 
> Remus:
> https://inbox.sourceware.org/binutils/08aa86e2-f1ba-49cf-a755- 
> ecb7114308e5@linux.ibm.com/

Hi Matthieu,

I think the diffs for gen-sframe.c can be dropped altogether.

The code in gen-sframe.c is already checking for the relevant ABI.  And 
SFrame
generation here feeds off the DWARF cfi_insn created by dw2gencfi* process.
With your changes in gas/dw2gencfi.c, user will anyway see a 'Error: unknown
pseudo-op: ' for say .cfi_window_save on AMD64.

Having said that, IMO, inspite of whether we error out / silently ignore 
these
usages for non-supported arches in dw2gencfi, its best that the code in
gen-sframe.c continues to warn and skip FDE like it was doing earlier.

Thanks
Indu