Re: [PATCH 11/12] unwind: Implement compat fp unwind

[Jens Remus <jremus@linux.ibm.com>]

Hello Peter!

On 10/17/2025 5:47 PM, Jens Remus wrote:
> while rebasing the unwind user sframe series on top of this series and
> https://lore.kernel.org/linux-trace-kernel/20251007214008.080852573@kernel.org/
> I ran into the following issue:
> 
> On 9/24/2025 9:59 AM, Peter Zijlstra wrote:
> 
>> --- a/include/linux/unwind_user_types.h
>> +++ b/include/linux/unwind_user_types.h
>> @@ -36,6 +36,7 @@ struct unwind_user_state {
>>  	unsigned long				ip;
>>  	unsigned long				sp;
>>  	unsigned long				fp;
>> +	unsigned int				ws;
> 
> Factoring out the word size (ws) from the CFA, FP, and RA offsets is
> clever.  Wondering though whether that would be an issue for unwind user
> sframe.  Do all architectures guarantee that those offsets are aligned
> to the native word size?
> 
>>  	enum unwind_user_type			current_type;
>>  	unsigned int				available_types;
>>  	bool					done;
> 
>> --- a/kernel/unwind/user.c
>> +++ b/kernel/unwind/user.c
> 
>> @@ -29,21 +44,21 @@ static int unwind_user_next_fp(struct un
>>  	}
>>  
>>  	/* Get the Canonical Frame Address (CFA) */
>> -	cfa += frame->cfa_off;
>> +	cfa += state->ws * frame->cfa_off;
> 
> In SFrame the CFA, FP, and RA offsets are unscaled.  Would it be ok, if
> unwind user sframe would factor state->ws from those offset values?  What
> if they were not aligned?  unwind user sframe would then have to fail.

Sorry that I did not immediately think about the most obvious solution
tho above issues:  to not factor out the word size from the frame CFA,
FP, and RA offsets.  What do you think about making the following
changes to this and giyour subsequent patch?  That would work nicely
with unwind user sframe.


diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
--- a/kernel/unwind/user.c
+++ b/kernel/unwind/user.c
@@ -8,19 +8,15 @@
 #include <linux/unwind_user.h>
 #include <linux/uaccess.h>
 
-static const struct unwind_user_frame fp_frame = {
-	ARCH_INIT_USER_FP_FRAME
-};
-
 #define for_each_user_frame(state) \
 	for (unwind_user_start(state); !(state)->done; unwind_user_next(state))
 
 static inline int
-get_user_word(unsigned long *word, unsigned long base, int off, int size)
+get_user_word(unsigned long *word, unsigned long base, int off, unsigned int ws)
 {
-	unsigned long __user *addr = (void __user *)base + (off * size);
+	unsigned long __user *addr = (void __user *)base + off;
 #ifdef CONFIG_COMPAT
-	if (size == sizeof(int)) {
+	if (ws == sizeof(int)) {
 		unsigned int data;
 		int ret = get_user(data, (unsigned int __user *)addr);
 		*word = data;
@@ -32,6 +28,9 @@ get_user_word(unsigned long *word, unsigned long base, int off, int size)
 
 static int unwind_user_next_fp(struct unwind_user_state *state)
 {
+	const struct unwind_user_frame fp_frame = {
+		ARCH_INIT_USER_FP_FRAME(state->ws)
+	};
 	const struct unwind_user_frame *frame = &fp_frame;
 	unsigned long cfa, fp, ra;
 
@@ -44,7 +43,7 @@ static int unwind_user_next_fp(struct unwind_user_state *state)
 	}
 
 	/* Get the Canonical Frame Address (CFA) */
-	cfa += state->ws * frame->cfa_off;
+	cfa += frame->cfa_off;
 
 	/* stack going in wrong direction? */
 	if (cfa <= state->sp)


diff --git a/arch/x86/include/asm/unwind_user.h b/arch/x86/include/asm/unwind_user.h
--- a/arch/x86/include/asm/unwind_user.h
+++ b/arch/x86/include/asm/unwind_user.h
@@ -2,10 +2,10 @@
 #ifndef _ASM_X86_UNWIND_USER_H
 #define _ASM_X86_UNWIND_USER_H
 
-#define ARCH_INIT_USER_FP_FRAME				\
-	.cfa_off	=  2,				\
-	.ra_off		= -1,				\
-	.fp_off		= -2,				\
+#define ARCH_INIT_USER_FP_FRAME(ws)			\
+	.cfa_off	=  2*(ws),			\
+	.ra_off		= -1*(ws),			\
+	.fp_off		= -2*(ws),			\
 	.use_fp		= true,
 
 #endif /* _ASM_X86_UNWIND_USER_H */


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/