[PATCH, V2 1/5] libsframe: fix issue finding FRE in PCMASK type SFrame FDEs

[Jens Remus]

Hello Indu,

thank you for taking my patches with your series!  I am sorry that I did
not make it to send the following improvement and minor fix yesterday.

On 22.05.2025 19:53, Indu Bhagat wrote:
> SFrame FDEs of type SFRAME_FDE_TYPE_PCMASK are used for repetitive code
> patterns, e.g., pltN entries.  For SFrame FDEs of type
> SFRAME_FDE_TYPE_PCMASK, sframe_fre_check_range_p erroneously tested the
> given PC instead of the masked PC offset from function start address.
> Therefore it only worked correctly by chance, e.g., if the function start
> address was aligned on the repetition block size.
> 
> For regular SFrame FDEs the PC offset from function start address must
> be within a SFrame FRE's start IP offset and end IP offset.  For SFrame
> FDEs of type SFRAME_FDE_TYPE_PCMASK, the masked PC offset must be within
> that range.
> 
> SFrame FRE start/end IP offsets are relative to the SFrame FDE function
> start address. For regular SFrame FDEs, the PC offset from function
> start address must be within a SFrame FRE's start IP offset and end IP
> offset.  For SFRAME_FDE_TYPE_PCMASK type FDEs, the masked PC offset must
> be within that range.
> 
> Exercise the testcase for a variety of placements; without the fix some
> of these tests will fail.  Also, make the testcase itself easier to
> follow by adding appropriate vars where applicable.

> diff --git a/libsframe/sframe.c b/libsframe/sframe.c
> index c2693b978ec..c1bc692829a 100644
> --- a/libsframe/sframe.c
> +++ b/libsframe/sframe.c
> @@ -372,40 +372,27 @@ sframe_fre_check_range_p (sframe_func_desc_entry *fdep,
>  			  int32_t start_ip_offset, int32_t end_ip_offset,
>  			  int32_t pc)

As discussed in the V1 thread, I came to the conclusion that both
start_ip_offset and end_ip_offset need to be uint32_t to correctly
reflect that they are actually unsigned:
https://inbox.sourceware.org/binutils/7d587a26-570c-40af-89fd-cf948864e5f8@linux.ibm.com/

>  {
> -  int32_t start_ip, end_ip;
>    int32_t func_start_addr;
>    uint8_t rep_block_size;
>    uint32_t fde_type;
> -  int32_t masked_pc;
> +  int32_t pc_offset;

pc_offset also needs to be unsigned, which is always guaranteed if
sfde_func_start_address <= pc, which is currently always the case
in the only caller of sframe_fre_check_range_p.

I have sent a separate patch "[PATCH] libsframe: handle SFrame FRE
start/end IP offsets as unsigned" in reply to your cover letter to
correct that.  Please consider to take it as part or your series.

I have also sent a minor related fixup in reply to one of my patches.

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/