]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
[SFrame-V3] include: gas: libsframe: split FDE into idx and attr
authorIndu Bhagat <indu.bhagat@oracle.com>
Fri, 16 Jan 2026 00:43:23 +0000 (16:43 -0800)
committerIndu Bhagat <indu.bhagat@oracle.com>
Fri, 16 Jan 2026 01:02:27 +0000 (17:02 -0800)
This patch introduces a structural change to the SFrame V3 format. It
shifts the SFrame Function Descriptor Entry (FDE) (a physical entity in
SFrame V2) into a conceptual one in SFrame V3, such that an FDE is now
split into two distinct parts to optimize the binary search table and
data organization:
 - FDE Index (sframe_func_desc_idx_v3): This structure contains the
   essential indexing information: the function start address offset,
   function size in bytes, and the offset to the SFrame FDE
   attribute/Frame Row Entries (FREs) area for the function.
 - FDE Attributes (sframe_func_desc_attr_v3): The metadata regarding the
   function (number and size of FREs, FDE type, and repetition block
   size etc.) is moved to a new structure.

On-Disk Layout: In V3, the "Attributes" are now stored immediately
preceding the SFrame FREs for that function. The sfde_func_start_fre_off
now points to the attr structure, and the actual FREs follow immediately
after.  IOW, the "Attributes" are now moved to the FRE sub-section,
located immediately preceding the FREs for the respective function.

The above layout has the advantage that:
  a) its cleaner with separation between the index elements vs other data
  b) the index has better cache locality (by virtue of it being smaller
     than the layout in SFrame V2).
  c) As the format evolves, the guarantees of alignment for FDE index
     are easier to maintain.  FDE attr, being in the SFrame FRE
     sub-section, carry no guarantees of alignment.

This had been previously suggested and communicated in an earlier
discussion on binutils mailling list
https://inbox.sourceware.org/binutils/29b1f7b0-61ea-410c-8aca-d5dd6115e668@oracle.com/

The read/write paths in sframe.c are updated to account for this split.

sframe_fde_tbl_init now requires access to the FRE buffer to populate
the internal FDE table, as the attributes are no longer resident in the
FDE section.

flip_sframe is refactored into version-specific handlers (_v2 and _v3)
because the endian-swapping logic now differs significantly. In V3, the
iterator must jump from the FDE table to the FRE section to swap the
attributes.

Lastly, the two entities generating SFrame sections (GAS and GNU ld)
both now must _not_ set the sfde_func_start_fre_off to zero, when the
number of FREs is zero.  This is because now there will be some valid
attr data at that location.

Backward Compatibility: Due to the need to support readelf/objdump for
SFrame V2 sections, the patch explicitly maintains V2 support via
separate code paths (e.g., flip_sframe_fdes_with_fres_v2)

Note about alignment: Now that the sframe_func_desc_idx_v3 is refactored
out of the conceptual SFrame FDE, SFrame FDE index member elements are
at aligned boundaries again.  The alignment property for SFrame FDE was
broken from an ealier patch "[08/36] sframe: gas: libsframe: use
uint16_t for num_fres of FDE" up until this one.

include/
* sframe.h (sframe_func_desc_entry_v3): Remove sfde_func_num_fres,
sfde_func_info, sfde_func_info2, and sfde_func_rep_size.  Rename
to sframe_func_desc_idx_v3.
(sframe_func_desc_attr_v3): New SFrame FDE attribute structure.
libsframe/
* sframe.c (sframe_fde_tbl_init): Add argument for FRE buffer.
Read attributes from the FRE section for V3.
(flip_fde_desc): Rename from flip_fde. Check size against
sframe_func_desc_entry_v3.
(flip_fde_attr_v3): New function.
(sframe_decode_fde_desc_v2): New function extracted from
sframe_decode_fde.
(sframe_decode_fde_desc_v3): New function.
(sframe_decode_fde_attr_v3): New function.
(flip_sframe_fdes_with_fres_v2): New function for V2 flipping.
(flip_sframe_fdes_with_fres_v3): New function for V3 flipping.
(flip_sframe): Dispatch to version-specific flip functions.
(sframe_decode): Pass FRE buffer to sframe_fde_tbl_init.
(sframe_decoder_get_offsetof_fde_start_addr): Adjust for subset
of sframe_func_desc_entry_v3 restructured into
sframe_func_desc_idx_v3.
(sframe_encoder_get_offsetof_fde_start_addr): Likewise.
(sframe_find_fre): Skip attribute size to find FREs in V3.
(sframe_decoder_get_fre): Likewise.
(sframe_decoder_get_fres_buf): Likewise.
(sframe_encoder_add_fre): Add attribute size to byte count.
(sframe_encoder_add_fres_buf): Read attributes from buffer.
(sframe_encoder_write_fde): Write only FDE index fields.
(sframe_encoder_write_func_attr): New function.
(sframe_encoder_write_sframe): Write FDE attributes before FREs.
gas/
* gen-sframe.c (output_sframe_funcdesc): Do not reset
sfde_func_start_fre_off to zero when zero num FREs.
(output_sframe_func_desc_attr): New refactored out function.
(output_sframe_internal): Invoke output_sframe_func_desc_attr.
libsframe/testsuite/
* libsframe.decode/DATA2: Regenerate data file.

bfd/elf-sframe.c
gas/gen-sframe.c
include/sframe.h
libsframe/sframe.c
libsframe/testsuite/libsframe.decode/DATA2

index af6356f49fe0d15df7047813dc91b5dc94c059fa..511a297054b8f8d9260288dfb23c9ed4f22d82ba 100644 (file)
@@ -25,7 +25,7 @@
 #include "sframe-api.h"
 #include "sframe-internal.h"
 
-typedef sframe_func_desc_entry_v3 sframe_func_desc_entry;
+typedef sframe_func_desc_idx_v3 sframe_func_desc_entry;
 
 /* Return TRUE if the function has been marked for deletion during the linking
    process.  */
