[PATCH v1 3/4] aarch64 DWARF: add new CFI directive for PAuth_LR

[Jan Beulich]

On 25.11.2024 18:33, Matthieu Longo wrote:
> On 2024-11-25 16:40, Jan Beulich wrote:
>> On 25.11.2024 17:28, Matthieu Longo wrote:
>>> --- a/include/dwarf2.def
>>> +++ b/include/dwarf2.def
>>> @@ -788,9 +788,11 @@ DW_CFA (DW_CFA_MIPS_advance_loc8, 0x1d)
>>>   /* GNU extensions.
>>>      NOTE: DW_CFA_GNU_window_save is multiplexed on Sparc and AArch64.  */
>>>   DW_CFA (DW_CFA_GNU_window_save, 0x2d)
>>> -DW_CFA_DUP (DW_CFA_AARCH64_negate_ra_state, 0x2d)
>>>   DW_CFA (DW_CFA_GNU_args_size, 0x2e)
>>>   DW_CFA (DW_CFA_GNU_negative_offset_extended, 0x2f)
>>> +/* AArch64 extensions. */
>>> +DW_CFA (DW_CFA_AARCH64_negate_ra_state_with_pc, 0x2c)
>>> +DW_CFA_DUP (DW_CFA_AARCH64_negate_ra_state, 0x2d)
>>
>> While I understand an important aspect here is to stay in sync with the
>> gcc incarnation of this file, I don't think it's a good idea to re-order
>> things. Instead 0x2c should go further up, such that all of these entries
>> remain numerically sorted. Only then will the 2nd line of the comment
>> actually remain directly next to _both_ values it applies to.
>>
>> Jan
> 
> The numeric sorting does not seem to be always respected when it makes 
> sense to group the entries.
> For instance, line 783, a few line above:
> 
>   DW_CFA (DW_CFA_lo_user, 0x1c)
>   DW_CFA (DW_CFA_hi_user, 0x3f)
> 
> I can do as you proposed, I could move the AArch64 extensions between 
> the SGI/MIPS entries and GNU ones.
> Something like:
> 
>    /* SGI/MIPS specific.  */
>    DW_CFA (DW_CFA_MIPS_advance_loc8, 0x1d)
>    /* AArch64 extensions. */
>    DW_CFA (DW_CFA_AARCH64_negate_ra_state_with_pc, 0x2c)
>    DW_CFA_DUP (DW_CFA_AARCH64_negate_ra_state, 0x2d)
>    /* GNU extensions.
>     NOTE: DW_CFA_GNU_window_save is multiplexed on Sparc and AArch64.  */
>    DW_CFA (DW_CFA_GNU_window_save, 0x2d)
>    DW_CFA (DW_CFA_GNU_args_size, 0x2e)
> 
> Do you think it looks better ?

Not necessarily, plus it would get worse when more extensions are added
like this.

   /* SGI/MIPS specific.  */
   DW_CFA (DW_CFA_MIPS_advance_loc8, 0x1d)
   /* AArch64 specific. */
   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)
   DW_CFA_DUP (DW_CFA_AARCH64_negate_ra_state, 0x2d)
   DW_CFA (DW_CFA_GNU_args_size, 0x2e)

IOW personally I consider it particularly relevant that the DW_CFA_DUP()
stay immediately next to (after) its corresponding DW_CFA().

Jan