Re: [PATCH v15 16/20] unwind_user/sframe: Add support for SFrame V3 flexible FDEs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The patch references undefined macros `SFRAME_REG_SP` and `SFRAME_REG_FP` and includes a nonexistent architecture-specific header `<asm/unwind_user_sframe.h>`, resulting in a build failure.
- [Low] The variable `ret` is used outside of its declared scope in `__find_fre()`, resulting in a compilation error.
--

commit 725751526853387385ad4dffd9fdbe24d42e98d3
Author: Jens Remus <jremus@linux.ibm.com>

unwind_user/sframe: Add support for SFrame V3 flexible FDEs

SFrame V3 introduces flexible FDEs in addition to the regular FDEs.
The key difference is that flexible FDEs encode the CFA, RA, and FP
tracking information using two FRE data words, a control word and an
offset, or a single padding data word of zero.

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index 6187379750db5..48709f0bafc78 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c
> @@ -12,6 +12,7 @@
>  #include <linux/mm.h>
>  #include <linux/string_helpers.h>
>  #include <linux/sframe.h>
> +#include <asm/unwind_user_sframe.h>

[Severity: Low]
Does this new architecture-specific header exist in the tree at this point?

It appears this include, along with macros like SFRAME_REG_SP and
SFRAME_REG_FP used later in the patch, causes a build failure because they
aren't introduced until a later commit (bfea14fc9b480 "unwind_user/sframe/x86:
Enable sframe unwinding on x86").

Each patch in a series should be self-contained and compile successfully on
its own.

[ ... ]

> @@ -356,9 +528,15 @@ static __always_inline int __find_fre(struct sframe_section *sec,
>  		return -EINVAL;
>  	fre = prev_fre;
>  
> -	sframe_init_cfa_rule_data(&frame->cfa, fre->info, fre->cfa_off);
> -	sframe_init_rule_data(&frame->ra, fre->ra_off);
> -	sframe_init_rule_data(&frame->fp, fre->fp_off);
> +	ret = sframe_init_cfa_rule_data(&frame->cfa, fre->cfa_ctl, fre->cfa_off);
        ^^^

[Severity: Low]
Is the variable ret in scope here?

It looks like int ret is declared inside the for loop in __find_fre(), so
accessing it outside the loop results in a compilation error.

This is fixed in a later commit (9c045164b82cb "unwind_user/sframe: Separate
reading of FRE from reading of FRE data words") which moves the declaration to
the function scope, but it breaks the build on this commit.

> +	if (ret)
> +		return ret;
> +	ret = sframe_init_rule_data(&frame->ra, fre->ra_ctl, fre->ra_off);
> +	if (ret)
> +		return ret;
> +	ret = sframe_init_rule_data(&frame->fp, fre->fp_ctl, fre->fp_off);
> +	if (ret)
> +		return ret;
>  	frame->outermost = SFRAME_V3_FRE_RA_UNDEFINED_P(fre->info);
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520154004.3845823-1-jremus@linux.ibm.com?part=16