[RFC 2/5] elf: Add SFrame stack tracing

[Jens Remus]

Hello Claudiu,

following are a few mostly generic remarks regarding the SFrame
processing, that I stumbled upon implementing experimental  s390-64
(s390x) support on top of your series.

On 18.03.2025 14:03, claudiu.zissulescu-ianculescu@oracle.com wrote:

> This patch adds the necessary bits to enable stack tracing using
> SFrame.  In the case the new SFrame stack tracing procedure doesn't
> find SFrame related info, the stack tracing falls back on default
> Dwarf implementation.
> 
> The new SFrame stack tracing procedure is added to bebug/backtrace.c
> file, the support functions are added in sysdeps folder, namely
> sframe.h, read-sframe.c and read-sfame.h.

> diff --git a/sysdeps/generic/sframe-read.c b/sysdeps/generic/sframe-read.c

> +/* Get the CFA offset from the FRE.  If the offset is invalid, sets errp.  */
> +
> +int32_t
> +sframe_fre_get_cfa_offset (sframe_decoder_ctx *dctx __attribute__ ((__unused__)),
> +			   sframe_frame_row_entry *fre)
> +{
> +  _Unwind_Reason_Code err;
> +  return sframe_get_fre_offset (fre, SFRAME_FRE_CFA_OFFSET_IDX, &err);
> +}

You intended to set errp, but then left that out.  While a SFrame FRE
currently is required to have at least the CFA offset (to unwind the SP)
there could be corrupted SFrame information that does not have one.
Also since the code does not validate the CFA offset value I would not
use the term "invalid" but "unavailable" (or the like) instead.

/* Get the CFA offset from the FRE.  If the offset is unavailable, sets errp.  */

int32_t
sframe_fre_get_cfa_offset (sframe_decoder_ctx *dctx __attribute__ ((__unused__)),
			   sframe_frame_row_entry *fre,
			   _Unwind_Reason_Code *errp)
{
   return sframe_get_fre_offset (fre, SFRAME_FRE_CFA_OFFSET_IDX, errp);
}

> +/* Get the FP offset from the FRE.  If the offset is invalid, sets errp.  */

Since the code does not validate the FP offset value I would not use the
term "invalid" but "unavailable" (or the like) instead.

> +
> +int32_t
> +sframe_fre_get_fp_offset (sframe_decoder_ctx *dctx,
> +			  sframe_frame_row_entry *fre,
> +			  _Unwind_Reason_Code *errp)

> +/* Get the RA offset from the FRE.  If the offset is invalid, sets errp.  */

Similar as for sframe_fre_get_cfa_offset.  You intended to set errp, but
then left that out.  As x86-64 does not use SFrame RA tracking and has a
fixed RA stack slot offset from CFA instead, it always has a RA offset.
Not sure about AArch64, which uses SFrame RA tracking and does not have
a fixed RA offset.  On s390x the funcion from the topmost frame does not
necessarily need to save the RA and/or FP register, for instance while in
the function prologue or in a leaf function.
Also since the code does not validate the CFA offset value I would not
use the term "invalid" but "unavailable" (or the like) instead.

> +
> +int32_t
> +sframe_fre_get_ra_offset (sframe_decoder_ctx *dctx,
> +			  sframe_frame_row_entry *fre)

Add:
			  _Unwind_Reason_Code *errp)

> +{
> +  _Unwind_Reason_Code err;

Replace above line by:

   *errp = _URC_NO_REASON;

> +  int8_t ra_offset = sframe_decoder_get_fixed_ra_offset (dctx);
> +  /* If the RA offset was not being tracked, return the fixed RA offset
> +     from the SFrame header.  */
> +  if (ra_offset != SFRAME_CFA_FIXED_RA_INVALID)
> +    return ra_offset;
> +
> +  /* Otherwise, get the RA offset from the FRE.  */
> +  return sframe_get_fre_offset (fre, SFRAME_FRE_RA_OFFSET_IDX, &err);

Replace above line by:

   return sframe_get_fre_offset (fre, SFRAME_FRE_RA_OFFSET_IDX, errp);

> +}

Similar as for sframe_fre_get_cfa_offset.  You intended to set errp, but
then left that out.  As x86-64 does not use SFrame RA tracking and has a
fixed RA stack slot offset from CFA instead, it always has a RA offset.
Not sure about AArch64, which uses SFrame RA tracking and does not have
a fixed RA offset.  On s390x the function from the topmost frame does not
necessarily need to save the RA and/or FP register, for instance while in
the function prologue or in a leaf function.
Also since the code does not validate the CFA offset value I would not
use the term "invalid" but "unavailable" (or the like) instead.

