[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250701161928.GW6263@brightrain.aerifal.cx>
Date: Tue, 1 Jul 2025 12:19:28 -0400
From: Rich Felker <dalias@...c.org>
To: Alex Rønne Petersen <alex@...xrp.com>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH v2] clone: clear the frame pointer in the child
process on relevant ports
On Tue, Jul 01, 2025 at 06:14:28PM +0200, Alex Rønne Petersen wrote:
> On Tue, Jul 1, 2025, at 18:00, Rich Felker 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.
> >> ---
> >> 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
> >
> > The alias fp does not seem to be supported across all assemblers. I'm
> > committing a fix that changes this to use the real name x29 unless
> > there's any objection and proposed correction.
>
> That seems fine.
>
> > This should be checked on other archs too.
>
> * arm: fp -> r11
> * loongarch: $fp -> $r22
> * m68k: %fp -> %a6
> * mips: $fp -> $30
>
> I'm not sure for which it's actually necessary, but I see no
> particular harm in just making the changes anyway, just in case.
Casual inspection showed mips and loongarch already use the role-based
mnemonics successfully elsewhere so I think they're fine. I'll check
the others. FWIW I think the role-based ones make the code more
readable and keeping them is preferable except where they're not
reliably supported.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
