|
Message-ID: <CAH9TF6Nqt79NQHZx+q+Jc8kmCVjgwEH_siWKN4R9Usp=areQ6A@mail.gmail.com> Date: Mon, 9 Dec 2024 14:04:11 +0100 From: Alex Rønne Petersen <alex@...xrp.com> To: Rich Felker <dalias@...c.org> Cc: musl@...ts.openwall.com Subject: Re: [PATCH] clone: clear the frame pointer in the child process on relevant ports 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. That said, I take your point re: risk, so omitting makes sense to me.
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.