public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [RFC PATCH v3 0/2] s390: Initial support to generate SFrame info from CFI directives in assembler
@ 2024-04-12 16:36 Jens Remus
  2024-04-12 16:36 ` [RFC PATCH v3 1/2] s390: Initial support to generate .sframe " Jens Remus
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jens Remus @ 2024-04-12 16:36 UTC (permalink / raw)
  To: binutils, Indu Bhagat; +Cc: Jens Remus, Andreas Krebbel

As suggested by Indu I have split my RFC V2 patch series to add initial
support to generate SFrame information on s390x into:

1. A separate series consisting of all the generic preparatory cleanups,
   enhancements, and fixes to the generation of SFrame information
   in the assembler: "[PATCH v3 00/15] sframe: Enhancements to SFrame
   info generation".

2. This RFC series on top with the actual "s390: Initial support
   to generate SFrame info from CFI directives in assembler"

Since the original patch series was at V2 I am sending both as V3.


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 patches
1 and 2 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.

Patch 1 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.
  GCC only does so in leaf-functions. That are functions that do not
  call any other functions. Therefore they are guaranteed to only ever
  be the topmost function on the call stack.
  This is addressed by patch 2, which extends SFrame with limited
  support to represent FP/RA saved in another register on s390x.

- The s390x ELF ABI does not designate a frame pointer register number.
  Currently GCC and clang mostly use register r11.
  This could be addressed by designating r11 as frame-pointer register
  in the s390x ELF ABI in a SFrame compatibility section.

- GCC GCC be observed to use r14 as frame-pointer register in the stack
  clash protector in the function prologue. This guarantees that the
  function is toplevel on the call stack during this non-default frame-
  pointer use.
  This could be addressed by changing GCC to use the "default" frame-
  pointer register r11. Another option would be to extend SFrame
  with limited support to track a non-SP/FP CFA base register number on
  s390x.

- Glibc uses non-default frame-pointer register r12 in two instances.
  This most likely can be addressed by chaning glibc to use the default
  frame-pointer register r11 instead.

- Glibc mcount() / fentry() use non-default return-adress register r0.
  This cannot be changed, but the use of Linux Kernel perf together
  with profiling can be documented as incompatible on s390x.

- 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 and
  is addressed by patch 2.

- GCC may produce code and CFI where the FP is saved on the stack
  while the RA is not. SFrame currently cannot represent this.
  See the respective patch in my separate preparatory patch series.
  This can be addressed by enhancing SFrame to represent this case.
  I will send two alternative options to achieve this as RFC soon.

Patch 2 (new) extends SFrame with limited support to represent FP/RA
saved in another register on s390x. Functions may use this only while
they are the toplevel function on the call stack. That is for instance
in leaf-functions and in the function prologue and epilogue.

[1] ELF ABI s390x Supplement:
    https://github.com/IBM/s390x-abi/releases


Changes v3 -> v2:
- New patch as noted above and in the patch notes.

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 (2):
  s390: Initial support to generate .sframe from CFI directives in
    assembler
  s390: SFrame track FP/RA saved in register

 gas/config/tc-s390.c                          | 55 +++++++++++++++++++
 gas/config/tc-s390.h                          | 31 +++++++++++
 gas/gen-sframe.c                              | 37 ++++++++++++-
 .../gas/cfi-sframe/cfi-sframe-s390-1.d        | 23 ++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-1.s        | 37 +++++++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-2.d        | 23 ++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-2.s        | 37 +++++++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-3.d        | 22 ++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-3.s        | 15 +++++
 .../gas/cfi-sframe/cfi-sframe-s390-4.d        | 21 +++++++
 .../gas/cfi-sframe/cfi-sframe-s390-4.s        |  6 ++
 .../gas/cfi-sframe/cfi-sframe-s390-err-1.d    | 15 +++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-1.s    | 37 +++++++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-2.d    | 15 +++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-2.s    | 37 +++++++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-4.d    | 15 +++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-4.s    |  5 ++
 .../gas/cfi-sframe/cfi-sframe-s390-err-5.d    | 15 +++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-5.s    | 22 ++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-6.d    | 15 +++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-6.s    | 15 +++++
 gas/testsuite/gas/cfi-sframe/cfi-sframe.exp   | 16 +++++-
 include/sframe.h                              |  7 ++-
 libsframe/doc/sframe-spec.texi                |  8 ++-
 libsframe/sframe-dump.c                       | 22 +++++++-
 25 files changed, 542 insertions(+), 9 deletions(-)
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-1.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-1.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-2.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-2.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-3.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-3.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-4.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-4.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-1.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-1.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-2.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-2.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-4.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-4.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-5.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-5.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-6.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-6.s

-- 
2.40.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH v3 1/2] s390: Initial support to generate .sframe from CFI directives in assembler
  2024-04-12 16:36 [RFC PATCH v3 0/2] s390: Initial support to generate SFrame info from CFI directives in assembler Jens Remus
@ 2024-04-12 16:36 ` Jens Remus
  2024-04-12 16:36 ` [RFC PATCH v3 2/2] s390: SFrame track FP/RA saved in register Jens Remus
  2024-04-23  0:26 ` [RFC PATCH v3 0/2] s390: Initial support to generate SFrame info from CFI directives in assembler Indu Bhagat
  2 siblings, 0 replies; 6+ messages in thread
From: Jens Remus @ 2024-04-12 16:36 UTC (permalink / raw)
  To: binutils, Indu Bhagat; +Cc: Jens Remus, Andreas Krebbel

This introduces initial support to generate .sframe from CFI directives
in assembler on s390x. Due to the following SFrame limitations it is
incomplete and does not work for most real-world applications (i.e.
generation of SFrame FDE is skipped):

- SFrame FP/RA tracking assumes the register contents to be saved on
  the stack (i.e. .cfi_offset). It does not support FP/RA register
  contents being saved in other registers (i.e. .cfi_register). GCC
  on s390x can be observed to save the FP/RA register contents in
  floating-point registers, but only in leaf functions.

- SFrame assumes a static FP register number. The s390x ELF ABI [1]
  does not specify any FP register number. GCC and clang on s390x
  usually use register 11 as frame pointer.
  GCC on s390x can also be observed to use register 14 (e.g. binutils
  and glibc builds) in the stack clash protector in the funcion
  prologue.
  glibc on s390x contains hand-written assembler code that uses
  register 12 for the frame pointer.

This s390x support is largely based on the AArch64 support from commit
b52c4ee46657 ("gas: generate .sframe from CFI directives").

SFrame ABI/arch identifier SFRAME_ABI_S390_ENDIAN_BIG is introduced
for s390 and added to the SFrame specification.

According to the s390x ELF ABI [1] the following calling conventions
are used on s390x architecture:
- Register 15 is used as stack pointer (SP). The CFA is initially
  located at offset +160 from the SP.
- Register 14 is used as return address (RA).
- There is no dedicated frame pointer. GCC and LLVM currently use
  register 11 as frame pointer.

The s390x ELF ABI [1] standard stack frame layout has the following
conventions:
- The return address (RA, r15) may be saved at offset -40 from the
  CFA. Unlike x86 AMD64 architecture it is not necessarily saved on
  the stack. Also compilers may use a non-standard stack frame layout,
  such as but not limited to GCC with option -mpacked-stack.
  Therefore SFrame RA tracking is used.
- The potential frame pointer (FP, r11) may be saved at offset -72
  from the CFA. It is not necessarily saved on the stack and compilers
  may use a non-standard stack frame layout (see above).
  Therefore SFrame FP tracking is used.

Support for SFrame is only enabled for z/Architecture (s390x) with
64-bit architecture mode. It is disabled for 32-bit architecture mode
and ESA/390 (s390).

Add s390-specific SFrame test cases. As for the error test cases add
ones that use a non-default frame pointer (FP) register number and ones
that save the return address (RA) in a non-default RA register number.

[1] ELF ABI s390x Supplement:
    https://github.com/IBM/s390x-abi/releases
[2] ELF ABI s390 Supplement:
    https://refspecs.linuxfoundation.org/ELF/zSeries/lzsabi0_s390.html
    https://refspecs.linuxfoundation.org/ELF/zSeries/lzsabi0_s390.pdf

include/
	* sframe.h (SFRAME_ABI_S390_ENDIAN_BIG): Define SFrame ABI/arch
	identifier for s390x. Reference s390x architecture in comments.

libsframe/
	* doc/sframe-spec.texi: Document SFrame ABI/arch identifier for
	s390x and reference s390x architecture.

gas/
	* config/tc-s390.h: s390x support to generate .sframe from CFI
	directives in assembler.
	* config/tc-s390.c: Likewise.
	* gen-sframe.c: Reference s390x architecture in comments.

