[RFC,SCHEME_B,5/7] libsframe: SFrame FDE function start addr is an offset from FDE

Message ID 20250407002559.6593-6-indu.bhagat@oracle.com
State New
Headers
Series Fix relocatable links with SFrame section |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 warning Skipped upon request
linaro-tcwg-bot/tcwg_binutils_build--master-arm warning Skipped upon request

Commit Message

Indu Bhagat April 7, 2025, 12:25 a.m. UTC
  Naturally, the change of semantics for 'SFrame FDE function start address'
has consequences on the implementation in libsframe.  As per the new
semantics:
  - Function start address in the SFrame FDE (sfde_func_start_address)
    is an offset from the FDE function start address field to the start
    PC of the associated function.

Note that, the libsframe library brings the SFrame section contents into
its own memory to create a sframe_decoder_ctx object via sframe_decode
().  Many internal and user-interfacing APIs then use sframe_decoder_ctx
object to interact and fulfill the work.

In context of changing semantics for sfde_func_start_address, following
relevant examples may help understand the impact:
  - sframe_find_fre () finds a the SFrame stack trace data (SFrame FRE)
    given a PC.  Now that the sfde_func_start_address includes the
    distance from the sfde_func_start_address field to the start of SFrame
    section itself, the comparison checks of sfde_func_start_address
    with the given PC need adjustment.
  - Some internal functions (sframe_get_funcdesc_with_addr_internal ()
    finds SFrame FDE by using binary seach comparing
    sfde_func_start_address fields, etc.) need adjustments.
  - sframe_encoder_write () sorts the SFrame FDEs before writing out
    the SFrame data.  Sorting of SFrame FDE via the internal function
    sframe_sort_funcdesc() needs adjustments: the new encoding of
    sfde_func_start_address means the distances are not from the same
    anchor, so cannot be sorted directly.

This patch takes the approach of adding two new internal functions:
  - sframe_offsetof_fde_func_start_addr ():  This function calculates the
    offset of SFrame FDE function start address field from the start of
    the SFrame section.
  - sframe_decoder_get_secrel_func_start_addr (): This function returns
    the offset of the start PC of the function from the start of SFrame
    section.

As the sframe_decoder_get_secrel_func_start_addr () API needs the value
of the function index in the FDE list, another internal API needs
sframe_fre_check_range_p () adjustments too.

Sorting the FDEs (via sframe_sort_funcdesc ()) is done by first bringing
all offsets in sfde_func_start_address relative to start of SFrame
section, followed by sorting, and then readjusting the offsets accroding
to the new position in the FDE list.

TBD:
 - FIXME add code comments. add documentation.
 - Version bump libsframe.  The change in encoding of
   sfde_func_start_address means the APIs sframe_encoder_add_funcdesc ()
   and sframe_find_fre () etc. are now backwards incompatible with previous
   releases.

ChangeLog:

        * libsframe/sframe.c (sframe_offsetof_fde_func_start_addr): New
	static function.
        (sframe_decoder_get_secrel_func_start_addr): Likewise.
        (sframe_fre_check_range_p): Adjust the interface a bit.
	(sframe_get_funcdesc_with_addr_internal): Use
	sframe_decoder_get_secrel_func_start_addr () when comparing
	sfde_func_start_address with user input offset.
        (sframe_find_fre): Adopt the new semantics.
        (sframe_sort_funcdesc): Likewise.
---
 libsframe/sframe.c | 68 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 53 insertions(+), 15 deletions(-)
  

Patch

diff --git a/libsframe/sframe.c b/libsframe/sframe.c
index c2693b978ec..dbd09f86857 100644
--- a/libsframe/sframe.c
+++ b/libsframe/sframe.c
@@ -363,15 +363,33 @@  sframe_decoder_get_funcdesc_at_index (sframe_decoder_ctx *ctx,
   return fdep;
 }
 
+static int32_t
+sframe_offsetof_fde_func_start_addr (size_t sframe_hdr_size,
+				     uint32_t func_idx)
+{
+  return (sframe_hdr_size + func_idx * sizeof (sframe_func_desc_entry));
+}
+
+static int32_t
+sframe_decoder_get_secrel_func_start_addr (sframe_decoder_ctx *dctx,
+					   uint32_t func_idx)
+{
+  int32_t func_start_addr = dctx->sfd_funcdesc[func_idx].sfde_func_start_address;
+  size_t sframe_hdr_size = sframe_decoder_get_hdr_size (dctx);
+  return (func_start_addr
+	  + sframe_offsetof_fde_func_start_addr (sframe_hdr_size, func_idx));
+}
 /* Check whether for the given FDEP, the SFrame Frame Row Entry identified via
    the START_IP_OFFSET and the END_IP_OFFSET, provides the stack trace
    information for the PC.  */
 
 static bool
