[6/8] s390: Add SFrame stack trace information for .plt section

  Enable SFrame stack tracing through PLT entries.  Based on x86-64.

On s390x both PLT0 and PLTn entries are 32-bytes in size.  Their code
neither alters the stack pointer (SP), frame pointer (FP), nor return
address (RA) registers.  Therefore the PLT0 can be represented using
a SFrame FDE of type PCINC with a SFrame "null" FRE and the PLTn can
be represented using a SFrame FDE of type PCMASK, with a repetition
block size of 32 (PLTn size), and a SFrame "null" FRE.

Note that as both the PLT0 entry and the PLTn entries have equal size
and can both be represented using the identical SFrame "null" FRE, the
whole .plt section on s390x could be represented using a single SFrame
FDE of type PCMASK, with a repetition block size of 32 (PLT0 and PLTn
size), and a single SFrame "null" FRE.  Keep the x86-64 logic with
separate SFrame FDEs for PLT0 and PLTn, to ease potential generalization
of the .sframe for .plt generation logic among architectures.

bfd/
	* elf64-s390.c: Include sframe.h and sframe-api.h.
	(PLT_SFRAME_FDE_START_OFFSET, SFRAME_PLT0_MAX_NUM_FRES,
	SFRAME_PLTN_MAX_NUM_FRES, elf_s390x_sframe_plt_fre,
	elf_s390x_sframe_plt): New .sframe template for .plt section.
	(elf_s390_link_hash_table): Add plt_cfe_ctx, plt_sframe, and
	sframe_plt fields.
	(_bfd_s390x_elf_create_sframe_plt): New function.  Fill in
	.sframe section for .plt section.
	(_bfd_s390x_elf_write_sframe_plt): New function.  Write .sframe
	section.
	(elf_s390_create_dynamic_sections): Create .sframe section for
	.plt section.
	(elf_s390_late_size_sections): Call
	_bfd_s390x_elf_create_sframe_plt and
	_bfd_s390x_elf_write_sframe_plt.
	(elf_s390_finish_dynamic_sections): Write .plt section start
	into .sframe FDE covering .plt section.  Call
	_bfd_elf_merge_section_sframe on htab->plt_sframe.

ld/
	* NEWS: Add news entry.

ld/testsuite/
	* ld-s390/s390.exp: Add new test.
	* ld-s390/sframe-plt-1.d: New linker-generated .sframe for .plt
	test.
	* ld-s390/sframe-simple-1.d: Adjust expected test output due to
	linker-generated .sframe for .plt.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
 bfd/elf64-s390.c                       | 259 +++++++++++++++++++++++++
 ld/NEWS                                |   4 +
 ld/testsuite/ld-s390/s390.exp          |   1 +
 ld/testsuite/ld-s390/sframe-plt-1.d    |  28 +++
 ld/testsuite/ld-s390/sframe-simple-1.d |   9 +-
 5 files changed, 297 insertions(+), 4 deletions(-)
 create mode 100644 ld/testsuite/ld-s390/sframe-plt-1.d
  