gas/testsuite/
	* gas/cfi-sframe/cfi-sframe.exp: Enable common SFrame test cases
	for s390x. Add s390-specific SFrame test cases.
	* gas/cfi-sframe/cfi-sframe-s390-1.s: New s390-specific SFrame
	test case.
	* gas/cfi-sframe/cfi-sframe-s390-1.d: Likewise.
	* gas/cfi-sframe/cfi-sframe-s390-2.s: Likewise.
	* gas/cfi-sframe/cfi-sframe-s390-2.d: Likewise.
	* gas/cfi-sframe/cfi-sframe-s390-err-1.s: New s390-specific
	SFrame error test case that uses a non-default register number
	for the frame pointer.
	* gas/cfi-sframe/cfi-sframe-s390-err-1.d: Likewise.
	* gas/cfi-sframe/cfi-sframe-s390-err-2.s: Likewise.
	* gas/cfi-sframe/cfi-sframe-s390-err-2.d: Likewise.
	* gas/cfi-sframe/cfi-sframe-s390-err-3.s: New s390-specific
	SFrame error test case that uses a non-default register number
	to save the return address.
	* gas/cfi-sframe/cfi-sframe-s390-err-3.d: Likewise.
	* gas/cfi-sframe/cfi-sframe-s390-err-4.s: New s390-specific
	SFrame error test case that uses a non-default return column
	(return address) register number.
	* gas/cfi-sframe/cfi-sframe-s390-err-4.d: Likewise.
	* gas/cfi-sframe/cfi-sframe-s390-err-5.s: New s390-specific
	SFrame error test case for FP without RA on stack.
	* gas/cfi-sframe/cfi-sframe-s390-err-5.d: Likewise.

Reviewed-by: Andreas Krebbel <krebbel@linux.ibm.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes v2 -> v3:
    - Update error test cases to changed SFrame warning message texts from
      patch "gas: User readable warnings if SFrame FDE is not generated".
    - New error test case 5 where FP is saved on stack while RA is not.
    - Updated the following notes.
    
    The SFrame support for s390x provided by this patch still has some open
    issues, which need to be addressed. Any ideas or assistance to overcome
    the SFrame limitations listed in the commit description are very
    welcome.
    
    Based on Indus suggestions in the discussion of v2 the following
    approach seems most promising (details follow below):
    - Enhance SFrame to represent FP without RA on stack.
    - Enhance SFrame to represent FP/RA-register saved in FPR-register
      (.cfi_register) on s390x only for compiler-generated leaf functions.
    
    SFrame related warnings from v3 test builds:
    
    Note that the increase in ".cfi_register specifying FP" (v2 -> v3) is
    due to no longer skipping FDE for ".cfi_register specifying SP" in v3.
    Previously some FDEs were skipped for the latter reason, which did not
    surface the former.
    Also note that "FP without RA on stack" is new in v3. I will send RFC
    patches to enable SFrame to represent this case in two flavors
    separately.
    
    Example #1: Warnings when compiling binutils with SFrame
    
    $ ../configure CC="gcc -B$HOME/temp/binutils-sframe/bin -Wa,--gsframe" \
      CXX="g++ -B$HOME/temp/binutils-sframe/bin -Wa,--gsframe" \
      --prefix=$HOME/temp/binutils-sframe
    $ make -j $(nproc) 2>&1 | tee make.log
    $ grep --only-matching "Warning:.*" make.log | sort | uniq -c
          1 Warning: skipping SFrame FDE due to .cfi_def_cfa defining
            non-SP/FP register 14 as CFA base register
        234 Warning: skipping SFrame FDE due to .cfi_register specifying FP
            register
          1 Warning: skipping SFrame FDE due to FP without RA on stack
    
    Example #2: Warnings when compiling glibc with SFrame
    
    $ ../configure CC="gcc -B$HOME/temp/binutils-sframe/bin -Wa,--gsframe" \
      CXX="g++ -B$HOME/temp/binutils-sframe/bin -Wa,--gsframe" \
      --prefix=$HOME/temp/glibc-sframe
    $ make -j $(nproc) 2>&1 | tee make.log
    $ grep --only-matching "Warning:.*" make.log | sort | uniq -c
          7 Warning: skipping SFrame FDE due to .cfi_def_cfa defining
            non-SP/FP register 14 as CFA base register
          2 Warning: skipping SFrame FDE due to .cfi_def_cfa_register
            defining non-SP/FP register 12 as CFA base register
        310 Warning: skipping SFrame FDE due to .cfi_register specifying FP
            register
          2 Warning: skipping SFrame FDE due to DWARF CFI op CFI_escape
            (0x103)
        180 Warning: skipping SFrame FDE due to FP without RA on stack
          2 Warning: skipping SFrame FDE due to non-default return-address
            register 0
    
    .cfi_def_cfa 14, ... originates from GCC generated prologue code, which
    uses register %r14 as frame-pointer (FP) register in the stack clash
    protector. GCC could potentially be changed to use register %r11 as FP
    register in this case.
    
    .cfi_def_cfa_register 12 originates from hand-written assembler code
    sysdeps/s390/s390-64/dl-trampoline.S, which uses register %r12 as frame
    pointer (FP).
    
    .cfi_return_column 0 and .cfi_escape both originate from hand-written
    assembler code sysdeps/s390/s390-64/s390x-mcount.S, which is compiled
    twice (with and without -DSHARED).
    mcount/fentry may only use registers that, according to the s390x ABI,
    are volatile (i.e. not saved or reserved) and do not have any role
    (e.g. argument, stack pointer, return address). This boils down to
    registers %r0 and %r1. Register %r0 is used as "return register" to
    branch to the function actually called. Register %r1 is used as temp.
    
    - .cfi_escape 0x14, 0x15, 0x14: Is used to code
      ".cfi_val_offset r15, -160", which would require binutils 2.28+ (glibc
      currently supports a minimum binutils 2.25). This CFI directive states
      that the contents of register %r15 are CFA-160 (not to be confused
      with saved at CFA-160).
      This specific warning in the build of glibc on s390x can be ignored,
      as ".cfi_val_offset <SP-register>, ..." can be ignored. The reason is
      that SFrame does not track the SP (on stack, in a register, as a CFA
      offset). Instead the SP value at entry can be computed from the CFA
      tracking information (see description of patch "gas: Don't skip SFrame
      FDE if .cfi_register specifies SP register") for details.
    
    - .cfi_return_column 0: See v2 comments below for details.
    
    .cfi_register 11, <FPR-reg> originates from GCC generated prologue code,
    which may save the frame-pointer (FP) register %r11 in a floating-point
    register (FPR), but only in leaf functions.
    SFrame could be enhanced to represent .cfi_register <FP|RA-reg> for
    compiler-generated leaf functions on s390x. I have a hack, which uses
    the LSB (lowest significant bit) of the FP/RA offset to indicate that
    the offset value is not an offset, but a DWARF register number shifted
    to the left by one. The LSB can be used as indication, as CFA offsets
    must be a multiple of the data-alignment factor -8 on s390x. In DWARF
    CFI any CFA offsets are actually encoded as factored offsets using this
    architecture-dependent data-alignment factor. This is only an option,
    unless SFrame would take the opportunity to do the same and store any
    CFA offsets as factored offsets and save the architecture-dependent
    data-alignment factor in the SFrame Header.
    
    "FP without RA on stack" currently cannot be represented by SFrame. I
    reviewed several of the reported instances. All of them were for GCC
    generated code for variadic functions (taking a variable number of
    arguments). GCC saves all unused argument registers, which are
    potentially part of the variable argument list (starting somewhere
    between %r2 and %r6), as well as any registers required to be saved on
    the stack. For that purpose it uses a single STM (Store Multiple)
    instruction, whose range then includes includes the FP register %r11
    due to the need to save RA register %r14, if a function call can occur
    required. For return it then only restores the RA register %r14 and SP
    register %r15, since it did not modify the FP register %r11.
    Note that another source for "FP without RA on stack" is GCC option
    "-mpreserve-args". In a test build of glibc their number increases
    from 180 to 548.
    
    SFrame related warnings from v2 test builds for reference:
    
    Example #1: Warnings when compiling binutils with SFrame
    
    $ ../configure CC="gcc -B$HOME/temp/binutils-gcc/bin -Wa,--gsframe" \
      CXX="g++ -B$HOME/temp/binutils-gcc/bin -Wa,--gsframe" \
      --prefix=$HOME/temp/binutils-sframe
    $ make -j $(nprocs) 2>&1 | tee make.log
    $ grep --only-matching "Warning:.*" make.log | sort | uniq -c
          1 Warning: skipping SFrame FDE due to .cfi_def_cfa specifying CFA
            base register other than SP or FP (14 instead of 15 or 11)
        205 Warning: skipping SFrame FDE due to .cfi_register specifying FP
            register (11)
         56 Warning: skipping SFrame FDE due to .cfi_register specifying SP
            register (15)
    
    Example #2: Warnings when compiling glibc with SFrame
    
    $ ../configure CC="gcc -B$HOME/temp/binutils-sframe/bin -Wa,--gsframe" \
      CXX="g++ -B$HOME/temp/binutils-sframe/bin -Wa,--gsframe" \
      --prefix=$HOME/temp/glibc-sframe
    $ make -j $(nprocs) 2>&1 | tee make.log
    $ grep --only-matching "Warning:.*" make.log | sort | uniq -c
          2 Warning: skipping SFrame FDE due to .cfi_def_cfa_register
            specifying CFA base register other than SP or FP (12 instead of
            15 or 11)
          7 Warning: skipping SFrame FDE due to .cfi_def_cfa specifying CFA
            base register other than SP or FP (14 instead of 15 or 11)
        225 Warning: skipping SFrame FDE due to .cfi_register specifying FP
            register (11)
        187 Warning: skipping SFrame FDE due to .cfi_register specifying SP
            register (15)
          2 Warning: skipping SFrame FDE due to DWARF CFI op CFI_escape
            (0x103)
          2 Warning: skipping SFrame FDE due to non-default DWARF return
            column (0 instead of 14)
    
    .cfi_def_cfa 14, ... originates from GCC generated code, which uses
    register %r14 as frame pointer (FP).
    
    .cfi_def_cfa_register 12 originates from hand-written assembler code
    sysdeps/s390/s390-64/dl-trampoline.S, which uses register %r12 as frame
    pointer (FP).
    
    .cfi_return_column 0 and .cfi_escape both originate from hand-written
    assembler code sysdeps/s390/s390-64/s390x-mcount.S, which is compiled
    twice (with and without -DSHARED).
    
    - .cfi_escape 0x14, 0x15, 0x14: Is used to code
      ".cfi_val_offset r15, -160", which would require binutils 2.28+ (glibc
      currently supports a minimum binutils 2.25). This CFI directive states
      that the contents of register %r15 are CFA-160 (not to be confused
      with saved at CFA-160).
    
    - .cfi_return_column 0: The following comment explains why register %r0
      contains the return address:
    
      "The _mcount implementation now has to call __mcount_internal with the
      address of .LP0 as first parameter and the return address as second
      parameter. &.LP0 was loaded to %r1 and the return address is in %r14.
      _mcount may not modify any register.
    
      Alternatively, at the start of each function __fentry__ is called
      using a single
    
              brasl  0,__fentry__
    
      instruction.  In this case %r0 points to the callee, and %r14 points
      to the caller.  These values need to be passed to __mcount_internal
      using the same sequence as for _mcount, so the code below is shared
      between both functions.
      The only major difference is that __fentry__ cannot return through
      %r0, in which the return address is located, because br instruction
      is a no-op with this register.  Therefore %r1, which is clobbered by
      the PLT anyway, is used."
    
      Excerpt of the relevant glibc source code:
    
              .globl C_SYMBOL_NAME(MCOUNT_SYMBOL)
              .type C_SYMBOL_NAME(MCOUNT_SYMBOL), @function
              cfi_startproc
              .align ALIGNARG(4)
      C_LABEL(MCOUNT_SYMBOL)
              cfi_return_column (glue(r, MCOUNT_CALLEE_REG))
              /* Save the caller-clobbered registers.  */
              aghi  %r15,-224
              cfi_adjust_cfa_offset (224)
              /* binutils 2.28+: .cfi_val_offset r15, -160 */
              .cfi_escape \
                      /* DW_CFA_val_offset */ 0x14, \
                      /* r15 */               0x0f, \
                      /* scaled offset */     0x14
              stmg  %r14,%r5,160(%r15)
    
              cfi_offset (r14, -224)
              cfi_offset (r0, -224+16)
      ...

 gas/config/tc-s390.c                          | 55 +++++++++++++++++++
 gas/config/tc-s390.h                          | 31 +++++++++++
 gas/gen-sframe.c                              |  2 +-
 .../gas/cfi-sframe/cfi-sframe-s390-1.d        | 23 ++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-1.s        | 37 +++++++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-2.d        | 23 ++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-2.s        | 37 +++++++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-1.d    | 15 +++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-1.s    | 37 +++++++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-2.d    | 15 +++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-2.s    | 37 +++++++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-3.d    | 15 +++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-3.s    |  6 ++
 .../gas/cfi-sframe/cfi-sframe-s390-err-4.d    | 15 +++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-4.s    |  5 ++
 .../gas/cfi-sframe/cfi-sframe-s390-err-5.d    | 15 +++++
 .../gas/cfi-sframe/cfi-sframe-s390-err-5.s    | 22 ++++++++
 gas/testsuite/gas/cfi-sframe/cfi-sframe.exp   | 14 ++++-
 include/sframe.h                              |  7 ++-
 libsframe/doc/sframe-spec.texi                |  8 ++-
 20 files changed, 412 insertions(+), 7 deletions(-)
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-1.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-1.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-2.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-2.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-1.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-1.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-2.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-2.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-4.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-4.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-5.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-5.s

diff --git a/gas/config/tc-s390.c b/gas/config/tc-s390.c
index 659c6af392bd..1f8b67adbdea 100644
--- a/gas/config/tc-s390.c
+++ b/gas/config/tc-s390.c
@@ -24,6 +24,8 @@
 #include "subsegs.h"
 #include "dwarf2dbg.h"
 #include "dw2gencfi.h"
+#include "sframe.h"
+#include "gen-sframe.h"
 
 #include "opcode/s390.h"
 #include "elf/s390.h"
@@ -97,6 +99,17 @@ const char FLT_CHARS[] = "dD";
 /* The dwarf2 data alignment, adjusted for 32 or 64 bit.  */
 int s390_cie_data_alignment;
 
+/* Register numbers used for SFrame stack trace info.  */
+
+/* Stack-pointer DWARF register number according to s390x ELF ABI.  */
+unsigned int s390_sframe_cfa_sp_reg = 15;
+
+/* Frame-pointer DWARF register number accoring to s390x GCC/LLVM convention.  */
+unsigned int s390_sframe_cfa_fp_reg = 11;
+
+/* Return-address DWARF register number according to s390x ELF ABI.  */
+unsigned int s390_sframe_cfa_ra_reg = DWARF2_DEFAULT_RETURN_COLUMN;
+
 /* The target specific pseudo-ops which we support.  */
 
 /* Define the prototypes for the pseudo-ops */
@@ -2860,6 +2873,48 @@ tc_s390_regname_to_dw2regnum (char *regname)
   return regnum;
 }
 
