[Toybox] [musl] Re: Not sure how to debug this one.

[Rob Landley]

On 2/17/24 19:34, Rich Felker wrote:
> On Sun, Feb 18, 2024 at 12:45:35AM +0300, Valery Ushakov wrote:
>> On Fri, Feb 16, 2024 at 19:48:27 -0600, Rob Landley wrote:
>> 
>> > https://git.musl-libc.org/cgit/musl/tree/src/signal/sh/sigsetjmp.s

FYI the next round of debugging I had queued up was cutting and pasting the
sigsetjmp+setjmp assembly blob inline in the code (either as a C asm { }
directive or my own .s->.o nailed to the side of the link) and removing one
instruction from the end of the testfunc() at a time until the crash went away.
(Doesn't matter if it _works_, the point is does it _return_. Find the
instruction that poked the bear.)

I was just AFK most of the weekend...

> The only problem with the sh implementation is that, due to limited
> displacement range for load/store instructions, we have to add 60 to
> the sigjmp_buf base address to get the base register for accessing the
> saved return address and r8. This takes place in a temp register r6.
> However, the last line of this code path, the delay slot after the
> tail call to __sigsetjmp_tail, erroneously uses r4 (the base
> sigjmp_buf pointer) as the base for restoring r8:
> 
> 	 mov.l @(4+8,r4), r8
> 
> This corrupts the caller's saved register file. If I'm not mistaken,
> it overwrites the caller's r8 with the caller's r11.
> 
> Presumably Rob didn't actually hit a crash in sigsetjmp, but just
> inferred the crash was there via printf debugging. The actual crash
> must be in the caller once it gets back control with the wrong value
> in r8.

I saw that the printf after the sigsetjmp did not trigger, yes. Why was the
pending question...

> This has been an area I've wanted to have testing for for a long time,
> but I don't have a really good idea for how to implement the test. We
> would want the compiler to generate code that puts lots of
> intermediates in as many call-saved registers as possible, calls
> setjmp, then calls another function that *also* puts pressure on the
> register allocation and then calls longjmp. If it's okay for the test
> to be arch-specific, this could just work via __asm__ register
> constraints and a call to setjmp from asm, but I'm not sure if that
> would be a good way to do things in libc-test...
> 
> In any case, thanks to everyone who participated in this. This is a
> rather bad bug and a nice one to get fixed up in the release window!

Very much so. Thank you.

I pulled musl-git but didn't see a commit yet, lemme know when I can build a new
test toolchain to confirm the fix.

> Rich

Thanks,

Rob