[musl] [PATCH v2] clone: clear the frame pointer in the child process on relevant ports

[musl] [PATCH v2] clone: clear the frame pointer in the child process on relevant ports

[Alex Rønne Petersen]

This just mirrors what is done in the start code for the affected ports, as well
as what is already done for the three x86 ports.

Clearing the frame pointer helps protect FP-based unwinders which have no way of
knowing that the FP register should be considered undefined in the child process
portion of clone(). In practice, we found this change to be necessary when
running the Zig standard library tests under qemu-aarch64_be with musl linked.

This version of the patch omits the branch inversion on x86 and powerpc from the
previous version, per the discussion there.
---
 src/thread/aarch64/clone.s     | 3 ++-
 src/thread/arm/clone.s         | 3 ++-
 src/thread/loongarch64/clone.s | 1 +
 src/thread/m68k/clone.s        | 3 ++-
 src/thread/microblaze/clone.s  | 3 ++-
 src/thread/mips/clone.s        | 3 ++-
 src/thread/mips64/clone.s      | 3 ++-
 src/thread/mipsn32/clone.s     | 3 ++-
 src/thread/or1k/clone.s        | 3 ++-
 9 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/src/thread/aarch64/clone.s b/src/thread/aarch64/clone.s
index e3c83395..9ac272bd 100644
--- a/src/thread/aarch64/clone.s
+++ b/src/thread/aarch64/clone.s
@@ -24,7 +24,8 @@ __clone:
 	// parent
 	ret
 	// child
-1:	ldp x1,x0,[sp],#16
+1:	mov fp, 0
+	ldp x1,x0,[sp],#16
 	blr x1
 	mov x8,#93 // SYS_exit
 	svc #0
diff --git a/src/thread/arm/clone.s b/src/thread/arm/clone.s
index bb0965da..4ff0c0e8 100644
--- a/src/thread/arm/clone.s
+++ b/src/thread/arm/clone.s
@@ -19,7 +19,8 @@ __clone:
 	ldmfd sp!,{r4,r5,r6,r7}
 	bx lr

-1:	mov r0,r6
+1:	mov fp,#0
+	mov r0,r6
 	bl 3f
 2:	mov r7,#1
 	svc 0
diff --git a/src/thread/loongarch64/clone.s b/src/thread/loongarch64/clone.s
index a165b365..cb4aacfc 100644
--- a/src/thread/loongarch64/clone.s
+++ b/src/thread/loongarch64/clone.s
@@ -22,6 +22,7 @@ __clone:
 	beqz    $a0, 1f         # whether child process
 	jirl    $zero, $ra, 0   # parent process return
 1:
+	move    $fp, $zero
 	ld.d    $t8, $sp, 0     # function pointer
 	ld.d    $a0, $sp, 8     # argument pointer
 	jirl    $ra, $t8, 0     # call the user's function
diff --git a/src/thread/m68k/clone.s b/src/thread/m68k/clone.s
index f6dfa06f..42ec19f7 100644
--- a/src/thread/m68k/clone.s
+++ b/src/thread/m68k/clone.s
@@ -18,7 +18,8 @@ __clone:
 	beq 1f
 	movem.l (%sp)+,%d2-%d5
 	rts
-1:	move.l %a1,-(%sp)
+1:	suba.l %%fp,%%fp
+	move.l %a1,-(%sp)
 	jsr (%a0)
 	move.l #1,%d0
 	trap #0
diff --git a/src/thread/microblaze/clone.s b/src/thread/microblaze/clone.s
index b68cc5fc..64e3f074 100644
--- a/src/thread/microblaze/clone.s
+++ b/src/thread/microblaze/clone.s
@@ -22,7 +22,8 @@ __clone:
 	rtsd    r15, 8
 	nop

-1:	lwi     r3, r1, 0
+1:	add     r19, r0, r0
+	lwi     r3, r1, 0
 	lwi     r5, r1, 4
 	brald   r15, r3
 	nop