On 4/2/25 9:12 AM, Jens Remus wrote:
> Enable SFrame stack tracing through PLT entries.  Based on x86-64.
> 
> On s390x both PLT0 and PLTn entries are 32-bytes in size.  Their code
> neither alters the stack pointer (SP), frame pointer (FP), nor return
> address (RA) registers.  Therefore the PLT0 can be represented using
> a SFrame FDE of type PCINC with a SFrame "null" FRE and the PLTn can
> be represented using a SFrame FDE of type PCMASK, with a repetition
> block size of 32 (PLTn size), and a SFrame "null" FRE.
> 
> Note that as both the PLT0 entry and the PLTn entries have equal size
> and can both be represented using the identical SFrame "null" FRE, the
> whole .plt section on s390x could be represented using a single SFrame
> FDE of type PCMASK, with a repetition block size of 32 (PLT0 and PLTn
> size), and a single SFrame "null" FRE.  Keep the x86-64 logic with
> separate SFrame FDEs for PLT0 and PLTn, to ease potential generalization
> of the .sframe for .plt generation logic among architectures.
> 
> bfd/
> 	* elf64-s390.c: Include sframe.h and sframe-api.h.
> 	(PLT_SFRAME_FDE_START_OFFSET, SFRAME_PLT0_MAX_NUM_FRES,
> 	SFRAME_PLTN_MAX_NUM_FRES, elf_s390x_sframe_plt_fre,
> 	elf_s390x_sframe_plt): New .sframe template for .plt section.
> 	(elf_s390_link_hash_table): Add plt_cfe_ctx, plt_sframe, and
> 	sframe_plt fields.
> 	(_bfd_s390x_elf_create_sframe_plt): New function.  Fill in
> 	.sframe section for .plt section.
> 	(_bfd_s390x_elf_write_sframe_plt): New function.  Write .sframe
> 	section.
> 	(elf_s390_create_dynamic_sections): Create .sframe section for
> 	.plt section.
> 	(elf_s390_late_size_sections): Call
> 	_bfd_s390x_elf_create_sframe_plt and
> 	_bfd_s390x_elf_write_sframe_plt.> 	(elf_s390_finish_dynamic_sections): Write .plt section start
> 	into .sframe FDE covering .plt section.  Call
> 	_bfd_elf_merge_section_sframe on htab->plt_sframe.
> 
> ld/
> 	* NEWS: Add news entry.
> 
> ld/testsuite/
> 	* ld-s390/s390.exp: Add new test.
> 	* ld-s390/sframe-plt-1.d: New linker-generated .sframe for .plt
> 	test.
> 	* ld-s390/sframe-simple-1.d: Adjust expected test output due to
> 	linker-generated .sframe for .plt.
> 
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> ---
>   bfd/elf64-s390.c                       | 259 +++++++++++++++++++++++++
>   ld/NEWS                                |   4 +
>   ld/testsuite/ld-s390/s390.exp          |   1 +
>   ld/testsuite/ld-s390/sframe-plt-1.d    |  28 +++
>   ld/testsuite/ld-s390/sframe-simple-1.d |   9 +-
>   5 files changed, 297 insertions(+), 4 deletions(-)
>   create mode 100644 ld/testsuite/ld-s390/sframe-plt-1.d
> 
> diff --git a/bfd/elf64-s390.c b/bfd/elf64-s390.c
> index 259ad13be51f..478d5b86ebb0 100644
> --- a/bfd/elf64-s390.c
> +++ b/bfd/elf64-s390.c
> @@ -27,6 +27,8 @@
>   #include "elf/s390.h"
>   #include "elf-s390.h"
>   #include "dwarf2.h"
> +#include "sframe.h"
> +#include "sframe-api.h"
>   #include <stdarg.h>
>   
>   /* In case we're on a 32-bit machine, construct a 64-bit "-1" value
> @@ -594,6 +596,49 @@ static const bfd_byte elf_s390x_eh_frame_plt[] =
>     DW_CFA_nop, DW_CFA_nop, DW_CFA_nop
>   };
>   
> +/* .sframe covering the .plt section.  */
> +
> +/* This must be the same as sframe_get_hdr_size (sfh).  For s390x, this value
> +   is the same as sizeof (sframe_header) because there is no SFrame auxilliary
> +   header.  */
> +#define PLT_SFRAME_FDE_START_OFFSET	sizeof (sframe_header)
> +
> +#define SFRAME_PLT0_MAX_NUM_FRES 1
> +#define SFRAME_PLTN_MAX_NUM_FRES 1
> +
> +struct elf_s390x_sframe_plt
> +{
> +  unsigned int plt0_entry_size;
> +  unsigned int plt0_num_fres;
> +  const sframe_frame_row_entry *plt0_fres[SFRAME_PLT0_MAX_NUM_FRES];
> +
> +  unsigned int pltn_entry_size;
> +  unsigned int pltn_num_fres;
> +  const sframe_frame_row_entry *pltn_fres[SFRAME_PLTN_MAX_NUM_FRES];
> +};
> +
> +/* .sframe FRE covering the PLT0/PLTn .plt section entry.  */
> +static const sframe_frame_row_entry elf_s390x_sframe_plt_fre =
> +{
> +  0, /* SFrame FRE start address.  */
> +  {0, 160, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* 12 bytes.  */
> +  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_2B) /* FRE info.  */
> +};
> +
> +/* SFrame helper object for PLT.  */
> +static const struct elf_s390x_sframe_plt elf_s390x_sframe_plt =
> +{
> +  PLT_FIRST_ENTRY_SIZE,
> +  1, /* Number of FREs for PLT0.  */
> +  /* Array of SFrame FREs for PLT0.  */
> +  { &elf_s390x_sframe_plt_fre },
> +
> +  PLT_ENTRY_SIZE,
> +  1, /* Number of FREs for PLTn.  */
> +  /* Array of SFrame FREs for PLTn.  */
> +  { &elf_s390x_sframe_plt_fre },
> +};
> +
> 