@@ -487,16 +487,12 @@ _bfd_elf_merge_section_sframe (bfd *abfd,
       int64_t func_start_addr;
       bfd_vma address;
       uint32_t func_size = 0;
-      unsigned char func_info = 0;
-      unsigned char func_info2 = 0;
       unsigned int r_offset = 0;
       bool pltn_reloc_by_hand = false;
       unsigned int pltn_r_offset = 0;
-      uint8_t rep_block_size = 0;
 
       if (!sframe_decoder_get_funcdesc_v3 (sfd_ctx, i, &num_fres, &func_size,
-                                          &func_start_addr, &func_info,
-                                          &func_info2, &rep_block_size))
+                                          &func_start_addr, NULL, NULL, NULL))
        {
          /* If function belongs to a deleted section, skip editing the
             function descriptor entry.  */
@@ -560,11 +556,9 @@ _bfd_elf_merge_section_sframe (bfd *abfd,
              func_start_addr = address;
            }
 
-         /* Update the encoder context with updated content.  */
-         int err = sframe_encoder_add_funcdesc_v3 (sfe_ctx, func_start_addr,
-                                                   func_size, func_info,
-                                                   func_info2, rep_block_size,
-                                                   num_fres);
+         /* Update the encoder context with FDE index entry.  */
+         int err = sframe_encoder_add_funcdesc (sfe_ctx, func_start_addr,
+                                                func_size);
          cur_fidx++;
          BFD_ASSERT (!err);
        }
index e038c522d917bfa5b64677dc89424a148ab46f52..4e1f38a40fa2ef5f8b421dee61cda363487128ed 100644 (file)
@@ -56,7 +56,7 @@
 #endif
 
 /* gas emits SFrame Version 3 only at this time.  */
-typedef sframe_func_desc_entry_v3 sframe_func_desc_entry;
+typedef sframe_func_desc_idx_v3 sframe_func_desc_idx;
 
 /* List of SFrame FDE entries.  */
 
@@ -817,14 +817,12 @@ output_sframe_row_entry (const struct sframe_func_entry *sframe_fde,
 }
 
 static void
