Re: [PATCH] sparc: Remove unwind information from signal return stub functions

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 15/01/24 11:22, Daniel Cederman wrote:
> On 2024-01-15 14:41, Adhemerval Zanella Netto wrote:
>>
>>
>> On 12/01/24 06:26, Daniel Cederman wrote:
>>> The functions were previously written in C, but were not compiled
>>> with unwind information. The ENTRY/END macros includes .cfi_startproc
>>> and .cfi_endproc which adds unwind information. This caused the
>>> tests cleanup-8 and cleanup-10 in the GCC testsuite to fail.
>>> This patch adds a version of the ENTRY/END macros without the
>>> CFI instructions that can be used instead.
>>>
>>> sigaction registers a restorer address that is located two instructions
>>> before the stub function. This patch adds a two instruction padding to
>>> avoid that the unwinder accesses the unwind information from the function
>>> that the linker has placed right before it in memory. This fixes an issue
>>> with pthread_cancel that caused tst-mutex8-static (and other tests) to fail.
>>>
>>> This patch fixes the issues I have seen, but I am not sure if this is
>>> the right solution, so any feedback is welcome!
>>>
>>> Signed-off-by: Daniel Cederman <cederman@gaisler.com>
>>
>> I am not sure this is the correct fix, at least on a sparc machine I
>> do not any improvements on a sparcv9 build. But do I agree that this
> 
> With sparcv9 build, do you mean a 32-bit or 64-bit build? The patch only targeted the 32-bit version of the stubs and it has been working fine for me on both hardware and in QEMU.
> 
>> seems to be a wrong/missing unwind information for the signal return
>> stub: the tst-mutex8-static does seems be on an infinite loop in the
>> libgcc unwinder code.
>>
>> Does it help to restore the stub to be coded in C? It seems that the
> 
> Yes, that helps. And the only difference I noticed was that the assembly version had unwind information while the C version did not.
> 

In fact it was a mistake from my part while testing it on sparcv9 32 bits,
with this patch apply it does seems to fix the failures:

FAIL: debug/tst-backtrace4
FAIL: debug/tst-backtrace5
FAIL: nptl/tst-cancel24-static
FAIL: nptl/tst-cancelx20
FAIL: nptl/tst-cancelx21
FAIL: nptl/tst-cond8-static
FAIL: nptl/tst-mutex8-static
FAIL: nptl/tst-mutexpi8-static
FAIL: nptl/tst-rwlock15

So it does look from my part.