I am thinking these PLT entries are unique in some sense:
   - there is no stack usage
   - there is just one FRE in each PLT

I think it wouldn't hurt to add a testcase in 
libsframe/testsuite/libsframe.find/, similar to plt-findfre-1.c, but where:
   - the dummy FDEs (one for plt0 and another for pltN) have only one 
FRE each
   - and we invoke a few sframe_find_fre () with different IPs.

Also, looks like there is only one kind of PLT in s390x. Can you confirm ?

>   /* s390 ELF linker hash entry.  */
>   
> @@ -688,6 +733,11 @@ struct elf_s390_link_hash_table
>     asection *irelifunc;
>     asection *plt_eh_frame;
>   
> +  sframe_encoder_ctx *plt_cfe_ctx;
> +  asection *plt_sframe;
> +  /* The .sframe helper object for .plt section.  */
> +  const struct elf_s390x_sframe_plt *sframe_plt;
> +
>     union {
>       bfd_signed_vma refcount;
>       bfd_vma offset;
> @@ -1513,6 +1563,137 @@ elf_s390_adjust_dynamic_symbol (struct bfd_link_info *info,
>     return _bfd_elf_adjust_dynamic_copy (info, h, s);
>   }
>   
> +/* Create SFrame stack trace info for the PLT entries in the .plt section.  */
> +
> +static bool
> +_bfd_s390x_elf_create_sframe_plt (struct bfd_link_info *info)
> +{
> +  struct elf_s390_link_hash_table *htab;
> +
> +  unsigned int plt0_entry_size;
> +  unsigned char func_info;
> +  uint32_t fre_type;
> +  /* The dynamic plt section for which .sframe stack trace information is being
> +     created.  */
> +  asection *dpltsec;
> +
> +  int err = 0;
> +
> +  sframe_encoder_ctx **ectx = NULL;
> +  unsigned plt_entry_size = 0;
> +  unsigned int num_pltn_fres = 0;
> +  unsigned int num_pltn_entries = 0;
> +  const sframe_frame_row_entry * const *pltn_fres;
> +
> +  htab = elf_s390_hash_table (info);
> +  ectx = &htab->plt_cfe_ctx;
> +  dpltsec = htab->elf.splt;
> +
> +  plt0_entry_size = htab->sframe_plt->plt0_entry_size;
> +  plt_entry_size = htab->sframe_plt->pltn_entry_size;
> +  pltn_fres = htab->sframe_plt->pltn_fres;
> +  num_pltn_fres = htab->sframe_plt->pltn_num_fres;
> +  num_pltn_entries = (dpltsec->size - plt0_entry_size) / plt_entry_size;
> +
> +  *ectx = sframe_encode (SFRAME_VERSION_2,
> +			 0,
> +			 SFRAME_ABI_S390_ENDIAN_BIG,
> +			 SFRAME_CFA_FIXED_FP_INVALID,
> +			 SFRAME_CFA_FIXED_RA_INVALID,
> +			 &err);
> +
> +  /* FRE type is dependent on the size of the function.  */
> +  fre_type = sframe_calc_fre_type (dpltsec->size);
> +  func_info = sframe_fde_create_func_info (fre_type, SFRAME_FDE_TYPE_PCINC);
> +
> +  /* Add SFrame FDE and the associated FREs for PLT0 if PLT0 has been
> +     generated.  */
> +  if (plt0_entry_size)
> +    {
> +      /* Add SFrame FDE for PLT0, the function start address is updated later
> +	 at _bfd_elf_merge_section_sframe time.  */
> +      sframe_encoder_add_funcdesc_v2 (*ectx,
> +				      0, /* func start addr.  */
> +				      plt0_entry_size,
> +				      func_info,
> +				      0, /* Rep block size.  */
> +				      0 /* Num FREs.  */);
> +      sframe_frame_row_entry plt0_fre;
> +      unsigned int num_plt0_fres = htab->sframe_plt->plt0_num_fres;
> +      for (unsigned int j = 0; j < num_plt0_fres; j++)
> +	{
> +	  plt0_fre = *(htab->sframe_plt->plt0_fres[j]);
> +	  sframe_encoder_add_fre (*ectx, 0, &plt0_fre);
> +	}
> +    }
> +
> +  if (num_pltn_entries)
> +    {
> +      /* PLTn entries use an SFrame FDE of type
> +	 SFRAME_FDE_TYPE_PCMASK to exploit the repetitive
> +	 pattern of the instructions in these entries.  Using this SFrame FDE
> +	 type helps in keeping the SFrame stack trace info for PLTn entries
> +	 compact.  */
> +      func_info	= sframe_fde_create_func_info (fre_type,
> +					       SFRAME_FDE_TYPE_PCMASK);
> +      /* Add the SFrame FDE for all PCs starting at the first PLTn entry (hence,
> +	 function start address = plt0_entry_size.  As usual, this will be
> +	 updated later at _bfd_elf_merge_section_sframe, by when the
> +	 sections are relocated.  */
> +      sframe_encoder_add_funcdesc_v2 (*ectx,
> +				      plt0_entry_size, /* func start addr.  */
> +				      dpltsec->size - plt0_entry_size,
> +				      func_info,
> +				      plt_entry_size,
> +				      0 /* Num FREs.  */);
> +
> +      sframe_frame_row_entry pltn_fre;
> +      /* Now add the FREs for PLTn.  Simply adding the FREs suffices due
> +	 to the usage of SFRAME_FDE_TYPE_PCMASK above.  */
> +      for (unsigned int j = 0; j < num_pltn_fres; j++)
> +	{
> +	  unsigned int func_idx = plt0_entry_size ? 1 : 0;
> +	  pltn_fre = *(pltn_fres[j]);
> +	  sframe_encoder_add_fre (*ectx, func_idx, &pltn_fre);
> +	}
> +    }
> +
> +  return true;
> +}
> +
> +/* Write contents of the .sframe section.  */
> +
> +static bool
> +_bfd_s390x_elf_write_sframe_plt (struct bfd_link_info *info)
> +{
> +  struct elf_s390_link_hash_table *htab;
> +  sframe_encoder_ctx *ectx;
> +  size_t sec_size;
> +  asection *sec;
> +  bfd *dynobj;
> +
> +  int err = 0;
> +
> +  htab = elf_s390_hash_table (info);
> +  dynobj = htab->elf.dynobj;
> +
> +  ectx = htab->plt_cfe_ctx;
> +  sec = htab->plt_sframe;
> +
> +  BFD_ASSERT (ectx);
> +
> +  void *contents = sframe_encoder_write (ectx, &sec_size, &err);
> +
> +  sec->size = (bfd_size_type) sec_size;
> +  sec->contents = (unsigned char *) bfd_zalloc (dynobj, sec->size);
> +  sec->alloced = 1;
> +  memcpy (sec->contents, contents, sec_size);
> +
> +  sframe_encoder_free (&ectx);
> +
> +  return true;
> +}
> +
>   /* Allocate space in .plt, .got and associated reloc sections for
>      dynamic relocs.  */
>   
> @@ -1892,6 +2073,25 @@ elf_s390_late_size_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
>   	htab->plt_eh_frame->size = sizeof (elf_s390x_eh_frame_plt);
>       }
>   
> +  /* No need to size the .sframe section explicitly because the write-out
> +     mechanism is different.  Simply prep up the FDE/FRE for the
> +     .plt section.  */
> +  if (_bfd_elf_sframe_present (info))
> +    {
> +      if (htab->plt_sframe != NULL
> +	  && htab->elf.splt != NULL
> +	  && htab->elf.splt->size != 0
> +	  && !bfd_is_abs_section (htab->elf.splt->output_section))
> +	{
> +	  _bfd_s390x_elf_create_sframe_plt (info);
> +	  /* FIXME - Dirty Hack.  Set the size to something non-zero for now,
> +	     so that the section does not get stripped out below.  The precise
> +	     size of this section is known only when the contents are
> +	     serialized in _bfd_s390x_elf_write_sframe_plt.  */
> +	  htab->plt_sframe->size = sizeof (sframe_header) + 1;
> +	}
> +    }
> +
>     /* We now have determined the sizes of the various dynamic sections.
>        Allocate memory for them.  */
>     relocs = false;
> @@ -1904,6 +2104,7 @@ elf_s390_late_size_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
>   	  || s == htab->elf.sgot
>   	  || s == htab->elf.sgotplt
>   	  || s == htab->plt_eh_frame
> +	  || s == htab->plt_sframe
>   	  || s == htab->elf.sdynbss
>   	  || s == htab->elf.sdynrelro
>   	  || s == htab->elf.iplt
> @@ -1960,6 +2161,11 @@ elf_s390_late_size_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
>         if ((s->flags & SEC_HAS_CONTENTS) == 0)
>   	continue;
>   
> +      /* Skip allocating contents for .sframe section as it is written
> +	 out differently.  See below.  */
> +      if (s == htab->plt_sframe)
> +	continue;
> +
>         /* Allocate memory for the section contents.  We use bfd_zalloc
>   	 here in case unused entries are not reclaimed before the
>   	 section's contents are written out.  This should not happen,
> @@ -1981,6 +2187,15 @@ elf_s390_late_size_sections (bfd *output_bfd ATTRIBUTE_UNUSED,
>   		  htab->plt_eh_frame->contents + PLT_FDE_LEN_OFFSET);
>       }
>   
> +  if (_bfd_elf_sframe_present (info))
> +    {
> +      if (htab->plt_sframe != NULL
> +	  && htab->elf.splt != NULL
> +	  && htab->elf.splt->size != 0
> +	  && htab->plt_sframe->contents == NULL)
> +	_bfd_s390x_elf_write_sframe_plt (info);
> +    }
> +
>     return _bfd_elf_add_dynamic_tags (output_bfd, info, relocs);
>   }
>   
> @@ -3786,6 +4001,34 @@ elf_s390_finish_dynamic_sections (bfd *output_bfd,
>   	}
>       }
>   
> +  /* Make any adjustment if necessary and merge .sframe section to
> +     create the final .sframe section for output_bfd.  */
> +  if (htab->plt_sframe != NULL
> +      && htab->plt_sframe->contents != NULL)
> +    {
> +      if (htab->elf.splt != NULL
> +	  && htab->elf.splt->size != 0
> +	  && (htab->elf.splt->flags & SEC_EXCLUDE) == 0
> +	  && htab->elf.splt->output_section != NULL
> +	  && htab->plt_sframe->output_section != NULL)
> +	{
> +	  bfd_vma plt_start = htab->elf.splt->output_section->vma;
> +	  bfd_vma sframe_start = htab->plt_sframe->output_section->vma
> +				   + htab->plt_sframe->output_offset
> +				   + PLT_SFRAME_FDE_START_OFFSET;
> +	  bfd_put_signed_32 (dynobj, plt_start - sframe_start,
> +			     htab->plt_sframe->contents
> +			     + PLT_SFRAME_FDE_START_OFFSET);
> +	}
> +      if (htab->plt_sframe->sec_info_type == SEC_INFO_TYPE_SFRAME)
> +	{
> +	  if (! _bfd_elf_merge_section_sframe (output_bfd, info,
> +					       htab->plt_sframe,
> +					       htab->plt_sframe->contents))
> +	    return NULL;

Function has a return type of bool.  So:
   return false;

> +	}
> +    }
> +
>     return true;
>   }
>   
> @@ -4026,6 +4269,8 @@ elf_s390_create_dynamic_sections (bfd *dynobj,
>     if (htab == NULL)
>       return false;
>   
> +  htab->sframe_plt = &elf_s390x_sframe_plt;
> +
>     if (htab->elf.splt != NULL)
>       {
>         /* Create .eh_frame section for .plt section.  */
> @@ -4046,6 +4291,20 @@ elf_s390_create_dynamic_sections (bfd *dynobj,
>                   return false;
>               }
>           }
> +
> +      /* Create .sframe section for .plt section.  */
> +      if (!info->no_ld_generated_unwind_info)
> +	{

Should we also check that its s390x and not s390 (i.e. ELFCLASS64 ?) ?

> +	  flagword flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY
> +			    | SEC_HAS_CONTENTS | SEC_IN_MEMORY
> +			    | SEC_LINKER_CREATED);
> +
> +	  htab->plt_sframe = bfd_make_section_anyway_with_flags (dynobj,
> +								 ".sframe",
> +								 flags);
> +	  if (htab->plt_sframe == NULL)
> +	    return false;
> +	}
>       }
>   
>     return true;
> diff --git a/ld/NEWS b/ld/NEWS
> index 7b5e2e47c07b..f59f5e181a65 100644
> --- a/ld/NEWS
> +++ b/ld/NEWS
> @@ -1,5 +1,9 @@
>   -*- text -*-
>   
> +* On s390, generate SFrame stack trace information (.sframe) for the linker
> +  generated .plt section.  Enabled by default.  Can be disabled using linker
> +  option --no-ld-generated-unwind-info.
> +
>   * The linker's --stats option can take an optional argument which if used is
>     interpreted as a filename into which resource usage information should be
>     stored.  As an alternative mechanism the LD_STATS environment variable can
> diff --git a/ld/testsuite/ld-s390/s390.exp b/ld/testsuite/ld-s390/s390.exp
> index cf3a904481f8..38758cfa9ea9 100644
> --- a/ld/testsuite/ld-s390/s390.exp
> +++ b/ld/testsuite/ld-s390/s390.exp
> @@ -141,5 +141,6 @@ if [istarget "s390x-*-*"] {
>   
>       if { ![skip_sframe_tests] } {
>   	run_dump_test "sframe-simple-1"
> +	run_dump_test "sframe-plt-1"
>       }
>   }
> diff --git a/ld/testsuite/ld-s390/sframe-plt-1.d b/ld/testsuite/ld-s390/sframe-plt-1.d
> new file mode 100644
> index 000000000000..67b11bd3dcf4
> --- /dev/null
> +++ b/ld/testsuite/ld-s390/sframe-plt-1.d
> @@ -0,0 +1,28 @@
> +#as: --gsframe
> +#source: sframe-foo.s
> +#source: sframe-bar.s
> +#objdump: --sframe=.sframe
> +#ld: -shared --no-rosegment
> +#name: SFrame for plt0 and pltN
> +
> +.*: +file format .*
> +
> +Contents of the SFrame section .sframe:
> +  Header :
> +
> +    Version: SFRAME_VERSION_2
> +    Flags: SFRAME_F_FDE_SORTED
> +    Num FDEs: 4
> +    Num FREs: 8
> +
> +  Function Index :
> +
> +    func idx \[0\]: pc = 0x1e8, size = 32 bytes
> +    STARTPC +CFA +FP +RA +
> +    0+1e8 +sp\+160 +u +u +
> +
> +    func idx \[1\]: pc = 0x208, size = 32 bytes
> +    STARTPC\[m\] +CFA +FP +RA +
> +    0+0 +sp\+160 +u +u +
> +
> +#...
> diff --git a/ld/testsuite/ld-s390/sframe-simple-1.d b/ld/testsuite/ld-s390/sframe-simple-1.d
> index 4df5d492f01e..2967278e9f86 100644
> --- a/ld/testsuite/ld-s390/sframe-simple-1.d
> +++ b/ld/testsuite/ld-s390/sframe-simple-1.d
> @@ -12,16 +12,17 @@ Contents of the SFrame section .sframe:
>   
>       Version: SFRAME_VERSION_2
>       Flags: SFRAME_F_FDE_SORTED
> -    Num FDEs: 2
> -    Num FREs: 6
> +    Num FDEs: 4
> +    Num FREs: 8
>   
>     Function Index :
>   
> -    func idx \[0\]: pc = 0x228, size = 8 bytes
> +#...
> +    func idx \[2\]: pc = 0x228, size = 8 bytes
>       STARTPC +CFA +FP +RA +
>       0+228 +sp\+160 +u +u +
>   
> -    func idx \[1\]: pc = 0x230, size = 42 bytes
> +    func idx \[3\]: pc = 0x230, size = 42 bytes
>       STARTPC +CFA +FP +RA +
>       0+230 +sp\+160 +u +u +
>       0+236 +sp\+160 +u +c-48 +
  

