Re: [RFC PATCH 0/9] s390: Initial support to generate SFrame info from CFI directives in assembler

[Indu Bhagat <indu.bhagat@oracle.com>]

Hi Jens,

On 2/23/24 09:07, Jens Remus wrote:
> This patch series adds initial support to the assembler on s390x to
> generate SFrame stack trace information from CFI directives. It is
> largely based on the respective AArch64 support.
> 
> Please be aware that the SFrame support for s390x provided by patch 9
> of this series still has some open issues, which need to be addressed.
> Any ideas or assistance to overcome the current SFrame limitations
> listed below and in the patch description are very welcome.
> 
> Patches 1-8 are generic cleanups and enhancements to the generation
> of SFrame information in the assembler. Patch 9 adds the initial s390x
> support and is purely RFC.
> 
> 
> Patches 1-3 are minor cleanups/enhancements to the existing SFrame
> support on AArch64 and x86 AMD64.
> 
> Patch 4 enables readelf/objdump to dump the SFrame fixed offsets from
> CFA to the frame pointer (FP) and return address (RA).
> 
> Patch 5 enhances an SFrame warning message to print the human readable
> DWARF call frame instruction name.
> 
> Patch 6 and 7 resolve issues that cause the assembler to either generate
> bad SFrame FDE or to silently skip it. Both issues would be triggered by
> s390-specific SFrame error test cases introduced by patch 9.
> 
> Patch 8 adds verbose assembler warning messages when generation of
> SFrame FDE is skipped.
> 

Thanks for fixing issues and enhancing SFrame handling in Binutils.

> Patch 9 adds initial support to the assembler on s390x to generate
> SFrame stack trace information from CFI directives. Due to differences
> in the s390x ELF ABI [1] compared to the AArch64 and x68 AMD64 ELF ABIs
> and the simplified assumptions of the current SFrame format V2 there
> are several unresolved issues (see also patch description):
> 
> - GCC on s390x may save the frame pointer (FP; r11) and/or return
>    address (RA; r14) registers in other registers (e.g. floating-point
>    registers) instead of saving them on the stack. SFrame currently only
>    supports FP/RA tracking on the stack.
>    This can be reproduced by using a small inline assembler statement,
>    that specifies r11 (FP) and/or r14 (RA) as output operand(s). It can
>    also be observed when compiling a minimal C program using GCC without
>    optimizations.
> 
> - The s390x ELF ABI does not specify a frame pointer register number.
>    Currently GCC and clang both mostly use register r11, but GCC can
>    also be observed to use r14. On s390x a compiler may choose about
>    any register number for that purpose and declare it using CFI
>    directives.
>    This can be observed when compiling glibc, as parts of its hand-
>    written assembler uses register r12 as frame pointer.
> 
> - GCC on s390x may copy the return address (RA) from register r14 to
>    another register and even use that to return to the caller.
>    Effectively this is just a specialization of the first bullet.
> 
> If my understanding of the current SFrame format is not wrong I assume
> the following enhancements to SFrame would be required to fully support
> the s390x architecture:
> 
> - Tracking of the CFA base pointer register number specified in CFI
>    directives .cfi_def_cfa and .cfi_def_cfa_register.
> 
> - Tracking of SP, FP, and RA register contents being saved in other
>    registers instead of on the stack, as specified by CFI directive
>    .cfi_register.
> 
> - Both would effectively require support in SFrame to track all of the
>    17 registers specified as saved in the s390x ELF ABI (r6-r13, r15,
>    and f8-f15), since SP, FP, and RA could be saved in one of those and
>    thus would need to be restored during unwinding.
> 

While I am in the process of taking a closer look at the patchset, and 
the requirements of the s390x ABI to see how to best support with 
SFrame, I would like to add here that tracking all of callee-saved 
registers will significantly bloat up the .sframe section.  (As you 
already know, the offsets to recover the registers from stack are stated 
explicitly in the format).  Hence, adapting the SFrame format to track 
all callee-saved registers is not recommended.

> - Potentially optional, if the use in glibc could be removed:
>    Tracking of SP, FP, and RA register contents being CFA+offset, as
>    specified by .cfi_val_offset.
> 
> [1] ELF ABI s390x Supplement:
>      https://github.com/IBM/s390x-abi/releases
> 
> Changes v1 -> v2:
> - Resolved a regression reported by Linaro-TCWG-CI on AArch64 in one of
>    the generic ld SFrame test cases. The test case contained a
>    .cfi_def_cfa directive, specifying a CFA base register number that is
>    not necessarily a SFrame SP/FP register number on all architectures.
>    This caused the changes from patch 6 to skip SFrame FDE generation on
>    AArch64 (and s390x aswell) with a warning, causing the test case to
>    fail.
> 
> Thanks and regards,
> Jens
> 
> Jens Remus (9):
>    x86: Remove unused SFrame CFI RA register variable
>    aarch64: Align SFrame terminology in comments to specs and x86
>    sframe: Enhance comments for SFRAME_CFA_*_REG macros
>    readelf/objdump: Dump SFrame CFA fixed FP and RA offsets
>    gas: Print DWARF call frame insn name in SFrame warning message
>    gas: Skip SFrame FDE if CFI specifies non-FP/SP base register
>    gas: Warn if SFrame FDE is skipped due to non-default return column
>    gas: User readable warnings if SFrame FDE is not generated
>    s390: Initial support to generate .sframe from CFI directives in
>      assembler
>