[PATCH 1/3] libsframe: fix issue in finding FRE in SFRAME_FDE_TYPE_PCMASK type SFrame FDEs

[Jens Remus]

Maybe shorten the commit subject to:
 
libsframe: fix issue finding FRE in type PCMASK SFrame FDEs

On 20.05.2025 21:03, Indu Bhagat wrote:
> Lookup of information from SFrame FDEs of type SFRAME_FDE_TYPE_PCMASK,
> typically used for pltN entries had an issue: the current code will only
> work for certain placements of .plt and .sframe.  Fix it, and add a
> testcase.

I agree with Jan:  Please explain that SFrame FRE start/end IP offsets
are relative to the SFrame FDE function start address.  Therefore the
(masked) PC offset from function start address needs to be compared
against those start/end IP offsets.

For SFrame FDEs of type PCMASK sframe_fre_check_range_p erroneously
tested the masked PC instead of the masked PC offset from function start
address.  Therefore it only worked correctly by chance, if the function
start address was aligned on the repetition block size.

> 
> Make the testcase itself easier to follow by adding appropriate vars
> where applicable.


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

> @@ -398,10 +398,13 @@ sframe_fre_check_range_p (sframe_func_desc_entry *fdep,
>      }
>    else
>      {
> -      /* For FDEs for repetitive pattern of insns, we need to return the FRE
> -	 where pc % rep_block_size is between start_ip_offset and
> -	 end_ip_offset.  */
> -      masked_pc = pc % rep_block_size;
> +      /* For SFrame FDEs encoding information for repetitive pattern of insns,
> +	 masking with the rep_block_size is necessary to find the matching FRE.
> +	 start_ip_offset and end_ip_offset are _unsigned_values_ identifying

Changing start_ip_offset and end_ip_offset to uint32_t is probably no
longer feasible, as this would break the libsframe API?

> +	 the SFrame FRE.  So, perform a calculation to first get the distance
> +	 between the pc and func_start_addr, followed by modulo to find the
> +	 fitting range.  */
> +      masked_pc = (pc - func_start_addr) % rep_block_size;
>        ret = ((start_ip_offset <= masked_pc) && (end_ip_offset >= masked_pc));
>      }

I think the whole function could be simplified.  See my patch
"[PATCH 1/3] libsframe: simplify SFrame FRE range check" [1], which is based
on top of yours and which you could squash into yours.

[1]: [PATCH 1/3] libsframe: simplify SFrame FRE range check,
     https://inbox.sourceware.org/binutils/20250521083059.55048-2-jremus@linux.ibm.com/

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/