Hello Indu,

thank you for the review feedback!

On 16.04.2025 08:14, Indu Bhagat wrote:
> On 4/2/25 9:12 AM, Jens Remus wrote:
>> Enable SFrame stack tracing through PLT entries.  Based on x86-64.
>>
>> On s390x both PLT0 and PLTn entries are 32-bytes in size.  Their code
>> neither alters the stack pointer (SP), frame pointer (FP), nor return
>> address (RA) registers.  Therefore the PLT0 can be represented using
>> a SFrame FDE of type PCINC with a SFrame "null" FRE and the PLTn can
>> be represented using a SFrame FDE of type PCMASK, with a repetition
>> block size of 32 (PLTn size), and a SFrame "null" FRE.
>>
>> Note that as both the PLT0 entry and the PLTn entries have equal size
>> and can both be represented using the identical SFrame "null" FRE, the
>> whole .plt section on s390x could be represented using a single SFrame
>> FDE of type PCMASK, with a repetition block size of 32 (PLT0 and PLTn
>> size), and a single SFrame "null" FRE.  Keep the x86-64 logic with
>> separate SFrame FDEs for PLT0 and PLTn, to ease potential generalization
>> of the .sframe for .plt generation logic among architectures.

>> diff --git a/bfd/elf64-s390.c b/bfd/elf64-s390.c