-output_sframe_funcdesc (symbolS *start_of_fre_section,
-                       symbolS *fre_symbol,
-                       const struct sframe_func_entry *sframe_fde)
+output_sframe_funcdesc_idx (symbolS *start_of_fre_section,
+                           symbolS *fre_symbol,
+                           const struct sframe_func_entry *sframe_fde)
 {
   expressionS exp;
   symbolS *dw_fde_start_addrS, *dw_fde_end_addrS;
-  unsigned int pauth_key;
-  bool signal_p;
 
   dw_fde_start_addrS = get_dw_fde_start_addrS (sframe_fde->dw_fde);
   dw_fde_end_addrS = get_dw_fde_end_addrS (sframe_fde->dw_fde);
@@ -835,30 +833,35 @@ output_sframe_funcdesc (symbolS *start_of_fre_section,
   exp.X_add_symbol = dw_fde_start_addrS; /* to location.  */
   exp.X_op_symbol = symbol_temp_new_now (); /* from location.  */
   exp.X_add_number = 0;
-  emit_expr (&exp, sizeof_member (sframe_func_desc_entry,
-                                 sfde_func_start_offset));
+  emit_expr (&exp, sizeof_member (sframe_func_desc_idx,
+                                 sfdi_func_start_offset));
 
   /* Size of the function in bytes.  */
   exp.X_op = O_subtract;
   exp.X_add_symbol = dw_fde_end_addrS;
   exp.X_op_symbol = dw_fde_start_addrS;
   exp.X_add_number = 0;
-  emit_expr (&exp, sizeof_member (sframe_func_desc_entry,
-                                 sfde_func_size));
+  emit_expr (&exp, sizeof_member (sframe_func_desc_idx,
+                                 sfdi_func_size));
 
-  /* Offset to the first frame row entry.  */
-  if (sframe_fde->num_fres == 0)
-    /* For FDEs without any FREs, set sfde_func_start_fre_off to zero.  */
-    out_four (0);
-  else
-    {
-      exp.X_op = O_subtract;
-      exp.X_add_symbol = fre_symbol; /* Minuend.  */
-      exp.X_op_symbol = start_of_fre_section; /* Subtrahend.  */
-      exp.X_add_number = 0;
-      emit_expr (&exp, sizeof_member (sframe_func_desc_entry,
-                                     sfde_func_start_fre_off));
-    }
+  /* Offset to the function data (attribtues, FREs) in the FRE subsection.  */
+  exp.X_op = O_subtract;
+  exp.X_add_symbol = fre_symbol; /* Minuend.  */
+  exp.X_op_symbol = start_of_fre_section; /* Subtrahend.  */
+  exp.X_add_number = 0;
+  emit_expr (&exp, sizeof_member (sframe_func_desc_idx,
+                                 sfdi_func_start_fre_off));
+}
+
+static void
+output_sframe_func_desc_attr (const struct sframe_func_entry *sframe_fde)
+{
+  symbolS *dw_fde_start_addrS, *dw_fde_end_addrS;
+  unsigned int pauth_key;
+  bool signal_p;
+
+  dw_fde_start_addrS = get_dw_fde_start_addrS (sframe_fde->dw_fde);
+  dw_fde_end_addrS = get_dw_fde_end_addrS (sframe_fde->dw_fde);
 
   /* Number of FREs must fit uint16_t.  */
   gas_assert (sframe_fde->num_fres <= UINT16_MAX);
@@ -994,8 +997,8 @@ output_sframe_internal (void)
   i = 0;
   for (sframe_fde = all_sframe_fdes; sframe_fde; sframe_fde = sframe_fde->next)
     {
-      output_sframe_funcdesc (start_of_fre_section,
-                             fde_fre_symbols[i], sframe_fde);
+      output_sframe_funcdesc_idx (start_of_fre_section, fde_fre_symbols[i],
+                                 sframe_fde);
       i++;
     }
 
@@ -1008,6 +1011,9 @@ output_sframe_internal (void)
   for (sframe_fde = all_sframe_fdes; sframe_fde; sframe_fde = sframe_fde_next)
     {
       symbol_set_value_now (fde_fre_symbols[i]);
+
+      output_sframe_func_desc_attr (sframe_fde);
+
       for (sframe_fre = sframe_fde->sframe_fres;
           sframe_fre;
           sframe_fre = sframe_fre->next)
index e79892e6b49142f3bf9e7bcdb18010edefb10188..f1409d3626360f50b4e969a13ea23ebf9575c3aa 100644 (file)
@@ -277,18 +277,22 @@ typedef struct sframe_func_desc_entry_v2
    matching FRE.  */
 #define SFRAME_V3_FDE_PCTYPE_MASK  SFRAME_FDE_TYPE_PCMASK
 
-typedef struct sframe_func_desc_entry_v3
+typedef struct sframe_func_desc_idx_v3
 {
   /* Offset to the function start address.  Encoded as a signed offset,
      relative to the beginning of the current FDE.  */
-  int64_t sfde_func_start_offset;
+  int64_t sfdi_func_start_offset;
   /* Size of the function in bytes.  */
-  uint32_t sfde_func_size;
+  uint32_t sfdi_func_size;
   /* Offset of the first SFrame Frame Row Entry of the function, relative to the
      beginning of the SFrame Frame Row Entry sub-section.  */
-  uint32_t sfde_func_start_fre_off;
+  uint32_t sfdi_func_start_fre_off;
+} ATTRIBUTE_PACKED sframe_func_desc_idx_v3;
+
+typedef struct sframe_func_desc_attr_v3
+{
   /* Number of frame row entries for the function.  */
-  uint16_t sfde_func_num_fres;
+  uint16_t sfda_func_num_fres;
   /* Additional information for stack tracing from the function:
      - 4-bits: Identify the FRE type used for the function.
      - 1-bit: Identify the PC type of the function - mask or inc.
@@ -300,7 +304,7 @@ typedef struct sframe_func_desc_entry_v3
      | frame  |             |        Unused (amd64)       | PC Type |              |
      -------------------------------------------------------------------------------
      8        7             6                             5         4              0     */
-  uint8_t sfde_func_info;
+  uint8_t sfda_func_info;
   /* Additional information for stack tracing from the function:
      - 5-bits: FDE type.
      - 3-bits: Unused.
@@ -309,11 +313,11 @@ typedef struct sframe_func_desc_entry_v3
      |                                            |             |
      ------------------------------------------------------------
      8                7             6             5             0     */
-  uint8_t sfde_func_info2;
+  uint8_t sfda_func_info2;
   /* Size of the block of repeating insns.  Used for SFrame FDEs of type
      SFRAME_V3_FDE_PCTYPE_MASK.  */
-  uint8_t sfde_func_rep_size;
-} ATTRIBUTE_PACKED sframe_func_desc_entry_v3;
+  uint8_t sfda_func_rep_size;
+} ATTRIBUTE_PACKED sframe_func_desc_attr_v3;
 
 #define SFRAME_V3_FDE_FUNC_INFO(fde_pc_type, fre_type) \
   (SFRAME_V2_FUNC_INFO (fde_pc_type, fre_type))
index a32144d221bd19046fd5bbe63789490d359ae632..52788d8ac152a32894bd46d718304fce51d2fd4c 100644 (file)
@@ -140,22 +140,28 @@ sframe_fde_tbl_alloc (sf_fde_tbl **fde_tbl, unsigned int num_fdes)
 
 static int
 sframe_fde_tbl_init (sf_fde_tbl *fde_tbl, const char *fde_buf,
-                    size_t *fidx_size, unsigned int num_fdes, uint8_t ver)
+                    const char *fre_buf, size_t *fidx_size,
+                    unsigned int num_fdes, uint8_t ver)
 {
   if (ver == SFRAME_VERSION_3 && SFRAME_VERSION == SFRAME_VERSION_3)
     {
-      *fidx_size = num_fdes * sizeof (sframe_func_desc_entry_v3);
+      *fidx_size = num_fdes * sizeof (sframe_func_desc_idx_v3);
       for (unsigned int i = 0; i < num_fdes; i++)
        {
-         const sframe_func_desc_entry_v3 *fdep
-           = (sframe_func_desc_entry_v3 *)fde_buf + i;
-         fde_tbl->entry[i].func_start_pc_offset = fdep->sfde_func_start_offset;
-         fde_tbl->entry[i].func_size = fdep->sfde_func_size;
-         fde_tbl->entry[i].func_start_fre_off = fdep->sfde_func_start_fre_off;
-         fde_tbl->entry[i].func_num_fres = fdep->sfde_func_num_fres;
-         fde_tbl->entry[i].func_info = fdep->sfde_func_info;
-         fde_tbl->entry[i].func_info2 = fdep->sfde_func_info2;
-         fde_tbl->entry[i].func_rep_size = fdep->sfde_func_rep_size;
+         const sframe_func_desc_idx_v3 *fdep
+           = (sframe_func_desc_idx_v3 *)fde_buf + i;
+         fde_tbl->entry[i].func_start_pc_offset = fdep->sfdi_func_start_offset;
+         fde_tbl->entry[i].func_size = fdep->sfdi_func_size;
+         fde_tbl->entry[i].func_start_fre_off = fdep->sfdi_func_start_fre_off;
+         /* V3 organizes the following data closer to the SFrame FREs for the
+            function.  Access them via the sfde_func_start_fre_off.  */
+         const sframe_func_desc_attr_v3 *fattr
+           = (sframe_func_desc_attr_v3 *)(fre_buf
+                                          + fdep->sfdi_func_start_fre_off);
+         fde_tbl->entry[i].func_num_fres = fattr->sfda_func_num_fres;
+         fde_tbl->entry[i].func_info = fattr->sfda_func_info;
+         fde_tbl->entry[i].func_info2 = fattr->sfda_func_info2;
+         fde_tbl->entry[i].func_rep_size = fattr->sfda_func_rep_size;
        }
       fde_tbl->count = num_fdes;
     }
@@ -308,17 +314,17 @@ flip_header (char *buf, uint8_t ver ATTRIBUTE_UNUSED)
    should not be used.  */
 
 static int
-flip_fde (char *buf, size_t buf_size, uint8_t ver, size_t *fde_size)
+flip_fde_desc (char *buf, size_t buf_size, uint8_t ver)
 {
   if (ver == SFRAME_VERSION_3)
     {
-      sframe_func_desc_entry_v3 *fdep = (sframe_func_desc_entry_v3 *) buf;
-      swap_thing (fdep->sfde_func_start_offset);
-      swap_thing (fdep->sfde_func_size);
-      swap_thing (fdep->sfde_func_start_fre_off);
-      swap_thing (fdep->sfde_func_num_fres);
+      if (buf_size < sizeof (sframe_func_desc_idx_v3))
+       return SFRAME_ERR;
 
-      *fde_size = sizeof (sframe_func_desc_entry_v3);
+      sframe_func_desc_idx_v3 *fdep = (sframe_func_desc_idx_v3 *) buf;
+      swap_thing (fdep->sfdi_func_start_offset);
+      swap_thing (fdep->sfdi_func_size);
+      swap_thing (fdep->sfdi_func_start_fre_off);
     }
   else if (ver == SFRAME_VERSION_2)
     {
@@ -330,15 +336,25 @@ flip_fde (char *buf, size_t buf_size, uint8_t ver, size_t *fde_size)
       swap_thing (fdep->sfde_func_size);
       swap_thing (fdep->sfde_func_start_fre_off);
       swap_thing (fdep->sfde_func_num_fres);
-
-      *fde_size = sizeof (sframe_func_desc_entry_v2);
     }
   else
-    return SFRAME_ERR; /* No other versions are possible ATM.  */
+    return SFRAME_ERR;
 
   return 0;
 }
 
+static int
+flip_fde_attr_v3 (char *buf, size_t buf_size)
+{
+  if (buf_size < sizeof (sframe_func_desc_attr_v3))
+    return SFRAME_ERR;
+
+  /* sfda_func_num_fres is the first member of sframe_func_desc_attr_v3.  */
+  struct { uint16_t x; } ATTRIBUTE_PACKED *p = (void*)buf;
+  swap_thing (p->x);
+
+  return 0;
+}
 /* Check if SFrame header has valid data.  */
 
 static bool
@@ -588,47 +604,60 @@ sframe_fre_check_range_p (const sframe_decoder_ctx *dctx, uint32_t func_idx,
   return (start_ip_offset <= pc_offset) && (end_ip_offset >= pc_offset);
 }
 
-/* Read the on-disk SFrame FDE of SFrame version VER from location BUF of size
-   in bytes equal to BUF_SIZE.
+/* Read the on-disk SFrame FDE from location BUF of size in bytes equal to
+   BUF_SIZE.
 
    Return SFRAME_ERR if any error.  If error code is returned, the read values
    should not be used.  */
 
 static int
-sframe_decode_fde (const char *buf, size_t buf_size, uint8_t ver,
-                  uint32_t *num_fres, uint32_t *fre_type,
-                  uint32_t *fre_offset, size_t *fde_size)
+sframe_decode_fde_desc_v2 (const char *buf, size_t buf_size,
+                          uint32_t *num_fres, uint32_t *fre_type,
+                          uint32_t *fre_offset)
 {
-  if (ver == SFRAME_VERSION_3)
-    {
-      if (buf_size < sizeof (sframe_func_desc_entry_v3))
-       return SFRAME_ERR;
+  if (buf_size < sizeof (sframe_func_desc_entry_v2))
+    return SFRAME_ERR;
 
-      sframe_func_desc_entry_v3 *fdep = (sframe_func_desc_entry_v3 *) buf;
-      *num_fres = fdep->sfde_func_num_fres;
-      *fre_type = SFRAME_V3_FDE_FRE_TYPE (fdep->sfde_func_info);
-      *fre_offset = fdep->sfde_func_start_fre_off;
+  sframe_func_desc_entry_v2 *fdep = (sframe_func_desc_entry_v2 *) buf;
+  *num_fres = fdep->sfde_func_num_fres;
+  *fre_type = SFRAME_V2_FUNC_FRE_TYPE (fdep->sfde_func_info);
+  *fre_offset = fdep->sfde_func_start_fre_off;
 
-      *fde_size = sizeof (sframe_func_desc_entry_v3);
-    }
-  else if (ver == SFRAME_VERSION_2)
-    {
-      if (buf_size < sizeof (sframe_func_desc_entry_v2))
-       return SFRAME_ERR;
+  return 0;
+}
 
-      sframe_func_desc_entry_v2 *fdep = (sframe_func_desc_entry_v2 *) buf;
-      *num_fres = fdep->sfde_func_num_fres;
-      *fre_type = SFRAME_V1_FUNC_FRE_TYPE (fdep->sfde_func_info);
-      *fre_offset = fdep->sfde_func_start_fre_off;
+/* Read the on-disk SFrame FDE from location BUF of size in bytes equal to
+   BUF_SIZE.
 
-      *fde_size = sizeof (sframe_func_desc_entry_v2);
-    }
-  else
+   Return SFRAME_ERR if any error.  If error code is returned, the read values
+   should not be used.  */
+
+static int
+sframe_decode_fde_idx_v3 (const char *buf, size_t buf_size,
+                         uint32_t *fre_offset)
+{
+  if (buf_size < sizeof (sframe_func_desc_idx_v3))
     return SFRAME_ERR;
 
+  sframe_func_desc_idx_v3 *fdep = (sframe_func_desc_idx_v3 *) buf;
+  *fre_offset = fdep->sfdi_func_start_fre_off;
+
   return 0;
 }
 
+static int
+sframe_decode_fde_attr_v3 (const char *buf, size_t buf_size,
+                          uint16_t *num_fres)
+{
+  if (buf_size < sizeof (sframe_func_desc_attr_v3))
+    return SFRAME_ERR;
+
+  /* sfda_func_num_fres is the first member of sframe_func_desc_attr_v3.  */
+  const struct { uint16_t x; } ATTRIBUTE_PACKED *p = (void *)buf;
+  *num_fres = p->x;
+
+  return 0;
+}
 static int
 flip_fre (char *fp, uint32_t fre_type, size_t *fre_size)
 {
@@ -674,64 +703,172 @@ flip_fre (char *fp, uint32_t fre_type, size_t *fre_size)
    If an error code is returned, the buffer should not be used.  */
 
 static int
-flip_sframe (char *frame_buf, size_t buf_size, uint32_t to_foreign)
+flip_sframe_fdes_with_fres_v2 (char *frame_buf, size_t buf_size,
+                              uint32_t to_foreign)
 {
-  unsigned int i, j, prev_frep_index;
-  const sframe_header *ihp;
-  uint8_t ver;
-  char *fdes;
-  char *fres;
-  const char *buf_end;
   char *fp = NULL;
-  unsigned int num_fdes = 0;
   uint32_t num_fres = 0;
   uint32_t fre_type = 0;
   uint32_t fre_offset = 0;
-  size_t esz = 0, fsz = 0;
-  size_t hdrsz = 0;
+  size_t esz = 0;
   int err = 0;
   /* For error checking.  */
   size_t fde_bytes_flipped = 0;
   size_t fre_bytes_flipped = 0;
 
   /* Header must be in host endianness at this time.  */
-  ihp = (sframe_header *)frame_buf;
+  const sframe_header *ihp = (sframe_header *)frame_buf;
 
   if (!sframe_header_sanity_check_p (ihp))
     return sframe_set_errno (&err, SFRAME_ERR_BUF_INVAL);
 
   /* The contents of the SFrame header are safe to read.  Get the number of
      FDEs and the first FDE in the buffer.  */
-  hdrsz = sframe_get_hdr_size (ihp);
-  num_fdes = ihp->sfh_num_fdes;
-  ver = ihp->sfh_preamble.sfp_version;
-  fdes = frame_buf + hdrsz + ihp->sfh_fdeoff;
-  fres = frame_buf + hdrsz + ihp->sfh_freoff;
-  buf_end = frame_buf + buf_size;
-
-  j = 0;
-  prev_frep_index = 0;
+  size_t hdrsz = sframe_get_hdr_size (ihp);
+  uint32_t num_fdes = ihp->sfh_num_fdes;
+  uint8_t ver = ihp->sfh_preamble.sfp_version;
+  char *fdes = frame_buf + hdrsz + ihp->sfh_fdeoff;
+  char *fres = frame_buf + hdrsz + ihp->sfh_freoff;
+  const char *buf_end = frame_buf + buf_size;
+
+  unsigned int i = 0, j = 0;
+  unsigned int prev_frep_index = 0;
+  size_t fsz = sizeof (sframe_func_desc_entry_v2);
   for (i = 0; i < num_fdes; fdes += fsz, i++)
     {
       if (fdes >= buf_end)
        goto bad;
 
-      if (to_foreign && sframe_decode_fde (fdes, buf_end - fdes, ver,
-                                          &num_fres, &fre_type, &fre_offset,
-                                          &fsz))
+      /* Handle FDE.  */
+      if (to_foreign && sframe_decode_fde_desc_v2 (fdes, buf_end - fdes,
+                                                  &num_fres, &fre_type,
+                                                  &fre_offset))
        goto bad;
 
-      if (flip_fde (fdes, buf_end - fdes, ver, &fsz))
+      if (flip_fde_desc (fdes, buf_end - fdes, ver))
+       goto bad;
+
+      if (!to_foreign && sframe_decode_fde_desc_v2 (fdes, buf_end - fdes,
+                                                   &num_fres, &fre_type,
+                                                   &fre_offset))
        goto bad;
 
       fde_bytes_flipped += fsz;
 
-      if (!to_foreign && sframe_decode_fde (fdes, buf_end - fdes, ver,
-                                           &num_fres, &fre_type, &fre_offset,
-                                           &fsz))
+      /* Handle FREs.  */
+      fp = fres + fre_offset;
+      for (; j < prev_frep_index + num_fres; j++)
+       {
+         if (flip_fre (fp, fre_type, &esz))
+           goto bad;
+         fre_bytes_flipped += esz;
+
+         if (esz == 0 || esz > buf_size)
+           goto bad;
+         fp += esz;
+       }
+      prev_frep_index = j;
+    }
+
+  /* All FDEs must have been endian flipped by now.  */
+  if (i != num_fdes || fde_bytes_flipped > ihp->sfh_freoff - ihp->sfh_fdeoff)
+    goto bad;
+
+  /* All FREs must have been endian flipped by now.  */
+  if (j != ihp->sfh_num_fres || fre_bytes_flipped > ihp->sfh_fre_len)
+    goto bad;
+
+  /* Optional trailing section padding.  */
+  size_t frame_size = hdrsz + ihp->sfh_freoff + fre_bytes_flipped;
+  for (fp = frame_buf + frame_size; fp < frame_buf + buf_size; fp++)
+    if (*fp != '\0')
+      goto bad;
+
+  /* Done.  */
+  return 0;
+bad:
+  return SFRAME_ERR;
+}
+
+/* Endian flip the contents of FRAME_BUF of size BUF_SIZE.
+   The SFrame header in the FRAME_BUF must be endian flipped prior to
+   calling flip_sframe_fdes_with_fres_v3.
+
+   Endian flipping at decode time vs encode time have different needs.  At
+   encode time, the frame_buf is in host endianness, and hence, values should
+   be read up before the buffer is changed to foreign endianness.  This change
+   of behaviour is specified via TO_FOREIGN arg.
+
+   If an error code is returned, the buffer should not be used.  */
+
+static int
+flip_sframe_fdes_with_fres_v3 (char *frame_buf, size_t buf_size,
+                              uint32_t to_foreign)
+{
+  char *fp = NULL;
+  uint16_t num_fres = 0;
+  uint32_t fre_type = 0;
+  uint32_t fre_offset = 0;
+  size_t esz = 0;
+  int err = 0;
+  /* For error checking.  */
+  size_t fde_bytes_flipped = 0;
+  size_t fre_bytes_flipped = 0;
+
+  /* Header must be in host endianness at this time.  */
+  const sframe_header *ihp = (sframe_header *)frame_buf;
+
+  if (!sframe_header_sanity_check_p (ihp))
+    return sframe_set_errno (&err, SFRAME_ERR_BUF_INVAL);
+
+  /* The contents of the SFrame header are safe to read.  Get the number of
+     FDEs and the first FDE in the buffer.  */
+  size_t hdrsz = sframe_get_hdr_size (ihp);
+  uint32_t num_fdes = ihp->sfh_num_fdes;
+  uint8_t ver = ihp->sfh_preamble.sfp_version;
+  char *fdes = frame_buf + hdrsz + ihp->sfh_fdeoff;
+  char *fres = frame_buf + hdrsz + ihp->sfh_freoff;
+  const char *buf_end = frame_buf + buf_size;
+
+  unsigned int i = 0, j = 0;
+  unsigned int prev_frep_index = 0;
+  size_t fsz = sizeof (sframe_func_desc_idx_v3);
+  for (i = 0; i < num_fdes; fdes += fsz, i++)
+    {
+      if (fdes >= buf_end)
+       goto bad;
+
+      /* Handle FDE.  */
+      if (to_foreign && sframe_decode_fde_idx_v3 (fdes, buf_end - fdes,
+                                                 &fre_offset))
        goto bad;
 
+      if (flip_fde_desc (fdes, buf_end - fdes, ver))
+       goto bad;
+
+      if (!to_foreign && sframe_decode_fde_idx_v3 (fdes, buf_end - fdes,
+                                                  &fre_offset))
+       goto bad;
+
+      fde_bytes_flipped += fsz;
+
+      /* Handle FDE attr (only in V3).  */
       fp = fres + fre_offset;
+      if (to_foreign && sframe_decode_fde_attr_v3 (fp, buf_end - fp,
+                                                  &num_fres))
+       goto bad;
+
+      if (flip_fde_attr_v3 (fp, buf_end - fp))
+       goto bad;
+
+      fre_bytes_flipped += sizeof (sframe_func_desc_attr_v3);
+
+      if (!to_foreign && sframe_decode_fde_attr_v3 (fp, buf_end - fp,
+                                                   &num_fres))
+       goto bad;
+
+      /* Handle FREs.  */
+      fp += sizeof (sframe_func_desc_attr_v3);
       for (; j < prev_frep_index + num_fres; j++)
        {
          if (flip_fre (fp, fre_type, &esz))
@@ -759,12 +896,37 @@ flip_sframe (char *frame_buf, size_t buf_size, uint32_t to_foreign)
     if (*fp != '\0')
       goto bad;
 
-  /* Success.  */
+  /* Done.  */
   return 0;
 bad:
   return SFRAME_ERR;
 }
 
+static int
+flip_sframe (char *frame_buf, size_t buf_size, uint32_t to_foreign)
+{
+  int err = 0;
+
+  /* Header must be in host endianness at this time.  */
+  const sframe_header *ihp = (sframe_header *)frame_buf;
+  if (!sframe_header_sanity_check_p (ihp))
+    return sframe_set_errno (&err, SFRAME_ERR_BUF_INVAL);
+  uint8_t ver = ihp->sfh_preamble.sfp_version;
+
+  if (ver == SFRAME_VERSION_3)
+    err = flip_sframe_fdes_with_fres_v3 (frame_buf, buf_size, to_foreign);
+  else if (ver == SFRAME_VERSION_2)
+    err = flip_sframe_fdes_with_fres_v2 (frame_buf, buf_size, to_foreign);
+  else
+    return sframe_set_errno (&err, SFRAME_ERR_BUF_INVAL);
+
+  if (err)
+    return sframe_set_errno (&err, SFRAME_ERR_BUF_INVAL);
+
+  /* Success.  */
+  return 0;
+}
+
 /* Expands the memory allocated for the SFrame Frame Row Entry (FRE) table
    FRE_TBL.  This function is called when the current FRE buffer is
    insufficient and more stack trace data in the form of COUNT number of SFrame
@@ -1325,6 +1487,7 @@ sframe_decode (const char *sf_buf, size_t sf_size, int *errp)
 
   /* SFrame FDEs are at an offset of sfh_fdeoff from SFrame header end.  */
   if (sframe_fde_tbl_init (dctx->sfd_funcdesc, frame_buf + dhp->sfh_fdeoff,
+                          frame_buf + dhp->sfh_freoff,
                           &fidx_size, dhp->sfh_num_fdes, sfp->sfp_version))
     {
       sframe_ret_set_errno (errp, SFRAME_ERR_BUF_INVAL);
@@ -1430,8 +1593,8 @@ sframe_decoder_get_offsetof_fde_start_addr (const sframe_decoder_ctx *dctx,
 
   if (sframe_decoder_get_version (dctx) == SFRAME_VERSION_3)
     return (sframe_decoder_get_hdr_size (dctx)
-           + func_idx * sizeof (sframe_func_desc_entry_v3)
-           + offsetof (sframe_func_desc_entry_v3, sfde_func_start_offset));
+           + func_idx * sizeof (sframe_func_desc_idx_v3)
+           + offsetof (sframe_func_desc_idx_v3, sfdi_func_start_offset));
   else if (sframe_decoder_get_version (dctx) == SFRAME_VERSION_2)
     return (sframe_decoder_get_hdr_size (dctx)
            + func_idx * sizeof (sframe_func_desc_entry_v2)
@@ -1539,6 +1702,7 @@ sframe_find_fre (const sframe_decoder_ctx *ctx, int64_t pc,
   if ((ctx == NULL) || (frep == NULL))
     return sframe_set_errno (&err, SFRAME_ERR_INVAL);
 
+  uint8_t ver = sframe_decoder_get_version (ctx);
   /* Find the FDE which contains the PC, then scan its fre entries.  */
   fdep = sframe_get_funcdesc_with_addr_internal (ctx, pc, &err, &func_idx);
   if (fdep == NULL || ctx->sfd_fres == NULL)
@@ -1547,6 +1711,8 @@ sframe_find_fre (const sframe_decoder_ctx *ctx, int64_t pc,
   fre_type = sframe_get_fre_type (fdep);
 
   fres = ctx->sfd_fres + fdep->func_start_fre_off;
+  if (ver == SFRAME_VERSION_3)
+    fres += sizeof (sframe_func_desc_attr_v3);
   func_start_pc_offset = sframe_decoder_get_secrel_func_start_addr (ctx,
                                                                    func_idx);
 
@@ -1679,15 +1845,19 @@ sframe_decoder_get_fre (const sframe_decoder_ctx *ctx,
   if (ctx == NULL || fre == NULL)
     return sframe_set_errno (&err, SFRAME_ERR_INVAL);
 
+  uint8_t ver = sframe_decoder_get_version (ctx);
+
   /* Get function descriptor entry at index func_idx.  */
   fdep = sframe_decoder_get_funcdesc_at_index (ctx, func_idx);
-
   if (fdep == NULL)
     return sframe_set_errno (&err, SFRAME_ERR_FDE_NOTFOUND);
 
   fre_type = sframe_get_fre_type (fdep);
   /* Now scan the FRE entries.  */
   fres = ctx->sfd_fres + fdep->func_start_fre_off;
+  if (ver == SFRAME_VERSION_3)
+    fres += sizeof (sframe_func_desc_attr_v3);
+
   for (i = 0; i < fdep->func_num_fres; i++)
    {
      /* Decode the FRE at the current position.  Return it if valid.  */
@@ -1748,7 +1918,8 @@ sframe_decoder_get_fres_buf (const sframe_decoder_ctx *dctx,
   fre_type = sframe_get_fre_type (fdep);
   /* Update the pointer to (and total size of) the FRE entries.  */
   *fres_buf = dctx->sfd_fres + fdep->func_start_fre_off;
-  const char *tmp_buf = *fres_buf;
+  const char *tmp_buf = *fres_buf + sizeof (sframe_func_desc_attr_v3);
+  *fres_buf_size = sizeof (sframe_func_desc_attr_v3);
   while (i < *num_fres)
     {
       /* Avoid cost of full decoding at this time.  */
@@ -1929,8 +2100,8 @@ sframe_encoder_get_offsetof_fde_start_addr (sframe_encoder_ctx *ectx,
     *errp = 0;
 
   return (sframe_encoder_get_hdr_size (ectx)
-         + func_idx * sizeof (sframe_func_desc_entry_v3)
-         + offsetof (sframe_func_desc_entry_v3, sfde_func_start_offset));
+         + func_idx * sizeof (sframe_func_desc_idx_v3)
+         + offsetof (sframe_func_desc_idx_v3, sfdi_func_start_offset));
 }
 
 /* Add an SFrame FRE to function at FUNC_IDX'th function descriptor entry in
@@ -1995,6 +2166,9 @@ sframe_encoder_add_fre (sframe_encoder_ctx *ectx,
   ectx->sfe_fres = fre_tbl;
   ectx->sfe_fre_nbytes += esz;
 
+  if (!fdep->func_num_fres)
+    ectx->sfe_fre_nbytes += sizeof (sframe_func_desc_attr_v3);
+
   ehp = sframe_encoder_get_header (ectx);
   ehp->sfh_num_fres = fre_tbl->count;
 
@@ -2047,14 +2221,24 @@ sframe_encoder_add_fres_buf (sframe_encoder_ctx *ectx,
       fre_tbl = tmp;
     }
 
+  /* Update the SFrame func attr values.  */
+  fdep->func_num_fres = num_fres;
+  const char *fres = fres_buf + sizeof (uint16_t);
+  fdep->func_info = *(uint8_t *)fres;
+  fres += sizeof (uint8_t);
+  fdep->func_info2 = *(uint8_t *)fres;
+  fres += sizeof (uint8_t);
+  fdep->func_rep_size = *(uint8_t *)fres;
+  fres += sizeof (uint8_t);
+
   uint32_t fre_type = sframe_get_fre_type (fdep);
   uint32_t remaining = num_fres;
-  size_t buf_size = 0;
+  size_t buf_size = sizeof (sframe_func_desc_attr_v3);
   while (remaining)
     {
       ectx_frep = &fre_tbl->entry[fre_tbl->count];
       /* Copy the SFrame FRE data over to the encoder object's fre_tbl.  */
-      sframe_decode_fre (fres_buf, ectx_frep, fre_type, &esz);
+      sframe_decode_fre (fres, ectx_frep, fre_type, &esz);
 
       if (!sframe_fre_sanity_check_p (ectx_frep))
        return sframe_set_errno (&err, SFRAME_ERR_FRE_INVAL);
@@ -2066,23 +2250,19 @@ sframe_encoder_add_fres_buf (sframe_encoder_ctx *ectx,
         libsframe/33131.  */
       sframe_assert (ectx_frep->fre_start_addr <= fdep->func_size);
 
-      ectx->sfe_fre_nbytes += esz;
-
       fre_tbl->count++;
-      fres_buf += esz;
+      fres += esz;
       buf_size += esz;
       remaining--;
     }
 
   sframe_assert (fres_buf_size == buf_size);
   ectx->sfe_fres = fre_tbl;
+  ectx->sfe_fre_nbytes += buf_size;
 
   sframe_header *ehp = sframe_encoder_get_header (ectx);
   ehp->sfh_num_fres = fre_tbl->count;
 
-  /* Update the value of the number of FREs for the function.  */
-  fdep->func_num_fres = num_fres;
-
   return 0;
 
 bad:
@@ -2356,26 +2536,46 @@ sframe_encoder_write_fre (char *contents, sframe_frame_row_entry *frep,
   return 0;
 }
 
+/* Write an SFrame FDE Index element for SFrame V3 into the provided buffer at
+   CONTENTS.  Update FDE_WRITE_SIZE with the number of bytes written to the
+   buffer.  Return 0 on success, SFRAME_ERR otherwise.  */
+
 static int
-sframe_encoder_write_fde (const sframe_header *sfhp ATTRIBUTE_UNUSED,
-                         char *contents, sframe_func_desc_entry_int *fde,
-                         size_t *fde_write_size)
+sframe_encoder_write_fde_idx (const sframe_header *sfhp ATTRIBUTE_UNUSED,
+                             char *contents,
+                             const sframe_func_desc_entry_int *fde,
+                             size_t *fde_write_size)
 {
-  sframe_func_desc_entry_v3 *fdep = (sframe_func_desc_entry_v3 *)contents;
+  sframe_func_desc_idx_v3 *fdep = (sframe_func_desc_idx_v3 *)contents;
 
-  fdep->sfde_func_start_offset = fde->func_start_pc_offset;
-  fdep->sfde_func_size = fde->func_size;
-  fdep->sfde_func_start_fre_off = fde->func_start_fre_off;
-  fdep->sfde_func_num_fres = (uint16_t)fde->func_num_fres;
-  fdep->sfde_func_info = fde->func_info;
-  fdep->sfde_func_info2 = fde->func_info2;
-  fdep->sfde_func_rep_size = fde->func_rep_size;
+  fdep->sfdi_func_start_offset = fde->func_start_pc_offset;
+  fdep->sfdi_func_size = fde->func_size;
+  fdep->sfdi_func_start_fre_off = fde->func_start_fre_off;
 
-  *fde_write_size = sizeof (sframe_func_desc_entry_v3);
+  *fde_write_size = sizeof (sframe_func_desc_idx_v3);
 
   return 0;
 }
 
+/* Write the SFrame FDE Attribute element for SFrame V3 into the provided
+   buffer at CONTENTS.  Return 0 on success, SFRAME_ERR otherwise.  */
+
+static int
+sframe_encoder_write_fde_attr (char *contents,
+                              const sframe_func_desc_entry_int *fde)
+{
+  sframe_func_desc_attr_v3 *fattr = (sframe_func_desc_attr_v3 *)contents;
+
+  /* Access to num_fres may be unaligned.  */
+  uint16_t num_fres = (uint16_t)fde->func_num_fres;
+  memcpy (fattr, &num_fres, sizeof (uint16_t));
+
+  fattr->sfda_func_info = fde->func_info;
+  fattr->sfda_func_info2 = fde->func_info2;
+  fattr->sfda_func_rep_size = fde->func_rep_size;
+
+  return 0;
+}
 /* Serialize the core contents of the SFrame section and write out to the
    output buffer held in the encoder context ECTX.  Sort the SFrame FDEs on
    start PC if SORT_FDE_P is true.  Return SFRAME_ERR if failure.  */
@@ -2403,7 +2603,7 @@ sframe_encoder_write_sframe (sframe_encoder_ctx *ectx, bool sort_fde_p)
   contents = ectx->sfe_data;
   buf_size = ectx->sfe_data_size;
   num_fdes = sframe_encoder_get_num_fidx (ectx);
-  all_fdes_size = num_fdes * sizeof (sframe_func_desc_entry_v3);
+  all_fdes_size = num_fdes * sizeof (sframe_func_desc_idx_v3);
   ehp = sframe_encoder_get_header (ectx);
   hdr_size = sframe_get_hdr_size (ehp);
 
@@ -2437,13 +2637,13 @@ sframe_encoder_write_sframe (sframe_encoder_ctx *ectx, bool sort_fde_p)
       fre_type = sframe_get_fre_type (fdep);
       num_fres = fdep->func_num_fres;
 
-      /* For FDEs without any FREs, set sfde_func_start_fre_off to zero.  */
-      if (num_fres == 0)
-       fdep->func_start_fre_off = 0;
-
       if (num_fres > 0 && fr_info == NULL)
        return sframe_set_errno (&err, SFRAME_ERR_FRE_INVAL);
 
+      sframe_encoder_write_fde_attr (contents, fdep);
+      contents += sizeof (sframe_func_desc_attr_v3);
+      fre_size += sizeof (sframe_func_desc_attr_v3);
+
       for (j = 0; j < num_fres; j++)
        {
          fre_index = global + j;
@@ -2480,8 +2680,8 @@ sframe_encoder_write_sframe (sframe_encoder_ctx *ectx, bool sort_fde_p)
   /* Write out the FDE table sorted on funtion start address.  */
   for (i = 0; i < num_fdes; i++)
     {
-      sframe_encoder_write_fde (ehp, contents, &fd_info->entry[i],
-                               &fde_write_size);
+      sframe_encoder_write_fde_idx (ehp, contents, &fd_info->entry[i],
+                                   &fde_write_size);
       contents += fde_write_size;
     }
 
@@ -2510,7 +2710,7 @@ sframe_encoder_write (sframe_encoder_ctx *ectx, size_t *encoded_size,
 
   ehp = sframe_encoder_get_header (ectx);
   hdrsize = sframe_get_hdr_size (ehp);
-  fsz = sframe_encoder_get_num_fidx (ectx) * sizeof (sframe_func_desc_entry_v3);
+  fsz = sframe_encoder_get_num_fidx (ectx) * sizeof (sframe_func_desc_idx_v3);
   fresz = ectx->sfe_fre_nbytes;
 
   /* Encoder writes out data in the latest SFrame format version.  */
index e297dad35b2402da62d8ed013f548769857ab4ea..483fd43c4840a88f9195686cdea48aa572846ffa 100644 (file)
Binary files a/libsframe/testsuite/libsframe.decode/DATA2 and b/libsframe/testsuite/libsframe.decode/DATA2 differ