diff --git a/src/thread/mips/clone.s b/src/thread/mips/clone.s
index 04463385..229b987e 100644
--- a/src/thread/mips/clone.s
+++ b/src/thread/mips/clone.s
@@ -27,7 +27,8 @@ __clone:
 	addu $sp, $sp, 16
 	jr $ra
 	nop
-1:	lw $25, 0($sp)
+1:	move $fp, $0
+	lw $25, 0($sp)
 	lw $4, 4($sp)
 	jalr $25
 	nop
diff --git a/src/thread/mips64/clone.s b/src/thread/mips64/clone.s
index 2d86899a..8de3db6c 100644
--- a/src/thread/mips64/clone.s
+++ b/src/thread/mips64/clone.s
@@ -25,7 +25,8 @@ __clone:
 	nop
 	jr	$ra
 	nop
-1:	ld	$25, 0($sp)	# function pointer
+1:	move	$fp, $0
+	ld	$25, 0($sp)	# function pointer
 	ld	$4, 8($sp)	# argument pointer
 	jalr	$25		# call the user's function
 	nop
diff --git a/src/thread/mipsn32/clone.s b/src/thread/mipsn32/clone.s
index 4d3c8c7a..9571231a 100644
--- a/src/thread/mipsn32/clone.s
+++ b/src/thread/mipsn32/clone.s
@@ -25,7 +25,8 @@ __clone:
 	nop
 	jr	$ra
 	nop
-1:	lw	$25, 0($sp)	# function pointer
+1:	move	$fp, $0
+	lw	$25, 0($sp)	# function pointer
 	lw	$4, 4($sp)	# argument pointer
 	jalr	$25		# call the user's function
 	nop
diff --git a/src/thread/or1k/clone.s b/src/thread/or1k/clone.s
index 2473ac20..05c55c69 100644
--- a/src/thread/or1k/clone.s
+++ b/src/thread/or1k/clone.s
@@ -23,7 +23,8 @@ __clone:
 	l.jr	r9
 	 l.nop

-1:	l.lwz	r11, 0(r1)
+1:	l.ori	r2, r0, 0
+	l.lwz	r11, 0(r1)
 	l.jalr	r11
 	 l.lwz	r3, 4(r1)

--
2.40.1

Re: [musl] [PATCH v2] clone: clear the frame pointer in the child process on relevant ports

[Rich Felker]

On Thu, Dec 12, 2024 at 05:56:04PM +0100, Alex Rønne Petersen wrote:
> This just mirrors what is done in the start code for the affected ports, as well
> as what is already done for the three x86 ports.
> 
> Clearing the frame pointer helps protect FP-based unwinders which have no way of
> knowing that the FP register should be considered undefined in the child process
> portion of clone(). In practice, we found this change to be necessary when
> running the Zig standard library tests under qemu-aarch64_be with musl linked.
> 
> This version of the patch omits the branch inversion on x86 and powerpc from the
> previous version, per the discussion there.

Trying to apply this, but...

> diff --git a/src/thread/m68k/clone.s b/src/thread/m68k/clone.s
> index f6dfa06f..42ec19f7 100644
> --- a/src/thread/m68k/clone.s
> +++ b/src/thread/m68k/clone.s
> @@ -18,7 +18,8 @@ __clone:
>  	beq 1f
>  	movem.l (%sp)+,%d2-%d5
>  	rts
> -1:	move.l %a1,-(%sp)
> +1:	suba.l %%fp,%%fp
> +	move.l %a1,-(%sp)
>  	jsr (%a0)
>  	move.l #1,%d0
>  	trap #0

This does not assemble and is most likely not valid. I get:

src/thread/m68k/clone.s:21: Error: operands mismatch -- statement `suba.l %%fp,%%fp' ignored

Do you know what the right instruction to use here is for m68k? And,
have you tested that the rest of the affected archs build?

Rich

Re: [musl] [PATCH v2] clone: clear the frame pointer in the child process on relevant ports

[Alex Rønne Petersen]

On Sat, Feb 22, 2025 at 2:06 AM Rich Felker <dalias@libc.org> wrote:
>
> On Thu, Dec 12, 2024 at 05:56:04PM +0100, Alex Rønne Petersen wrote:
> > This just mirrors what is done in the start code for the affected ports, as well
> > as what is already done for the three x86 ports.
> >
> > Clearing the frame pointer helps protect FP-based unwinders which have no way of
> > knowing that the FP register should be considered undefined in the child process
> > portion of clone(). In practice, we found this change to be necessary when
> > running the Zig standard library tests under qemu-aarch64_be with musl linked.
> >
> > This version of the patch omits the branch inversion on x86 and powerpc from the
> > previous version, per the discussion there.
>
> Trying to apply this, but...
>
> > diff --git a/src/thread/m68k/clone.s b/src/thread/m68k/clone.s
> > index f6dfa06f..42ec19f7 100644
> > --- a/src/thread/m68k/clone.s
> > +++ b/src/thread/m68k/clone.s
> > @@ -18,7 +18,8 @@ __clone:
> >       beq 1f
> >       movem.l (%sp)+,%d2-%d5
> >       rts
> > -1:   move.l %a1,-(%sp)
> > +1:   suba.l %%fp,%%fp
> > +     move.l %a1,-(%sp)
> >       jsr (%a0)
> >       move.l #1,%d0
> >       trap #0
>
> This does not assemble and is most likely not valid. I get:
>
> src/thread/m68k/clone.s:21: Error: operands mismatch -- statement `suba.l %%fp,%%fp' ignored
>
> Do you know what the right instruction to use here is for m68k? And,
> have you tested that the rest of the affected archs build?

Sorry, this was a silly mistake from copying the code from inline
assembly in Zig, where % has to be escaped with an extra %. It should
just simply be suba.l %fp,%fp.

Beyond that, I've assembled all of the archs affected here
successfully, and for those that LLVM supports, they've been tested by
way of Zig's test suite under QEMU.

Re: [musl] [PATCH v2] clone: clear the frame pointer in the child process on relevant ports

[Rich Felker]

On Sat, Feb 22, 2025 at 02:32:45AM +0100, Alex Rønne Petersen wrote:
> On Sat, Feb 22, 2025 at 2:06 AM Rich Felker <dalias@libc.org> wrote:
> >
> > On Thu, Dec 12, 2024 at 05:56:04PM +0100, Alex Rønne Petersen wrote:
> > > This just mirrors what is done in the start code for the affected ports, as well
> > > as what is already done for the three x86 ports.
> > >
> > > Clearing the frame pointer helps protect FP-based unwinders which have no way of
> > > knowing that the FP register should be considered undefined in the child process
> > > portion of clone(). In practice, we found this change to be necessary when
> > > running the Zig standard library tests under qemu-aarch64_be with musl linked.
> > >
> > > This version of the patch omits the branch inversion on x86 and powerpc from the
> > > previous version, per the discussion there.
> >
> > Trying to apply this, but...
> >
> > > diff --git a/src/thread/m68k/clone.s b/src/thread/m68k/clone.s
> > > index f6dfa06f..42ec19f7 100644
> > > --- a/src/thread/m68k/clone.s
> > > +++ b/src/thread/m68k/clone.s
> > > @@ -18,7 +18,8 @@ __clone:
> > >       beq 1f
> > >       movem.l (%sp)+,%d2-%d5
> > >       rts
> > > -1:   move.l %a1,-(%sp)
> > > +1:   suba.l %%fp,%%fp
> > > +     move.l %a1,-(%sp)
> > >       jsr (%a0)
> > >       move.l #1,%d0
> > >       trap #0
> >
> > This does not assemble and is most likely not valid. I get:
> >
> > src/thread/m68k/clone.s:21: Error: operands mismatch -- statement `suba.l %%fp,%%fp' ignored
> >
> > Do you know what the right instruction to use here is for m68k? And,
> > have you tested that the rest of the affected archs build?
> 
> Sorry, this was a silly mistake from copying the code from inline
> assembly in Zig, where % has to be escaped with an extra %. It should
> just simply be suba.l %fp,%fp.

Thanks, that fixed it. I should have read closer...

> Beyond that, I've assembled all of the archs affected here
> successfully, and for those that LLVM supports, they've been tested by
> way of Zig's test suite under QEMU.

OK.

Rich