[PATCH 3/5] libsframe: make flip_fde version aware

[Jens Remus]

Hello Indu!

On 10/17/2025 9:05 AM, Indu Bhagat via Binutils wrote:
> Future versions of the format may have a different representation of an
> SFrame FDE.  As the format evolves, endian flipping will need to be version
> aware.
> 
> Refactor flip_fde a bit by carving out an internal sframe_decode_fde API
> which can read information from an on-disk SFrame FDE.
> 
> libsframe/
>         * sframe.c (flip_fde): Make version aware.
>         (sframe_decode_fde): New internal definition.
>         (flip_sframe): Use the new definitions.

> diff --git a/libsframe/sframe.c b/libsframe/sframe.c

> @@ -198,13 +198,30 @@ flip_header (sframe_header *sfheader)
>    swap_thing (sfheader->sfh_freoff);
>  }
>  
> -static void
> -flip_fde (sframe_func_desc_entry *fdep)
> +/* Endian flip the SFrame FDE at BUF, given the SFrame version VER.  Update
> +   the FDE_SIZE to the size of the SFrame FDE flipped.
> +
> +   Return SFRAME_ERR if any error.  If error code is returned, the flipped FDEP
> +   should not be used.  */
> +
> +static int
> +flip_fde (char *buf, uint8_t ver, size_t *fde_size)

This is not strictly related to this patch, but I was astonished that
that there is no checking whether buf still has *fde_size bytes left.
Maybe pass the remaining buffer size into this function and have it
check?

flip_fde (char *buf, size_t size, uint8_t ver, size_t *fde_size)

>  {
> -  swap_thing (fdep->sfde_func_start_address);
> -  swap_thing (fdep->sfde_func_size);
> -  swap_thing (fdep->sfde_func_start_fre_off);
> -  swap_thing (fdep->sfde_func_num_fres);
> +
> +  if (ver == SFRAME_VERSION_2)
> +    {
> +      sframe_func_desc_entry_v2 *fdep = (sframe_func_desc_entry_v2 *) buf;

      if (size < sizeof (sframe_func_desc_entry_v2))
        return SFRAME_ERR;

Or SFRAME_ERR_BUF_INVAL?  Or introduce a new error code
SFRAME_ERR_BUF_OVERFLOW?

> +      swap_thing (fdep->sfde_func_start_address);
> +      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.  */

Why not SFRAME_ERR_VERSION_INVAL?

> +
> +  return 0;
>  }
>  
>  /* Check if SFrame header has valid data.  */
> @@ -428,6 +445,30 @@ sframe_fre_check_range_p (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.
> +
> +   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, uint8_t ver, uint32_t *num_fres,

Same as above:

sframe_decode_fde (const char *buf, size_t size, uint8_t ver, uint32_t *num_fres,


> +		   uint32_t *fre_type, uint32_t *fre_offset, size_t *fde_size)
> +{
> +  if (ver == SFRAME_VERSION_2)
> +    {
> +      sframe_func_desc_entry_v2 *fdep = (sframe_func_desc_entry_v2 *) buf;

      if (size < sizeof (sframe_func_desc_entry_v2))
        return SFRAME_ERR;

Or SFRAME_ERR_BUF_INVAL or the like.

> +      *num_fres = fdep->sfde_func_num_fres;
> +      *fre_type = sframe_get_fre_type (fdep);
> +      *fre_offset = fdep->sfde_func_start_fre_off;
> +
> +      *fde_size = sizeof (sframe_func_desc_entry_v2);
> +    }
> +  else
> +    return SFRAME_ERR;
> +
> +  return 0;
> +}
Thanks and 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/