[PATCH] gas: sframe: skip DW_CFA_GNU_args_size when safe to ignore

[Sam James]

Jan Beulich <jbeulich@suse.com> writes:

> On 08.09.2025 20:14, Sam James wrote:
>> Indu Bhagat via Binutils <binutils@sourceware.org> writes:
>> 
>>> Currently, gas warns and skips generating SFrame FDE when it sees:
>>>      .cfi_escape 0x2e,XX
>>>
>>> From the documentation of DW_CFA_GNU_args_size:
>>>  "The DW_CFA_GNU_args_size instruction takes an unsigned LEB128 operand
>>>   representing an argument size. This instruction specifies the total of
>>>   the size of the arguments which have been pushed onto the stack."
>>>
>>> With origins seemingly for VAX architecture, the usage of
>>> DW_CFA_GNU_args_size seems to have evolved.  The purpose of
>>> DW_CFA_GNU_args_size is to adjust SP when performing virtual stack
>>> unwinding for exception handling.
>>>
>>> For the purpose of stack tracing using SFrame, DW_CFA_GNU_args_size is
>>> safe to skip, especially when the CFA restoration is known to be FP
>>> based.
>> 
>> I would personally include a link to that ML post/RFC from you, just for
>> reference in case anyone digs in future..
>> 
>> Anyway, yes please, but I can't approve it. This is one of two "known
>> warnings" that came up in the discussion with us and also Jan.
>
> Btw, with this ...
>
>>> gas/
>>>         * gen-sframe.c (sframe_xlate_do_escape_gnu_args_size): New
>>> 	definition.
>>>         (sframe_xlate_do_cfi_escape): Handle DW_CFA_GNU_args_size.
>>> gas/testsuite/
>>>         * gas/cfi-sframe/cfi-sframe.exp: New test.
>>>         * gas/cfi-sframe/cfi-sframe-common-12.d: New test.
>>>         * gas/cfi-sframe/cfi-sframe-common-12.s: New test.
>>>         * gas/cfi-sframe/cfi-sframe-x86_64-3.d: New test.
>>>         * gas/cfi-sframe/cfi-sframe-x86_64-3.s: New test.
>>> ---
>>>  gas/gen-sframe.c                              | 62 ++++++++++++++++++-
>>>  .../gas/cfi-sframe/cfi-sframe-common-12.d     | 22 +++++++
>>>  .../gas/cfi-sframe/cfi-sframe-common-12.s     |  8 +++
>>>  .../gas/cfi-sframe/cfi-sframe-x86_64-3.d      | 21 +++++++
>>>  .../gas/cfi-sframe/cfi-sframe-x86_64-3.s      |  5 ++
>>>  gas/testsuite/gas/cfi-sframe/cfi-sframe.exp   |  2 +
>>>  6 files changed, 119 insertions(+), 1 deletion(-)
>>>  create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-common-12.d
>>>  create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-common-12.s
>>>  create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-3.d
>>>  create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-3.s
>
> ... diffstat, aiui Indu doesn't require any approval, as she's maintainer of
> SFrame code.

ACK, though I also wanted to let you know I was thinking about the
problem still and not ignoring your review of my earlier patches.

>
> Jan

sam