>> +/* .sframe covering the .plt section.  */
>> +
>> +/* This must be the same as sframe_get_hdr_size (sfh).  For s390x, this value
>> +   is the same as sizeof (sframe_header) because there is no SFrame auxilliary
>> +   header.  */
>> +#define PLT_SFRAME_FDE_START_OFFSET    sizeof (sframe_header)
>> +
>> +#define SFRAME_PLT0_MAX_NUM_FRES 1
>> +#define SFRAME_PLTN_MAX_NUM_FRES 1
>> +
>> +struct elf_s390x_sframe_plt
>> +{
>> +  unsigned int plt0_entry_size;
>> +  unsigned int plt0_num_fres;
>> +  const sframe_frame_row_entry *plt0_fres[SFRAME_PLT0_MAX_NUM_FRES];
>> +
>> +  unsigned int pltn_entry_size;
>> +  unsigned int pltn_num_fres;
>> +  const sframe_frame_row_entry *pltn_fres[SFRAME_PLTN_MAX_NUM_FRES];
>> +};
>> +
>> +/* .sframe FRE covering the PLT0/PLTn .plt section entry.  */
>> +static const sframe_frame_row_entry elf_s390x_sframe_plt_fre =
>> +{
>> +  0, /* SFrame FRE start address.  */
>> +  {0, 160, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* 12 bytes.  */
>> +  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_2B) /* FRE info.  */
>> +};
>> +
>> +/* SFrame helper object for PLT.  */
>> +static const struct elf_s390x_sframe_plt elf_s390x_sframe_plt =
>> +{
>> +  PLT_FIRST_ENTRY_SIZE,
>> +  1, /* Number of FREs for PLT0.  */
>> +  /* Array of SFrame FREs for PLT0.  */
>> +  { &elf_s390x_sframe_plt_fre },
>> +
>> +  PLT_ENTRY_SIZE,
>> +  1, /* Number of FREs for PLTn.  */
>> +  /* Array of SFrame FREs for PLTn.  */
>> +  { &elf_s390x_sframe_plt_fre },
>> +};
>> +
>>
> 
> I am thinking these PLT entries are unique in some sense:
>    - there is no stack usage
>    - there is just one FRE in each PLT
> 
> I think it wouldn't hurt to add a testcase in
> libsframe/testsuite/libsframe.find/, similar to plt-findfre-1.c,
> but where:
>    - the dummy FDEs (one for plt0 and another for pltN) have only
>      one FRE each
>    - and we invoke a few sframe_find_fre () with different IPs.

