[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241209130741.GS10433@brightrain.aerifal.cx>
Date: Mon, 9 Dec 2024 08:07:42 -0500
From: Rich Felker <dalias@...c.org>
To: Alex Rønne Petersen <alex@...xrp.com>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] clone: clear the frame pointer in the child
process on relevant ports
On Mon, Dec 09, 2024 at 02:04:11PM +0100, Alex Rønne Petersen wrote:
> On Mon, Dec 9, 2024 at 1:30 PM Rich Felker <dalias@...c.org> wrote:
> >
> > On Sat, Dec 07, 2024 at 07:47:38PM +0100, Alex Rønne Petersen wrote:
> > > On Sat, Dec 7, 2024 at 6:04 AM Rich Felker <dalias@...c.org> wrote:
> > > >
> > > > On Fri, Dec 06, 2024 at 07:48:53PM +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. For consistency, I also changed
> > > > > the x86 ports and the powerpc port to have the child process portion at the end
> > > > > of clone().
> > > >
> > > > Can you submit without this independent change? Readers of history
> > > > should be able to confirm that the patch does not make any other
> > > > functional change, and it's hard to do that when reorganizing code is
> > > > mixed with the change.
> > >
> > > Can do. Should I send it as a separate patch or would you rather I
> > > just omit that particular change?
> >
> > My leaning would be omit. For changes like this, my heuristic is to
> > ask (1) whether, if I were a user, I would be angry if a regression
> > somehow got introduced through the change (which offered nothing of
> > value to me as a user in return for the risk), and (2) whether, as a
> > reviewer reading git log looking for accidentally or intentionally
> > introduced bugs, I'd appreciate spending my time reading the change.
>
> Just for some background: The reason I included the change was that in
> the equivalent Zig standard library code, we did exactly this because
> we needed to add DWARF CFI annotations (.cfi_undefined) to the child
> process portion of clone(). If the child process portion isn't moved
> last, then the CFI annotations end up covering parent process code
> that they shouldn't. My thinking was that it might make sense to
> similarly rearrange the code in musl just in case CFI annotations ever
> need to be added there.
Thanks for explaning the motivation. I think it would make sense to do
that change with adding CFI if the latter becomes something we want to
do at some point (it might be wanted just as debug info if nothing
else?) but not unless/until that.
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.