/* Get the RA offset from the FRE.  If the offset is unavailable, sets errp.  */

int32_t
sframe_fre_get_ra_offset (sframe_decoder_ctx *dctx,
			  sframe_frame_row_entry *fre,
			  _Unwind_Reason_Code *errp)
{
   *errp = _URC_NO_REASON;
   int8_t ra_offset = sframe_decoder_get_fixed_ra_offset (dctx);

}

> diff --git a/sysdeps/generic/sframe-read.h b/sysdeps/generic/sframe-read.h

> +/* Get the CFA offset from the FRE.  */

/* Get the CFA offset from the FRE.  If the offset is unavailable, sets errp.  */

> +
> +extern int32_t
> +sframe_fre_get_cfa_offset (sframe_decoder_ctx *dtcx,
> +			   sframe_frame_row_entry *fre);

Add:
			   _Unwind_Reason_Code *errp);

> +
> +/* Get the FP offset from the FRE.  If the offset is invalid, sets errp.  */

s/invalid/unavailable/ (or the like - see above).

> +
> +extern int32_t
> +sframe_fre_get_fp_offset (sframe_decoder_ctx *dctx,
> +			  sframe_frame_row_entry *fre,
> +			  _Unwind_Reason_Code *errp);
> +
> +/* Get the RA offset from the FRE.  */

/* Get the RA offset from the FRE.  If the offset is unavailable, sets errp.  */

> +
> +extern int32_t
> +sframe_fre_get_ra_offset (sframe_decoder_ctx *dctx,
> +			  sframe_frame_row_entry *fre);

Add:
			   _Unwind_Reason_Code *errp);

> diff --git a/sysdeps/generic/sframe.c b/sysdeps/generic/sframe.c

> +/* Backtrace the stack and collect the stacktrace given SFrame info.
> +   If successful, store the return addresses in RA_LST. The SIZE
> +   argument specifies the maximum number of return addresses that can
> +   be stored in RA_LST and contains the number of the addresses
> +   collected.  */
> +
> +int
> +stacktrace_sframe (void **ra_lst, int count, frame *frame)
> +{
> +  _Unwind_Ptr sframe_vma, cfa, return_addr, ra_stack_loc, rfp_stack_loc, pc;
> +  int cfa_offset, rfp_offset, ra_offset, i;
> +  sframe_frame_row_entry fred, *frep = &fred;
> +
> +  if (!ra_lst || !count)
> +    return 0;
> +
> +  for (i = 0; i < count; i++)
> +    {

...

> +      /* Get the CFA offset from the FRE.  */
> +      cfa_offset = sframe_fre_get_cfa_offset (dctx, frep);

       /* Get the CFA offset from the FRE.  If the offset is unavailable,
          sets errp.  */
       cfa_offset = sframe_fre_get_cfa_offset (dctx, frep, &err);
       if (err != _URC_NO_REASON)
         return i;

> +
> +      /* Get the base reg id from the FRE info.  */
> +      cfa = ((sframe_fre_get_base_reg_id (frep)
> +	      == SFRAME_BASE_REG_SP) ? frame->sp : frame->fp) + cfa_offset;
> +
> +      /* Get the RA offset from the FRE.  */
> +      ra_offset = sframe_fre_get_ra_offset (dctx, frep);

       /* Get the RA offset from the FRE.  If the offset is unavailable,
         sets errp.  */
       ra_offset = sframe_fre_get_ra_offset (dctx, frep, &err);
       if (err != _URC_NO_REASON)
         return i;

> +

      /* RA offset is available, get the value stored in the stack
        location.  */

> +      ra_stack_loc = cfa + ra_offset;
> +      return_addr = read_stack_value (ra_stack_loc);
> +
> +      ra_lst[i] = (void *)return_addr;
> +
> +      /* Set up for the next frame.  */
> +      /* Get the FP offset from the FRE.  If the offset is invalid,
> +	 sets errp.  */

Nit: s/invalid/unavailable/ (or the like).

> +      rfp_offset = sframe_fre_get_fp_offset (dctx, frep, &err);
> +      if (err == _URC_NO_REASON)
> +	{
> +	  /* Frame is valid, get the value stored in the stack
> +	     location.  */

Nit: s/valid/available/ (or the like).

> +	  rfp_stack_loc = cfa + rfp_offset;
> +	  frame->fp = read_stack_value (rfp_stack_loc);
> +	}
> +
> +      frame->sp = cfa;
> +      frame->pc = return_addr;
> +    }
> +  return i;
> +}

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
+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/