+/* Whether SFrame stack trace info is supported.  */
+
+bool
+s390_support_sframe_p (void)
+{
+  /* At this time, SFrame is supported for s390x (64-bit) only.  */
+  return (s390_arch_size == 64);
+}
+
+/* Specify if RA tracking is needed.  */
+
+bool
+s390_sframe_ra_tracking_p (void)
+{
+  return true;
+}
+
+/* Specify the fixed offset to recover RA from CFA.
+   (useful only when RA tracking is not needed).  */
+
+offsetT
+s390_sframe_cfa_ra_offset (void)
+{
+  return (offsetT) SFRAME_CFA_FIXED_RA_INVALID;
+}
+
+/* Get the abi/arch indentifier for SFrame.  */
+
+unsigned char
+s390_sframe_get_abi_arch (void)
+{
+  unsigned char sframe_abi_arch = 0;
+
+  if (s390_support_sframe_p ())
+    {
+      gas_assert (target_big_endian);
+      sframe_abi_arch = SFRAME_ABI_S390_ENDIAN_BIG;
+    }
+
+  return sframe_abi_arch;
+}
+
 void
 s390_elf_final_processing (void)
 {
diff --git a/gas/config/tc-s390.h b/gas/config/tc-s390.h
index cd353045a822..c8f172d8737b 100644
--- a/gas/config/tc-s390.h
+++ b/gas/config/tc-s390.h
@@ -98,3 +98,34 @@ extern int s390_cie_data_alignment;
 extern void s390_elf_final_processing (void);
 
 #define elf_tc_final_processing s390_elf_final_processing
+
+/* SFrame  */
+
+/* Whether SFrame stack trace info is supported.  */
+extern bool s390_support_sframe_p (void);
+#define support_sframe_p s390_support_sframe_p
+
+/* The stack-pointer register number for SFrame stack trace info.  */
+extern unsigned int s390_sframe_cfa_sp_reg;
+#define SFRAME_CFA_SP_REG s390_sframe_cfa_sp_reg
+
+/* The frame-pointer register number for SFrame stack trace info.  */
+extern unsigned int s390_sframe_cfa_fp_reg;
+#define SFRAME_CFA_FP_REG s390_sframe_cfa_fp_reg
+
+/* The return address register number for SFrame stack trace info.  */
+extern unsigned int s390_sframe_cfa_ra_reg;
+#define SFRAME_CFA_RA_REG s390_sframe_cfa_ra_reg
+
+/* Specify if RA tracking is needed.  */
+extern bool s390_sframe_ra_tracking_p (void);
+#define sframe_ra_tracking_p s390_sframe_ra_tracking_p
+
+/* Specify the fixed offset to recover RA from CFA.
+   (useful only when RA tracking is not needed).  */
+extern offsetT s390_sframe_cfa_ra_offset (void);
+#define sframe_cfa_ra_offset s390_sframe_cfa_ra_offset
+
+/* The abi/arch indentifier for SFrame.  */
+unsigned char s390_sframe_get_abi_arch (void);
+#define sframe_get_abi_arch s390_sframe_get_abi_arch
diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
index 7e815f9603ef..93a5eebcc9d8 100644
--- a/gas/gen-sframe.c
+++ b/gas/gen-sframe.c
@@ -675,7 +675,7 @@ output_sframe_internal (void)
 #endif
   out_one (fixed_ra_offset);
 
-  /* None of the AMD64, or AARCH64 ABIs need the auxiliary header.
+  /* None of the AMD64, AARCH64, or S390 ABIs need the auxiliary header.
      When the need does arise to use this field, the appropriate backend
      must provide this information.  */
   out_one (0); /* Auxiliary SFrame header length.  */
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-1.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-1.d
new file mode 100644
index 000000000000..211804a2309a
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-1.d
@@ -0,0 +1,23 @@
+#objdump: --sframe=.sframe
+#name: SFrame generation on s390
+#...
+Contents of the SFrame section .sframe:
+
+  Header :
+
+    Version: SFRAME_VERSION_2
+    Flags: NONE
+    Num FDEs: 1
+    Num FREs: 6
+
+  Function Index :
+
+    func idx \[0\]: pc = 0x0, size = 40 bytes
+    STARTPC +CFA +FP +RA +
+    0+0000 +sp\+160 +u +u +
+    0+0006 +sp\+160 +c\-72 +c\-48 +
+    0+000c +sp\+336 +c\-72 +c\-48 +
+    0+0010 +fp\+336 +c\-72 +c\-48 +
+    0+001c +sp\+160 +u +u +
+    0+001e +fp\+336 +c\-72 +c\-48 +
+#pass
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-1.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-1.s
new file mode 100644
index 000000000000..56d8425ae2b1
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-1.s
@@ -0,0 +1,37 @@
+	.cfi_sections .sframe
+	.cfi_startproc
+	stmg	%r6,%r15,48(%r15)
+	.cfi_offset 6, -112
+	.cfi_offset 7, -104
+	.cfi_offset 8, -96
+	.cfi_offset 9, -88
+	.cfi_offset 10, -80
+	.cfi_offset 11, -72
+	.cfi_offset 12, -64
+	.cfi_offset 13, -56
+	.cfi_offset 14, -48
+	.cfi_offset 15, -40
+	lay	%r15,-176(%r15)
+	.cfi_def_cfa_offset 336
+	lgr	%r11,%r15
+	.cfi_def_cfa_register 11
+	lay	%r15,-128(%r15)
+.Lreturn:
+	lmg	%r6,%r15,224(%r11)
+	.cfi_remember_state
+	.cfi_restore 15
+	.cfi_restore 14
+	.cfi_restore 13
+	.cfi_restore 12
+	.cfi_restore 11
+	.cfi_restore 10
+	.cfi_restore 9
+	.cfi_restore 8
+	.cfi_restore 7
+	.cfi_restore 6
+	.cfi_def_cfa 15, 160
+	br	%r14
+	.cfi_restore_state
+	lay     %r15,-128(%r15)
+	j	.Lreturn
+	.cfi_endproc
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-2.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-2.d
new file mode 100644
index 000000000000..211804a2309a
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-2.d
@@ -0,0 +1,23 @@
+#objdump: --sframe=.sframe
+#name: SFrame generation on s390
+#...
+Contents of the SFrame section .sframe:
+
+  Header :
+
+    Version: SFRAME_VERSION_2
+    Flags: NONE
+    Num FDEs: 1
+    Num FREs: 6
+
+  Function Index :
+
+    func idx \[0\]: pc = 0x0, size = 40 bytes
+    STARTPC +CFA +FP +RA +
+    0+0000 +sp\+160 +u +u +
+    0+0006 +sp\+160 +c\-72 +c\-48 +
+    0+000c +sp\+336 +c\-72 +c\-48 +
+    0+0010 +fp\+336 +c\-72 +c\-48 +
+    0+001c +sp\+160 +u +u +
+    0+001e +fp\+336 +c\-72 +c\-48 +
+#pass
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-2.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-2.s
new file mode 100644
index 000000000000..4d58cdaf64a7
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-2.s
@@ -0,0 +1,37 @@
+	.cfi_sections .sframe
+	.cfi_startproc
+	stmg	%r6,%r15,48(%r15)
+	.cfi_rel_offset 6, 48
+	.cfi_rel_offset 7, 56
+	.cfi_rel_offset 8, 64
+	.cfi_rel_offset 9, 72
+	.cfi_rel_offset 10, 80
+	.cfi_rel_offset 11, 88
+	.cfi_rel_offset 12, 96
+	.cfi_rel_offset 13, 104
+	.cfi_rel_offset 14, 112
+	.cfi_rel_offset 15, 120
+	lay	%r15,-176(%r15)
+	.cfi_def_cfa_offset 336
+	lgr	%r11,%r15
+	.cfi_def_cfa_register 11
+	lay	%r15,-128(%r15)
+.Lreturn:
+	lmg	%r6,%r15,224(%r11)
+	.cfi_remember_state
+	.cfi_restore 15
+	.cfi_restore 14
+	.cfi_restore 13
+	.cfi_restore 12
+	.cfi_restore 11
+	.cfi_restore 10
+	.cfi_restore 9
+	.cfi_restore 8
+	.cfi_restore 7
+	.cfi_restore 6
+	.cfi_def_cfa 15, 160
+	br	%r14
+	.cfi_restore_state
+	lay     %r15,-128(%r15)
+	j	.Lreturn
+	.cfi_endproc
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-1.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-1.d
new file mode 100644
index 000000000000..5875e568bd66
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-1.d
@@ -0,0 +1,15 @@
+#name: SFrame generation on s390
+#as: --gsframe
+#warning: skipping SFrame FDE due to .cfi_def_cfa_register defining non-SP/FP register 10 as CFA base register
+#objdump: --sframe=.sframe
+#...
+Contents of the SFrame section .sframe:
+
+  Header :
+
+    Version: SFRAME_VERSION_2
+    Flags: NONE
+    Num FDEs: 0
+    Num FREs: 0
+
+#pass
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-1.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-1.s
new file mode 100644
index 000000000000..85e36c39d2a3
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-1.s
@@ -0,0 +1,37 @@
+	.cfi_sections .sframe
+	.cfi_startproc
+	stmg	%r6,%r15,48(%r15)
+	.cfi_offset 6, -112
+	.cfi_offset 7, -104
+	.cfi_offset 8, -96
+	.cfi_offset 9, -88
+	.cfi_offset 10, -80
+	.cfi_offset 11, -72
+	.cfi_offset 12, -64
+	.cfi_offset 13, -56
+	.cfi_offset 14, -48
+	.cfi_offset 15, -40
+	lay	%r15,-176(%r15)
+	.cfi_def_cfa_offset 336
+	lgr	%r10,%r15
+	.cfi_def_cfa_register 10	# non-default frame pointer register
+	lay	%r15,-128(%r15)
+.Lreturn:
+	lmg	%r6,%r15,224(%r10)
+	.cfi_remember_state
+	.cfi_restore 15
+	.cfi_restore 14
+	.cfi_restore 13
+	.cfi_restore 12
+	.cfi_restore 11
+	.cfi_restore 10
+	.cfi_restore 9
+	.cfi_restore 8
+	.cfi_restore 7
+	.cfi_restore 6
+	.cfi_def_cfa 15, 160
+	br	%r14
+	.cfi_restore_state
+	lay     %r15,-128(%r15)
+	j	.Lreturn
+	.cfi_endproc
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-2.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-2.d
new file mode 100644
index 000000000000..e1da5c19f21e
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-2.d
@@ -0,0 +1,15 @@
+#name: SFrame generation on s390
+#as: --gsframe
+#warning: skipping SFrame FDE due to .cfi_def_cfa defining non-SP/FP register 10 as CFA base register
+#objdump: --sframe=.sframe
+#...
+Contents of the SFrame section .sframe:
+
+  Header :
+
+    Version: SFRAME_VERSION_2
+    Flags: NONE
+    Num FDEs: 0
+    Num FREs: 0
+
+#pass
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-2.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-2.s
new file mode 100644
index 000000000000..0bb274a9e991
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-2.s
@@ -0,0 +1,37 @@
+	.cfi_sections .sframe
+	.cfi_startproc
+	stmg	%r6,%r15,48(%r15)
+	.cfi_offset 6, -112
+	.cfi_offset 7, -104
+	.cfi_offset 8, -96
+	.cfi_offset 9, -88
+	.cfi_offset 10, -80
+	.cfi_offset 11, -72
+	.cfi_offset 12, -64
+	.cfi_offset 13, -56
+	.cfi_offset 14, -48
+	.cfi_offset 15, -40
+	lay	%r15,-176(%r15)
+	.cfi_def_cfa_offset 336
+	lgr	%r10,%r15
+	.cfi_def_cfa 10, 336	# non-default frame pointer register
+	lay	%r15,-128(%r15)
+.Lreturn:
+	lmg	%r6,%r15,224(%r10)
+	.cfi_remember_state
+	.cfi_restore 15
+	.cfi_restore 14
+	.cfi_restore 13
+	.cfi_restore 12
+	.cfi_restore 11
+	.cfi_restore 10
+	.cfi_restore 9
+	.cfi_restore 8
+	.cfi_restore 7
+	.cfi_restore 6
+	.cfi_def_cfa 15, 160
+	br	%r14
+	.cfi_restore_state
+	lay     %r15,-128(%r15)
+	j	.Lreturn
+	.cfi_endproc
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.d
new file mode 100644
index 000000000000..4b3262f02f02
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.d
@@ -0,0 +1,15 @@
+#name: SFrame generation on s390
+#as: --gsframe
+#warning: skipping SFrame FDE due to .cfi_register specifying RA register
+#objdump: --sframe=.sframe
+#...
+Contents of the SFrame section .sframe:
+
+  Header :
+
+    Version: SFRAME_VERSION_2
+    Flags: NONE
+    Num FDEs: 0
+    Num FREs: 0
+
+#pass
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.s
new file mode 100644
index 000000000000..f0a5bd577304
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.s
@@ -0,0 +1,6 @@
+	.cfi_sections .sframe
+	.cfi_startproc
+	lgr	%r7,%r14
+	.cfi_register 14, 7	# non-default return address register
+	br	%r7
+	.cfi_endproc
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-4.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-4.d
new file mode 100644
index 000000000000..cdfdd4a8e6ab
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-4.d
@@ -0,0 +1,15 @@
+#name: SFrame generation on s390
+#as: --gsframe
+#warning: skipping SFrame FDE due to non-default return-address register 7
+#objdump: --sframe=.sframe
+#...
+Contents of the SFrame section .sframe:
+
+  Header :
+
+    Version: SFRAME_VERSION_2
+    Flags: NONE
+    Num FDEs: 0
+    Num FREs: 0
+
+#pass
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-4.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-4.s
new file mode 100644
index 000000000000..cfdea75e0b95
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-4.s
@@ -0,0 +1,5 @@
+	.cfi_sections .sframe
+	.cfi_startproc
+	.cfi_return_column 7	# non-default return address register
+	br	%r7
+	.cfi_endproc
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-5.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-5.d
new file mode 100644
index 000000000000..57af27d26419
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-5.d
@@ -0,0 +1,15 @@
+#name: SFrame generation on s390
+#as: --gsframe
+#warning: skipping SFrame FDE due to FP without RA on stack
+#objdump: --sframe=.sframe
+#...
+Contents of the SFrame section .sframe:
+
+  Header :
+
+    Version: SFRAME_VERSION_2
+    Flags: NONE
+    Num FDEs: 0
+    Num FREs: 0
+
+#pass
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-5.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-5.s
new file mode 100644
index 000000000000..c0c3911496b2
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-5.s
@@ -0,0 +1,22 @@
+	.cfi_sections .sframe
+	.cfi_startproc
+	stmg	%r4,%r15,32(%r15)
+	.cfi_offset 6, -112
+	.cfi_offset 7, -104
+	.cfi_offset 8, -96
+	.cfi_offset 9, -88
+	.cfi_offset 10, -80
+	.cfi_offset 11, -72
+	.cfi_offset 12, -64
+	.cfi_offset 13, -56
+	.cfi_offset 14, -48
+	.cfi_offset 15, -40
+	lay	%r15,-192(%r15)
+	.cfi_def_cfa_offset 352
+.Lreturn:
+	lmg	%r14,%r15,304(%r15)
+	.cfi_restore 15
+	.cfi_restore 14
+	.cfi_def_cfa_offset 160
+	br	%r14
+	.cfi_endproc
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp b/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp
index cfae28d39aaf..f97fa9e170a2 100644
--- a/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp
@@ -64,7 +64,8 @@ proc gas_x86_64_check { } {
 }
 
 # common tests
-if  { ([istarget "x86_64-*-*"] || [istarget "aarch64*-*-*"]) \
+if  { ([istarget "x86_64-*-*"] || [istarget "aarch64*-*-*"] ||
+       [istarget "s390x-*-*"]) \
        && [gas_sframe_check] } then {
 
     global ASFLAGS
@@ -99,3 +100,14 @@ if { [istarget "aarch64*-*-*"] && [gas_sframe_check] } then {
     run_dump_test "cfi-sframe-aarch64-2"
     run_dump_test "cfi-sframe-aarch64-pac-ab-key-1"
 }
+
+# s390 specific tests
+if { [istarget "s390x*-*-*"] && [gas_sframe_check] } then {
+    run_dump_test "cfi-sframe-s390-1"
+    run_dump_test "cfi-sframe-s390-2"
+    run_dump_test "cfi-sframe-s390-err-1"
+    run_dump_test "cfi-sframe-s390-err-2"
+    run_dump_test "cfi-sframe-s390-err-3"
+    run_dump_test "cfi-sframe-s390-err-4"
+    run_dump_test "cfi-sframe-s390-err-5"
+}
diff --git a/include/sframe.h b/include/sframe.h
index b3d0c2e937d3..90bc92a32f84 100644
--- a/include/sframe.h
+++ b/include/sframe.h
@@ -93,6 +93,7 @@ extern "C"
 #define SFRAME_ABI_AARCH64_ENDIAN_BIG      1 /* AARCH64 big endian.  */
 #define SFRAME_ABI_AARCH64_ENDIAN_LITTLE   2 /* AARCH64 little endian.  */
 #define SFRAME_ABI_AMD64_ENDIAN_LITTLE     3 /* AMD64 little endian.  */
+#define SFRAME_ABI_S390_ENDIAN_BIG         4 /* S390 big endian.  */
 
 /* SFrame FRE types.  */
 #define SFRAME_FRE_TYPE_ADDR1	0
@@ -190,7 +191,7 @@ typedef struct sframe_func_desc_entry
      - 2-bits: Unused.
      ------------------------------------------------------------------------
      |     Unused    |  PAC auth A/B key (aarch64) |  FDE type |   FRE type   |
-     |               |        Unused (amd64)       |           |              |
+     |               |     Unused (amd64, s390)    |           |              |
      ------------------------------------------------------------------------
      8               6                             5           4              0     */
   uint8_t sfde_func_info;
@@ -248,7 +249,7 @@ typedef struct sframe_fre_info
      - 1 bit: Mangled RA state bit (aarch64 only).
      ----------------------------------------------------------------------------------
      | Mangled-RA (aarch64) |  Size of offsets   |   Number of offsets    |   base_reg |
-     |  Unused (amd64)      |                    |                        |            |
+     | Unused (amd64, s390) |                    |                        |            |
      ----------------------------------------------------------------------------------
      8                     7                    5                        1            0
 
@@ -274,7 +275,7 @@ typedef struct sframe_fre_info
 
 /* SFrame Frame Row Entry definitions.
 
-   Used for both AMD64 and AARCH64.
+   Used for AMD64, AARCH64, and S390.
 
    An SFrame Frame Row Entry is a self-sufficient record which contains
    information on how to generate the stack trace for the specified range of
diff --git a/libsframe/doc/sframe-spec.texi b/libsframe/doc/sframe-spec.texi
index d0f99bd698eb..a81faf49f5c1 100644
--- a/libsframe/doc/sframe-spec.texi
+++ b/libsframe/doc/sframe-spec.texi
@@ -74,8 +74,8 @@ The SFrame stack trace information is provided in a loaded section, known as the
 @code{.sframe} section.  When available, the @code{.sframe} section appears in
 a new segment of its own, PT_GNU_SFRAME.
 
-The SFrame format is currently supported only for select ABIs, namely, AMD64
-and AAPCS64.
+The SFrame format is currently supported only for select ABIs, namely, AMD64,
+AAPCS64, and s390.
 
 A portion of the SFrame format follows an unaligned on-disk representation.
 Some data structures, however, (namely the SFrame header and the SFrame
@@ -366,6 +366,10 @@ in the format.
 @item @code{SFRAME_ABI_AMD64_ENDIAN_LITTLE}
 @tab 3 @tab AMD64 little-endian
 
+@tindex SFRAME_ABI_S390_ENDIAN_BIG
+@item @code{SFRAME_ABI_S390_ENDIAN_BIG}
+@tab 4 @tab s390 big-endian
+
 @end multitable
 
 The presence of an explicit identification of ABI/arch in SFrame may allow
-- 
2.40.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH v3 2/2] s390: SFrame track FP/RA saved in register
  2024-04-12 16:36 [RFC PATCH v3 0/2] s390: Initial support to generate SFrame info from CFI directives in assembler Jens Remus
  2024-04-12 16:36 ` [RFC PATCH v3 1/2] s390: Initial support to generate .sframe " Jens Remus
@ 2024-04-12 16:36 ` Jens Remus
  2024-04-16 16:07   ` Jens Remus
  2024-04-23  0:26 ` [RFC PATCH v3 0/2] s390: Initial support to generate SFrame info from CFI directives in assembler Indu Bhagat
  2 siblings, 1 reply; 6+ messages in thread
From: Jens Remus @ 2024-04-12 16:36 UTC (permalink / raw)
  To: binutils, Indu Bhagat; +Cc: Jens Remus, Andreas Krebbel

GCC on s390x, when in a leaf function, can be observed to save the
frame pointer (FP) and/or return address (RA) register in a floating-
point registers (FPR) instead of on the stack. This is declared using
the following CFI directive:

  .cfi_register <fp/ra-regno>, <fpr-regno>

SFrame cannot represent the FP and/or RA being saved in another
register. It does only track the CFA base register (SP/FP), CFA offset
from CFA base register, and FP and RA save area offsets from CFA.

On s390x the FP and/or RA are only saved in another FPR when in a leaf
function. That is a function that does not call any other functions.
Therefore it can ever only be the topmost function in a call chain.
During function return, if the RA would be restored into a non-default
RA register, the function would also only ever be the topmost function
on the call stack.
An unwinder by default has access to all registers of the function that
if the topmost on the call stack. Therefore no further information would
be required for those registers.

Represent the FP/RA in another register on s390, by encoding the
register number shifted by one to the left with the least-significant
bit set in the offset as follows:

  offset = (regno << 1) | 1

Add s390-specific SFrame (error) test cases for FP/RA being saved in
FPRs in leaf-function.

gas/
	* gen-sframe.c (s390_sframe_xlate_do_register): New s390-
	specific function to represent FP/RA in another register on
	s390.
	(sframe_xlate_do_register): Invoke s390_sframe_xlate_do_register
	on s390.

libsframe/
	* sframe-dump.c (is_sframe_abi_arch_s390): New helper to test
	whether ABI/arch is s390.
	(dump_sframe_func_with_fres): Dump FP/RA in another register on
	s390.

gas/testsuite/
	* gas/cfi-sframe/cfi-sframe.exp: Update s390-specific SFrame
	(error) test cases.
	* gas/cfi-sframe/cfi-sframe-s390-3.s: New s390-specific SFrame
	test case for FP/RA saved in register on s390.
	* gas/cfi-sframe/cfi-sframe-s390-3.d: Likewise.
	* gas/cfi-sframe/cfi-sframe-s390-err-6.s: New s390-specific
	SFrame error test case for FP without RA saved in register.
	* gas/cfi-sframe/cfi-sframe-s390-err-6.d: Likewise.
        * gas/cfi-sframe/cfi-sframe-s390-err-3.s: s390-specific SFrame
	error test case 3 no longer triggers an error and is renamed as
	test case 4.
        * gas/cfi-sframe/cfi-sframe-s390-err-3.d: Likewise.
        * gas/cfi-sframe/cfi-sframe-s390-4.s: Likewise.
        * gas/cfi-sframe/cfi-sframe-s390-4.d: Likewise. Added expected
        SFrame dump output.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>

* gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-4.d: # GCL: missing file
* gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.d -> gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-6.d: # GCL: missing file
* gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.s -> gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-4.s: # GCL: missing file
---

Notes (jremus):
    Changes v2 -> v3:
    - New patch.
    
    Currently there is no mean to determine whether a function is a leaf
    function. Neither an annotation that a section of the assembler code
    does not call any function. Both would guarantee that the code will
    only ever appear topmost on the call stack, so that the unwinder by
    default has access to all registers.
    Therefore SFrame generation cannot determine whether saving of FP/RA
    registers in other registers such as FPR is actually valid. It needs
    to trust the CFI, as it would need to to anyway.
    
    The s390-specific functionality could be made dependent on TC_S390
    at compile-time instead of on sframe_get_abi_arch() at run-time.
    
    sframe_fre_set_{bp|ra}_track() could be renamed to
    sframe_fre_set_{fp|ra}_track_offset() and new
    sframe_fre_set_{fp|ra}_track_register() could be introduced. These
    would call some architecture-specific callback to perform the encoding
    of the dw2renum as offset.

 gas/gen-sframe.c                              | 35 +++++++++++++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-3.d        | 22 ++++++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-3.s        | 15 ++++++++
 .../gas/cfi-sframe/cfi-sframe-s390-4.d        | 21 +++++++++++
 ...frame-s390-err-3.s => cfi-sframe-s390-4.s} |  0
 ...e-s390-err-3.d => cfi-sframe-s390-err-6.d} |  2 +-
 .../gas/cfi-sframe/cfi-sframe-s390-err-6.s    | 15 ++++++++
 gas/testsuite/gas/cfi-sframe/cfi-sframe.exp   |  4 ++-
 libsframe/sframe-dump.c                       | 22 ++++++++++--
 9 files changed, 132 insertions(+), 4 deletions(-)
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-3.d
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-3.s
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-4.d
 rename gas/testsuite/gas/cfi-sframe/{cfi-sframe-s390-err-3.s => cfi-sframe-s390-4.s} (100%)
 rename gas/testsuite/gas/cfi-sframe/{cfi-sframe-s390-err-3.d => cfi-sframe-s390-err-6.d} (74%)
 create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-6.s

diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
index 93a5eebcc9d8..4cc86eb6c815 100644
--- a/gas/gen-sframe.c
+++ b/gas/gen-sframe.c
@@ -1150,6 +1150,37 @@ sframe_xlate_do_val_offset (struct sframe_xlate_ctx *xlate_ctx ATTRIBUTE_UNUSED,
   return SFRAME_XLATE_OK;
 }
 
+/* S390-specific translate DW_CFA_register into SFrame context.
+   Return SFRAME_XLATE_OK if success.  */
+
+static int
+s390_sframe_xlate_do_register (struct sframe_xlate_ctx *xlate_ctx,
+			       struct cfi_insn_data *cfi_insn)
+{
+  /* The scratchpad FRE currently being updated with each cfi_insn
+     being interpreted.  This FRE eventually gets linked in into the
+     list of FREs for the specific function.  */
+  struct sframe_row_entry *cur_fre = xlate_ctx->cur_fre;
+
+  gas_assert (cur_fre);
+
+  /* Change the rule for the register indicated by the register number to
+     be the specified register.  Encode the register number as offset by
+     shifting it to the left by one and setting the least-significant bit
+     (LSB).  The LSB can be used to differentiate offsets from register
+     numbers, as offsets from CFA are always a multiple of -8 on s390x.  */
+  if (cfi_insn->u.rr.reg1 == SFRAME_CFA_FP_REG)
+    sframe_fre_set_bp_track (cur_fre, cfi_insn->u.rr.reg2 << 1 | 1);
+#ifdef SFRAME_FRE_RA_TRACKING
+  else if (sframe_ra_tracking_p ()
+	   && cfi_insn->u.rr.reg1 == SFRAME_CFA_RA_REG)
+    sframe_fre_set_ra_track (cur_fre, cfi_insn->u.rr.reg2 << 1 | 1);
+#endif
+
+  /* Safe to skip.  */
+  return SFRAME_XLATE_OK;
+}
+
 /* Translate DW_CFA_register into SFrame context.
    Return SFRAME_XLATE_OK if success.  */
 
@@ -1157,6 +1188,10 @@ static int
 sframe_xlate_do_register (struct sframe_xlate_ctx *xlate_ctx ATTRIBUTE_UNUSED,
 			  struct cfi_insn_data *cfi_insn)
 {
+  /* Conditionally invoke S390-specific implementation.  */
+  if (sframe_get_abi_arch () == SFRAME_ABI_S390_ENDIAN_BIG)
+    return s390_sframe_xlate_do_register (xlate_ctx, cfi_insn);
+
   /* Previous value of register1 is register2.  However, if the specified
      register1 is not interesting (FP or RA reg), the current DW_CFA_register
      instruction can be safely skipped without sacrificing the asynchronicity of
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-3.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-3.d
new file mode 100644
index 000000000000..722874113225
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-3.d
@@ -0,0 +1,22 @@
+#objdump: --sframe=.sframe
+#name: SFrame generation on s390
+#...
+Contents of the SFrame section .sframe:
+
+  Header :
+
+    Version: SFRAME_VERSION_2
+    Flags: NONE
+    Num FDEs: 1
+    Num FREs: 5
+
+  Function Index :
+
+    func idx \[0\]: pc = 0x0, size = 26 bytes
+    STARTPC +CFA +FP +RA +
+    0+0000 +sp\+160 +u +u +
+    0+0004 +sp\+160 +u +r16 +
+    0+0008 +sp\+160 +r17 +r16 +
+    0+0014 +sp\+160 +u +r16 +
+    0+0018 +sp\+160 +u +u +
+#pass
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-3.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-3.s
new file mode 100644
index 000000000000..1d4497178ada
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-3.s
@@ -0,0 +1,15 @@
+	.cfi_sections .sframe
+	.cfi_startproc
+	ldgr	%f0,%r14
+	.cfi_register 14, 16
+	ldgr	%f2,%r11
+	.cfi_register 11, 17
+	la	%r11,0
+	la	%r14,0
+.Lreturn:
+	lgdr	%r11,%f2
+	.cfi_restore 11
+	lgdr	%r14,%f0
+	.cfi_restore 14
+	br	%r14
+	.cfi_endproc
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-4.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-4.d
new file mode 100644
index 000000000000..3c013ac2b6fe
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-4.d
@@ -0,0 +1,21 @@
+#name: SFrame generation on s390
+#as: --gsframe
+#objdump: --sframe=.sframe
+#...
+Contents of the SFrame section .sframe:
+
+  Header :
+
+    Version: SFRAME_VERSION_2
+    Flags: NONE
+    Num FDEs: 1
+    Num FREs: 2
+
+  Function Index :
+
+    func idx \[0\]: pc = 0x0, size = 6 bytes
+    STARTPC +CFA +FP +RA +
+    0+0000 +sp\+160 +u +u +
+    0+0004 +sp\+160 +u +r7 +
+
+#pass:
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-4.s
similarity index 100%
rename from gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.s
rename to gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-4.s
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-6.d
similarity index 74%
rename from gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.d
rename to gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-6.d
index 4b3262f02f02..57af27d26419 100644
--- a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.d
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-6.d
@@ -1,6 +1,6 @@
 #name: SFrame generation on s390
 #as: --gsframe
-#warning: skipping SFrame FDE due to .cfi_register specifying RA register
+#warning: skipping SFrame FDE due to FP without RA on stack
 #objdump: --sframe=.sframe
 #...
 Contents of the SFrame section .sframe:
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-6.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-6.s
new file mode 100644
index 000000000000..48b01ac5f57b
--- /dev/null
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-6.s
@@ -0,0 +1,15 @@
+	.cfi_sections .sframe
+	.cfi_startproc
+	ldgr	%f2,%r11
+	.cfi_register 11, 17
+	ldgr	%f0,%r14
+	.cfi_register 14, 16
+	la	%r11,0
+	la	%r14,0
+.Lreturn:
+	lgdr	%r14,%f0
+	.cfi_restore 14
+	lgdr	%r11,%f2
+	.cfi_restore 11
+	br	%r14
+	.cfi_endproc
diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp b/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp
index f97fa9e170a2..8b97b02222b3 100644
--- a/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp
+++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp
@@ -105,9 +105,11 @@ if { [istarget "aarch64*-*-*"] && [gas_sframe_check] } then {
 if { [istarget "s390x*-*-*"] && [gas_sframe_check] } then {
     run_dump_test "cfi-sframe-s390-1"
     run_dump_test "cfi-sframe-s390-2"
+    run_dump_test "cfi-sframe-s390-3"
+    run_dump_test "cfi-sframe-s390-4"
     run_dump_test "cfi-sframe-s390-err-1"
     run_dump_test "cfi-sframe-s390-err-2"
-    run_dump_test "cfi-sframe-s390-err-3"
     run_dump_test "cfi-sframe-s390-err-4"
     run_dump_test "cfi-sframe-s390-err-5"
+    run_dump_test "cfi-sframe-s390-err-6"
 }
diff --git a/libsframe/sframe-dump.c b/libsframe/sframe-dump.c
index 69633d53a33a..40ea531314ba 100644
--- a/libsframe/sframe-dump.c
+++ b/libsframe/sframe-dump.c
@@ -40,6 +40,14 @@ is_sframe_abi_arch_aarch64 (sframe_decoder_ctx *sfd_ctx)
   return aarch64_p;
 }
 
+/* Return TRUE if the SFrame section is associated with the s390 ABIs.  */
+
+static bool
+is_sframe_abi_arch_s390 (sframe_decoder_ctx *sfd_ctx)
+{
+  return sframe_decoder_get_abi_arch (sfd_ctx) == SFRAME_ABI_S390_ENDIAN_BIG;
+}
+
 static void
 dump_sframe_header (sframe_decoder_ctx *sfd_ctx)
 {
@@ -175,7 +183,12 @@ dump_sframe_func_with_fres (sframe_decoder_ctx *sfd_ctx,
 
       /* Dump SP/FP info.  */
       if (err[1] == 0)
-	sprintf (temp, "c%+d", fp_offset);
+	{
+	  if (is_sframe_abi_arch_s390 (sfd_ctx) && (fp_offset & 1))
+	    sprintf (temp, "r%d", fp_offset >> 1);
+	  else
+	    sprintf (temp, "c%+d", fp_offset);
+	}
       else
 	strcpy (temp, "u");
       printf ("%-10s", temp);
@@ -187,7 +200,12 @@ dump_sframe_func_with_fres (sframe_decoder_ctx *sfd_ctx,
 	  != SFRAME_CFA_FIXED_RA_INVALID)
 	strcpy (temp, "f");
       else if (err[2] == 0)
-	sprintf (temp, "c%+d", ra_offset);
+	{
+	  if (is_sframe_abi_arch_s390 (sfd_ctx) && (ra_offset & 1))
+	    sprintf (temp, "r%d", ra_offset >> 1);
+	  else
+	    sprintf (temp, "c%+d", ra_offset);
+	}
       else
 	strcpy (temp, "u");
 
-- 
2.40.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH v3 2/2] s390: SFrame track FP/RA saved in register
  2024-04-12 16:36 ` [RFC PATCH v3 2/2] s390: SFrame track FP/RA saved in register Jens Remus
@ 2024-04-16 16:07   ` Jens Remus
  0 siblings, 0 replies; 6+ messages in thread
From: Jens Remus @ 2024-04-16 16:07 UTC (permalink / raw)
  To: binutils, Indu Bhagat; +Cc: Andreas Krebbel

Am 12.04.2024 um 18:36 schrieb Jens Remus:
> GCC on s390x, when in a leaf function, can be observed to save the
> frame pointer (FP) and/or return address (RA) register in a floating-
> point registers (FPR) instead of on the stack. This is declared using
> the following CFI directive:
> 
>    .cfi_register <fp/ra-regno>, <fpr-regno>
> 
> SFrame cannot represent the FP and/or RA being saved in another
> register. It does only track the CFA base register (SP/FP), CFA offset
> from CFA base register, and FP and RA save area offsets from CFA.
> 
> On s390x the FP and/or RA are only saved in another FPR when in a leaf
> function. That is a function that does not call any other functions.
> Therefore it can ever only be the topmost function in a call chain.
> During function return, if the RA would be restored into a non-default
> RA register, the function would also only ever be the topmost function
> on the call stack.
> An unwinder by default has access to all registers of the function that
> if the topmost on the call stack. Therefore no further information would
> be required for those registers.
> 
> Represent the FP/RA in another register on s390, by encoding the
> register number shifted by one to the left with the least-significant
> bit set in the offset as follows:
> 
>    offset = (regno << 1) | 1
> 
> Add s390-specific SFrame (error) test cases for FP/RA being saved in
> FPRs in leaf-function.
> 
> gas/
> 	* gen-sframe.c (s390_sframe_xlate_do_register): New s390-
> 	specific function to represent FP/RA in another register on
> 	s390.
> 	(sframe_xlate_do_register): Invoke s390_sframe_xlate_do_register
> 	on s390.
> 
> libsframe/
> 	* sframe-dump.c (is_sframe_abi_arch_s390): New helper to test
> 	whether ABI/arch is s390.
> 	(dump_sframe_func_with_fres): Dump FP/RA in another register on
> 	s390.
> 
> gas/testsuite/
> 	* gas/cfi-sframe/cfi-sframe.exp: Update s390-specific SFrame
> 	(error) test cases.
> 	* gas/cfi-sframe/cfi-sframe-s390-3.s: New s390-specific SFrame
> 	test case for FP/RA saved in register on s390.
> 	* gas/cfi-sframe/cfi-sframe-s390-3.d: Likewise.
> 	* gas/cfi-sframe/cfi-sframe-s390-err-6.s: New s390-specific
> 	SFrame error test case for FP without RA saved in register.
> 	* gas/cfi-sframe/cfi-sframe-s390-err-6.d: Likewise.
>          * gas/cfi-sframe/cfi-sframe-s390-err-3.s: s390-specific SFrame
> 	error test case 3 no longer triggers an error and is renamed as
> 	test case 4.
>          * gas/cfi-sframe/cfi-sframe-s390-err-3.d: Likewise.
>          * gas/cfi-sframe/cfi-sframe-s390-4.s: Likewise.
>          * gas/cfi-sframe/cfi-sframe-s390-4.d: Likewise. Added expected
>          SFrame dump output.

I will fix the indentation in the next version.

> 
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> 
> * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-4.d: # GCL: missing file
> * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.d -> gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-6.d: # GCL: missing file
> * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-err-3.s -> gas/testsuite/gas/cfi-sframe/cfi-sframe-s390-4.s: # GCL: missing file

Above three lines have erroneously creeped in, while experimenting with 
custom (prepare-)commit-msg Git hooks to generate/validate the GNU 
Changelog entries. (sigh!) I will remove them in the next version.

> ---

...

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303) and z/VSE Support
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des 
Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der 
Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH v3 0/2] s390: Initial support to generate SFrame info from CFI directives in assembler
  2024-04-12 16:36 [RFC PATCH v3 0/2] s390: Initial support to generate SFrame info from CFI directives in assembler Jens Remus
  2024-04-12 16:36 ` [RFC PATCH v3 1/2] s390: Initial support to generate .sframe " Jens Remus
  2024-04-12 16:36 ` [RFC PATCH v3 2/2] s390: SFrame track FP/RA saved in register Jens Remus
@ 2024-04-23  0:26 ` Indu Bhagat
  2024-04-25  7:54   ` Indu Bhagat
  2 siblings, 1 reply; 6+ messages in thread
From: Indu Bhagat @ 2024-04-23  0:26 UTC (permalink / raw)
  To: Jens Remus, binutils; +Cc: Andreas Krebbel

On 4/12/24 09:36, Jens Remus wrote:
> As suggested by Indu I have split my RFC V2 patch series to add initial
> support to generate SFrame information on s390x into:
> 
> 1. A separate series consisting of all the generic preparatory cleanups,
>     enhancements, and fixes to the generation of SFrame information
>     in the assembler: "[PATCH v3 00/15] sframe: Enhancements to SFrame
>     info generation".
> 
> 2. This RFC series on top with the actual "s390: Initial support
>     to generate SFrame info from CFI directives in assembler"
> 
> Since the original patch series was at V2 I am sending both as V3.
> 
> 
> 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 patches
> 1 and 2 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.
> 
> Patch 1 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.
>    GCC only does so in leaf-functions. That are functions that do not
>    call any other functions. Therefore they are guaranteed to only ever
>    be the topmost function on the call stack.
>    This is addressed by patch 2, which extends SFrame with limited
>    support to represent FP/RA saved in another register on s390x.
> 
> - The s390x ELF ABI does not designate a frame pointer register number.
>    Currently GCC and clang mostly use register r11.
>    This could be addressed by designating r11 as frame-pointer register
>    in the s390x ELF ABI in a SFrame compatibility section.
> 
> - GCC GCC be observed to use r14 as frame-pointer register in the stack
>    clash protector in the function prologue. This guarantees that the
>    function is toplevel on the call stack during this non-default frame-
>    pointer use.
>    This could be addressed by changing GCC to use the "default" frame-
>    pointer register r11. Another option would be to extend SFrame
>    with limited support to track a non-SP/FP CFA base register number on
>    s390x.
> 
> - Glibc uses non-default frame-pointer register r12 in two instances.
>    This most likely can be addressed by chaning glibc to use the default
>    frame-pointer register r11 instead.
> 
> - Glibc mcount() / fentry() use non-default return-adress register r0.
>    This cannot be changed, but the use of Linux Kernel perf together
>    with profiling can be documented as incompatible on s390x.
> 
> - 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 and
>    is addressed by patch 2.
> 
> - GCC may produce code and CFI where the FP is saved on the stack
>    while the RA is not. SFrame currently cannot represent this.
>    See the respective patch in my separate preparatory patch series.
>    This can be addressed by enhancing SFrame to represent this case.
>    I will send two alternative options to achieve this as RFC soon.
> 

Hi Jens,

For this issue I have taken a look at your two proposals:
   - #R1 [RFC PATCH 0/1] sframe: Represent FP without RA on stack (padding)
   - #R2 [RFC PATCH 0/1] sframe: Represent FP without RA on stack (bitmap)

(Thanks a lot for working out those options in detail and providing the 
patches, and stats where applicable).

I think #R1 is good in the way it keeps the changes isolated to s390 
support but it is a bit wasteful to force an offset.  The latter 
indicates limitation in SFrame representation (pointing to the need to 
improve and not suffer by sacrificing space). As for #R2, IMO it makes 
the format less future-proof: it will only allow upto 4 offsets.

Now an option indeed is to:

1. Use your #R2 approach but keep it confined to s390.

We can use additional bits in the SFrame auxiliary header to indicate 
the fact that for s390, the 4-bits in sframe_fre_info are "flags for 
offsets" and not "Number of offsets". Hence keeping this limitation 
confined to the s390x incarnation of the format.

Adding additional information in the the auxiliary header area: Using 
sfh_auxhdr_len to define a 32-bit area (32-bit size so that FDEs 
accesses do not cause unaligned memory accesses; not sure myself yet if 
unaligned accesses are OK on s390 though).

Although I dont like the additional complexity of "adding info in the 
the aux hdr to allow a different interpretation of an existing field 
differently"; but I don't have a strong opinion against this.

2. There is an unused bit in sframe_fre_info.  We could consider using 
it (for s390x) to tag those FREs where RA has not been saved on stack, 
but FP is.

I dont think this information can be encoded in SFrame FDE.  There can 
be a function that does not save r14 in prologue, but does so later, 
right ? If not, we also have the following:

There are unused bits in SFrame FDE (see sfde_func_info) that can be 
used to indicate that an FDE does not save RA on stack, hence the second 
offset, if present, will be FP.  Then we can use it in conjunction with 
"RA-tracking-enabled".

I was thinking of suggesting to use the bit [(((sfde_func_info) >> 5) & 
0x1)] (i.e. the one currently used for Pauth key on aarch64).

We could instead also consider using one of the upper two bits 
(sfde_func_info), which are currently unused, if this feature will be 
useful for other arches.


> Patch 2 (new) extends SFrame with limited support to represent FP/RA
> saved in another register on s390x. Functions may use this only while
> they are the toplevel function on the call stack. That is for instance
> in leaf-functions and in the function prologue and epilogue.
> 
> [1] ELF ABI s390x Supplement:
>      https://github.com/IBM/s390x-abi/releases
> 
> 
> Changes v3 -> v2:
> - New patch as noted above and in the patch notes.
> 
> 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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH v3 0/2] s390: Initial support to generate SFrame info from CFI directives in assembler
  2024-04-23  0:26 ` [RFC PATCH v3 0/2] s390: Initial support to generate SFrame info from CFI directives in assembler Indu Bhagat
@ 2024-04-25  7:54   ` Indu Bhagat
  0 siblings, 0 replies; 6+ messages in thread
From: Indu Bhagat @ 2024-04-25  7:54 UTC (permalink / raw)
  To: Jens Remus, binutils; +Cc: Andreas Krebbel

On 4/22/24 17:26, Indu Bhagat wrote:
> On 4/12/24 09:36, Jens Remus wrote:
>> As suggested by Indu I have split my RFC V2 patch series to add initial
>> support to generate SFrame information on s390x into:
>>
>> 1. A separate series consisting of all the generic preparatory cleanups,
>>     enhancements, and fixes to the generation of SFrame information
>>     in the assembler: "[PATCH v3 00/15] sframe: Enhancements to SFrame
>>     info generation".
>>
>> 2. This RFC series on top with the actual "s390: Initial support
>>     to generate SFrame info from CFI directives in assembler"
>>
>> Since the original patch series was at V2 I am sending both as V3.
>>
>>
>> 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 patches
>> 1 and 2 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.
>>
>> Patch 1 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.
>>    GCC only does so in leaf-functions. That are functions that do not
>>    call any other functions. Therefore they are guaranteed to only ever
>>    be the topmost function on the call stack.
>>    This is addressed by patch 2, which extends SFrame with limited
>>    support to represent FP/RA saved in another register on s390x.
>>
>> - The s390x ELF ABI does not designate a frame pointer register number.
>>    Currently GCC and clang mostly use register r11.
>>    This could be addressed by designating r11 as frame-pointer register
>>    in the s390x ELF ABI in a SFrame compatibility section.
>>
>> - GCC GCC be observed to use r14 as frame-pointer register in the stack
>>    clash protector in the function prologue. This guarantees that the
>>    function is toplevel on the call stack during this non-default frame-
>>    pointer use.
>>    This could be addressed by changing GCC to use the "default" frame-
>>    pointer register r11. Another option would be to extend SFrame
>>    with limited support to track a non-SP/FP CFA base register number on
>>    s390x.
>>
>> - Glibc uses non-default frame-pointer register r12 in two instances.
>>    This most likely can be addressed by chaning glibc to use the default
>>    frame-pointer register r11 instead.
>>
>> - Glibc mcount() / fentry() use non-default return-adress register r0.
>>    This cannot be changed, but the use of Linux Kernel perf together
>>    with profiling can be documented as incompatible on s390x.
>>
>> - 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 and
>>    is addressed by patch 2.
>>
>> - GCC may produce code and CFI where the FP is saved on the stack
>>    while the RA is not. SFrame currently cannot represent this.
>>    See the respective patch in my separate preparatory patch series.
>>    This can be addressed by enhancing SFrame to represent this case.
>>    I will send two alternative options to achieve this as RFC soon.
>>
> 
> Hi Jens,
> 
> For this issue I have taken a look at your two proposals:
>    - #R1 [RFC PATCH 0/1] sframe: Represent FP without RA on stack (padding)
>    - #R2 [RFC PATCH 0/1] sframe: Represent FP without RA on stack (bitmap)
> 
> (Thanks a lot for working out those options in detail and providing the 
> patches, and stats where applicable).
> 
> I think #R1 is good in the way it keeps the changes isolated to s390 
> support but it is a bit wasteful to force an offset.  The latter 
> indicates limitation in SFrame representation (pointing to the need to 
> improve and not suffer by sacrificing space). As for #R2, IMO it makes 
> the format less future-proof: it will only allow upto 4 offsets.
> 
> Now an option indeed is to:
> 
> 1. Use your #R2 approach but keep it confined to s390.
> 
> We can use additional bits in the SFrame auxiliary header to indicate 
> the fact that for s390, the 4-bits in sframe_fre_info are "flags for 
> offsets" and not "Number of offsets". Hence keeping this limitation 
> confined to the s390x incarnation of the format.
> 
> Adding additional information in the the auxiliary header area: Using 
> sfh_auxhdr_len to define a 32-bit area (32-bit size so that FDEs 
> accesses do not cause unaligned memory accesses; not sure myself yet if 
> unaligned accesses are OK on s390 though).
> 
> Although I dont like the additional complexity of "adding info in the 
> the aux hdr to allow a different interpretation of an existing field 
> differently"; but I don't have a strong opinion against this.
> 
> 2. There is an unused bit in sframe_fre_info.  We could consider using 
> it (for s390x) to tag those FREs where RA has not been saved on stack, 
> but FP is.
> 
> I dont think this information can be encoded in SFrame FDE.  There can 
> be a function that does not save r14 in prologue, but does so later, 
> right ? If not, we also have the following:
> 
> There are unused bits in SFrame FDE (see sfde_func_info) that can be 
> used to indicate that an FDE does not save RA on stack, hence the second 
> offset, if present, will be FP.  Then we can use it in conjunction with 
> "RA-tracking-enabled".
> 
> I was thinking of suggesting to use the bit [(((sfde_func_info) >> 5) & 
> 0x1)] (i.e. the one currently used for Pauth key on aarch64).
> 
> We could instead also consider using one of the upper two bits 
> (sfde_func_info), which are currently unused, if this feature will be 
> useful for other arches.
> 

And ...

3. Similar to the padding solution, using SFrame FRE offset to store 
some metadata is also an option.  A rough sketch provided in the other 
thread.

(I am already of two minds about the padding solution: I initially found 
it wasteful, but overall it does fit well with the SFrame philosophy in 
that we dont lose the flexibility.)

> 
>> Patch 2 (new) extends SFrame with limited support to represent FP/RA
>> saved in another register on s390x. Functions may use this only while
>> they are the toplevel function on the call stack. That is for instance
>> in leaf-functions and in the function prologue and epilogue.
>>
>> [1] ELF ABI s390x Supplement:
>>      https://github.com/IBM/s390x-abi/releases
>>
>>
>> Changes v3 -> v2:
>> - New patch as noted above and in the patch notes.
>>
>> 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
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-04-25  7:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-12 16:36 [RFC PATCH v3 0/2] s390: Initial support to generate SFrame info from CFI directives in assembler Jens Remus
2024-04-12 16:36 ` [RFC PATCH v3 1/2] s390: Initial support to generate .sframe " Jens Remus
2024-04-12 16:36 ` [RFC PATCH v3 2/2] s390: SFrame track FP/RA saved in register Jens Remus
2024-04-16 16:07   ` Jens Remus
2024-04-23  0:26 ` [RFC PATCH v3 0/2] s390: Initial support to generate SFrame info from CFI directives in assembler Indu Bhagat
2024-04-25  7:54   ` Indu Bhagat

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).