-sframe_fre_check_range_p (sframe_func_desc_entry *fdep,
+sframe_fre_check_range_p (sframe_decoder_ctx *dctx,
+			  uint32_t func_idx,
 			  int32_t start_ip_offset, int32_t end_ip_offset,
 			  int32_t pc)
 {
+  sframe_func_desc_entry *fdep;
   int32_t start_ip, end_ip;
   int32_t func_start_addr;
   uint8_t rep_block_size;
@@ -382,10 +400,8 @@  sframe_fre_check_range_p (sframe_func_desc_entry *fdep,
 
   ret = false;
 
-  if (!fdep)
-    return ret;
-
-  func_start_addr = fdep->sfde_func_start_address;
+  fdep = &dctx->sfd_funcdesc[func_idx];
+  func_start_addr = sframe_decoder_get_secrel_func_start_addr (dctx, func_idx);
   fde_type = sframe_get_fde_type (fdep);
   mask_p = (fde_type == SFRAME_FDE_TYPE_PCMASK);
   rep_block_size = fdep->sfde_func_rep_size;
@@ -1032,7 +1048,7 @@  sframe_get_funcdesc_with_addr (sframe_decoder_ctx *ctx __attribute__ ((unused)),
 
 static sframe_func_desc_entry *
 sframe_get_funcdesc_with_addr_internal (sframe_decoder_ctx *ctx, int32_t addr,
-					int *errp)
+					int *errp, uint32_t *func_idx)
 {
   sframe_header *dhp;
   sframe_func_desc_entry *fdp;
@@ -1059,15 +1075,24 @@  sframe_get_funcdesc_with_addr_internal (sframe_decoder_ctx *ctx, int32_t addr,
     {
       int mid = low + (high - low) / 2;
 
-      if (fdp[mid].sfde_func_start_address == addr)
-	return fdp + mid;
+      if (sframe_decoder_get_secrel_func_start_addr (ctx, mid) == addr)
+	{
+	  *func_idx = mid;
+	  return fdp + mid;
+	}
 
-      if (fdp[mid].sfde_func_start_address < addr)
+      if (sframe_decoder_get_secrel_func_start_addr (ctx, mid) < addr)
 	{
 	  if (mid == (cnt - 1)) 	/* Check if it's the last one.  */
-	    return fdp + (cnt - 1);
-	  else if (fdp[mid+1].sfde_func_start_address > addr)
-	    return fdp + mid;
+	    {
+	      *func_idx = cnt - 1;
+	      return fdp + (cnt - 1);
+	    }
+	  else if (sframe_decoder_get_secrel_func_start_addr (ctx, mid+1) > addr)
+	    {
+	      *func_idx = mid;
+	      return fdp + mid;
+	    }
 	  low = mid + 1;
 	}
       else
@@ -1112,6 +1137,7 @@  sframe_find_fre (sframe_decoder_ctx *ctx, int32_t pc,
 {
   sframe_frame_row_entry cur_fre;
   sframe_func_desc_entry *fdep;
+  uint32_t func_idx;
   uint32_t fre_type, fde_type, i;
   int32_t start_ip_offset;
   int32_t func_start_addr;
@@ -1125,7 +1151,7 @@  sframe_find_fre (sframe_decoder_ctx *ctx, int32_t pc,
     return sframe_set_errno (&err, SFRAME_ERR_INVAL);
 
   /* Find the FDE which contains the PC, then scan its fre entries.  */
-  fdep = sframe_get_funcdesc_with_addr_internal (ctx, pc, &err);
+  fdep = sframe_get_funcdesc_with_addr_internal (ctx, pc, &err, &func_idx);
   if (fdep == NULL || ctx->sfd_fres == NULL)
     return sframe_set_errno (&err, SFRAME_ERR_DCTX_INVAL);
 
@@ -1134,7 +1160,7 @@  sframe_find_fre (sframe_decoder_ctx *ctx, int32_t pc,
   mask_p = (fde_type == SFRAME_FDE_TYPE_PCMASK);
 
   fres = ctx->sfd_fres + fdep->sfde_func_start_fre_off;
-  func_start_addr = fdep->sfde_func_start_address;
+  func_start_addr = sframe_decoder_get_secrel_func_start_addr (ctx, func_idx);
 
   for (i = 0; i < fdep->sfde_func_num_fres; i++)
    {
@@ -1149,7 +1175,7 @@  sframe_find_fre (sframe_decoder_ctx *ctx, int32_t pc,
      if (i == 0 && !mask_p && (start_ip_offset + func_start_addr) > pc)
        return sframe_set_errno (&err, SFRAME_ERR_FRE_INVAL);
 
-     if (sframe_fre_check_range_p (fdep, start_ip_offset, end_ip_offset, pc))
+     if (sframe_fre_check_range_p (ctx, func_idx, start_ip_offset, end_ip_offset, pc))
        {
 	 sframe_frame_row_entry_copy (frep, &cur_fre);
 	 return 0;
@@ -1643,14 +1669,26 @@  static int
 sframe_sort_funcdesc (sframe_encoder_ctx *encoder)
 {
   sframe_header *ehp;
+  size_t hdr_size;
 
   ehp = sframe_encoder_get_header (encoder);
+  hdr_size = sframe_get_hdr_size (ehp);
+
   /* Sort and write out the FDE table.  */
   sf_fde_tbl *fd_info = encoder->sfe_funcdesc;
   if (fd_info)
     {
+      for (unsigned int i = 0; i < fd_info->count; i++)
+	fd_info->entry[i].sfde_func_start_address
+	  += sframe_offsetof_fde_func_start_addr (hdr_size, i);
+
       qsort (fd_info->entry, fd_info->count,
 	     sizeof (sframe_func_desc_entry), fde_func);
+
+      for (unsigned int i = 0; i < fd_info->count; i++)
+	fd_info->entry[i].sfde_func_start_address
+	  -= sframe_offsetof_fde_func_start_addr (hdr_size, i);
+
       /* Update preamble's flags.  */
       ehp->sfh_preamble.sfp_flags |= SFRAME_F_FDE_SORTED;
     }