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

[Matthieu Longo]

On 2025-04-28 07:24, Jan Beulich wrote:
> On 25.04.2025 17:53, Matthieu Longo wrote:
>> On 2025-04-25 15:45, Jan Beulich wrote:
>>> On 25.04.2025 16:02, 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.
>>>
>>> Arm64 uses .cfi_negate_ra_state. .cfi_window_save was wrongly exposed
>>> everywhere except for Sparc. Hence as I view it either all other
>>> architectures need to retain it for backwards compatibility, or none
>>> of them. Yet perhaps I'm missing some peculiarity here.
>>
>> None of others architectures has used cfi_window_save, except AArch64.
>> It does not mean that some people out there were not mistakenly adding
>> the directive into their assembly files, but it was not doing anything
>> anyway as it was ignored by the tools.
>>
>> You can check this inside GCC, and see that it is only used for
>> unwinding inside Sparc and AArch64 backend.
> 
> Hmm, beside this being a gross hack (.cfi_negate_ra_state ought to be
> used there), commentary there even alludes to the possibility of using
> these elsewhere.

Please, could you be more explicit in what this is a gross hack ?
What is this commentary you are referring to ?

> Plus you leave aside the possibility of someone having
> found a(n) (ab)use for this for another target, perhaps in plain
> assembly.
> 

If someone decided to abuse it for another target, it is neither in LLVM 
nor GCC, nor Cranelift (that I am aware of). I think we should not keep 
all the doors of possibilities open here, and be more pragmatical.

The situation right now is not ideal, we try to make things better by 
being stricter to avoid future abuse, and make the code more 
understandable for everyone.

It might be worth accepting the risk that we will break some obscure 
code out there that is abusing something for which no guarantee was 
provided in the first place. Preserving backward compatibility for those 
use cases is more detrimental than beneficial.

On one hand, the backward compatibility we offer by preserving 
.cfi_window_save on AArch64 seems reasonable, and on the other hand, I 
am still struggling to see an acceptable reason for not disabling those 
directives for all the targets except Sparc and AArch64.

>> In the same way in GDB, only the AArch64 and Sparc vendor handler are
>> processing DW_CFA_AARCH64_negate_ra_state and DW_CFA_GNU_window_save.
>> aarch64-tdep.c:1410:  if (op == DW_CFA_AARCH64_negate_ra_state)
>> sparc-tdep.c:1600:  if (op != DW_CFA_GNU_window_save)
>>
>> In my understanding, there is no benefit in keeping the backward
>> compatibility with something that was never used. At worst, the build of
>> those assembly files would fail, and someone would have to remove the
>> directive that was never taken into account anyway.
> 
> What do you mean here? gcc isn't the only producer, and gdb isn't the
> only consumer. Or at least I know of no way to prove this.
> 

Outside of GCC, LLVM, GDB, Cranelift, I am not aware of any use of those 
DWARF instructions. It certainly does not mean that there is not, but 
again, are we pragmatic here ?

>> It seems to me an
>> acceptable risk of making the support of the directive only for Sparc
>> and AArch64.
> 
> This may be true. It's largely a policy question, and in such regards
> it may well be that I'm overly conservative.
> 
> Jan

Who should we involve to get an answer on policy questions ?

Matthieu.