I'll have a look.


> Also, looks like there is only one kind of PLT in s390x. Can you
> confirm ?

The GNU linker (ld) on s390x (64-bit ELF ABI) generates only one type of
entry for PLT0 and PLTn respectively:

- PLT0: See elf_s390x_first_plt_entry in bfd/elf64-s390.c
- PLTn: See elf_s390x_plt_entry in bfd/elf64-s390.c

On s390 (32-bit ELF ABI) there would be different flavors, whether the
PLT entries are for PIC or non-PIC (see elf_s390_plt_* in elf32-s390.c).

Note that other linkers may behave differently.  Mold for instance is
known to generate a different instruction sequence compared to GNU ld
for PLT entries on s390x. [1]

>> @@ -3786,6 +4001,34 @@ elf_s390_finish_dynamic_sections (bfd *output_bfd,
>>       }
>>       }
>> +  /* Make any adjustment if necessary and merge .sframe section to
>> +     create the final .sframe section for output_bfd.  */
>> +  if (htab->plt_sframe != NULL
>> +      && htab->plt_sframe->contents != NULL)
>> +    {
>> +      if (htab->elf.splt != NULL
>> +      && htab->elf.splt->size != 0
>> +      && (htab->elf.splt->flags & SEC_EXCLUDE) == 0
>> +      && htab->elf.splt->output_section != NULL
>> +      && htab->plt_sframe->output_section != NULL)
>> +    {
>> +      bfd_vma plt_start = htab->elf.splt->output_section->vma;
>> +      bfd_vma sframe_start = htab->plt_sframe->output_section->vma
>> +                   + htab->plt_sframe->output_offset
>> +                   + PLT_SFRAME_FDE_START_OFFSET;
>> +      bfd_put_signed_32 (dynobj, plt_start - sframe_start,
>> +                 htab->plt_sframe->contents
>> +                 + PLT_SFRAME_FDE_START_OFFSET);
>> +    }
>> +      if (htab->plt_sframe->sec_info_type == SEC_INFO_TYPE_SFRAME)
>> +    {
>> +      if (! _bfd_elf_merge_section_sframe (output_bfd, info,
>> +                           htab->plt_sframe,
>> +                           htab->plt_sframe->contents))
>> +        return NULL;
> 
> Function has a return type of bool.  So:
>    return false;

Good catch!  Fixed.

> 
>> +    }
>> +    }
>> +
>>     return true;
>>   }
>>   
>> @@ -4026,6 +4269,8 @@ elf_s390_create_dynamic_sections (bfd *dynobj,
>>     if (htab == NULL)
>>       return false;
>> +  htab->sframe_plt = &elf_s390x_sframe_plt;
>> +
>>     if (htab->elf.splt != NULL)
>>       {
>>         /* Create .eh_frame section for .plt section.  */
>> @@ -4046,6 +4291,20 @@ elf_s390_create_dynamic_sections (bfd *dynobj,
>>                   return false;
>>               }
>>           }
>> +
>> +      /* Create .sframe section for .plt section.  */
>> +      if (!info->no_ld_generated_unwind_info)
>> +    {
> 
> Should we also check that its s390x and not s390 (i.e. ELFCLASS64 ?) ?

No, this is not necessary, given this is in bfd/elf64-s390.c
(i.e. ELFCLASS64) and not bfd/elf32-s390.c (i.e. ELFCLASS32).

> 
>> +      flagword flags = (SEC_ALLOC | SEC_LOAD | SEC_READONLY
>> +                | SEC_HAS_CONTENTS | SEC_IN_MEMORY
>> +                | SEC_LINKER_CREATED);
>> +
>> +      htab->plt_sframe = bfd_make_section_anyway_with_flags (dynobj,
>> +                                 ".sframe",
>> +                                 flags);
>> +      if (htab->plt_sframe == NULL)
>> +        return false;
>> +    }
>>       }
>>     return true;

[1]: MaskRay: Toolchain notes on z/Architecture, section "Procedure
      Linkage Table,
      https://maskray.me/blog/2024-02-11-toolchain-notes-on-z-architecture

Regards,
Jens