[PATCH v4 1/4] gas: ld: libsframe: Support for SFrame FDEs without any FREs

[Indu Bhagat]

Hi Jens,

Thanks for accommodating the change of course by adopting SFrame FRE 
with 0 offsets.  I realize that should have come sooner..

On 7/25/25 9:23 AM, Jens Remus wrote:
> Allow SFrame sections without any FREs, that can occur if they solely
> contain FDEs without any FREs.  For FDEs without and FREs set the
> offset to the first FRE to zero.
> 
> libsframe/
> 	* sframe.c (sframe_encoder_write_sframe): Allow SFrame sections
> 	without any FREs.  For FDEs without any FREs set the offset to
> 	the first FRE to zero.
> 
> gas/
> 	* gen-sframe.c (output_sframe_funcdesc): For FDEs without any
> 	FREs set the offset to the first FRE to zero.
> 
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> ---
> 
> Notes (jremus):
>      Changes in V4:
>      - Use ehp->sfh_num_fres instead of adding a new API to get the total
>        number of FREs. (Indu)
>      - Squash related assembler change from V3.
>      - Reword commit subject and message.
>      
>      Changes in V3:
>      - New patch.
>      
>      This patch is strictly no longer required.  Posting again to keep a
>      copy for future reference, even if we decide not to make this change
>      now.  For the time being we might prefer to error out in the linker
>      when running into SFrame FDEs without any FREs.
> 

Whats the reason to prefer to error out for SFrame FDE without any FRE ?

>   gas/gen-sframe.c   | 18 ++++++++++++------
>   libsframe/sframe.c |  9 ++++++++-
>   2 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
> index d082b974be3f..acea1dc7dbb7 100644
> --- a/gas/gen-sframe.c
> +++ b/gas/gen-sframe.c
> @@ -651,12 +651,18 @@ output_sframe_funcdesc (symbolS *start_of_fre_section,
>   				  sfde_func_size));
>   
>     /* Offset to the first frame row entry.  */
> -  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));
> +  if (sframe_fde->num_fres == 0)
> +    /* SFrame FDEs without any FREs have a FRE offset of 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));
> +    }
>   
>     /* Number of FREs.  */
>     out_four (sframe_fde->num_fres);
> diff --git a/libsframe/sframe.c b/libsframe/sframe.c
> index 824d1fd7ed99..9814a3ae4c1f 100644
> --- a/libsframe/sframe.c
> +++ b/libsframe/sframe.c
> @@ -1880,7 +1880,7 @@ sframe_encoder_write_sframe (sframe_encoder_ctx *encoder)
>        - buffers must be malloc'd by the caller.  */
>     if ((contents == NULL) || (buf_size < hdr_size))
>       return sframe_set_errno (&err, SFRAME_ERR_BUF_INVAL);
> -  if (fr_info == NULL)
> +  if (ehp->sfh_num_fres > 0 && fr_info == NULL)
>       return sframe_set_errno (&err, SFRAME_ERR_FRE_INVAL);
>   
>     /* Write out the FRE table first.
> @@ -1903,6 +1903,13 @@ sframe_encoder_write_sframe (sframe_encoder_ctx *encoder)
>         fre_type = sframe_get_fre_type (fdep);
>         num_fres = fdep->sfde_func_num_fres;
>   
> +      /* FDEs without any FDEs have a FRE offset of zero.  */

Perhaps reword for clarity:

For FDES without any FREs, set sfde_func_start_fre_off to zero.

> +      if (num_fres == 0)
> +	fdep->sfde_func_start_fre_off = 0;
> +

Ah I see the adding funcdesc via sframe_encoder_add_funcdesc sets the 
sfde_func_start_fre_off.  BTW, I am curious if not doing so shows up as 
a problem somewhere ?

> +      if (num_fres > 0 && fr_info == NULL)
> +	return sframe_set_errno (&err, SFRAME_ERR_FRE_INVAL);
> +

Why check again per FDE ?

>         for (j = 0; j < num_fres; j++)
>   	{
>   	  fre_index = global + j;