[PATCH 0/6] Fix unwinding through sigreturn trampolines

[Daniel Kiss]

Hi Ard,

I like your suggestions and tuned a bit and now it works with the LLVM’s unwinders.

Register 96 is out of the DWARF spec[1] and will collide with SVE registers[2] so 32 is better which is the reserved register for PC.

my version:

#define ARM64_SIGFRAME_REGS_OFFSET 312 /* offsetof (struct rt_sigframe, uc.uc_mcontext.regs) */

    .text
    .cfi_startproc
    .cfi_signal_frame
    
    .cfi_def_cfa    sp, ARM64_SIGFRAME_REGS_OFFSET
    .irp x, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, \
        13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23,  \
        24, 25, 26, 27, 28, 29, 30, 31
    .cfi_offset \x, \x * 8
    .endr

    .cfi_offset 32, 32 * 8 // regs->pc
    .cfi_return_column 32

    nop
ENTRY(__kernel_rt_sigreturn)
    mov x8, #__NR_rt_sigreturn
    svc #0
    .cfi_endproc
ENDPROC(__kernel_rt_sigreturn)

[1] https://developer.arm.com/documentation/ihi0057/e
[2] https://developer.arm.com/documentation/100985/0000/

> 
> 
>> On 23 Jun 2020, at 17:43, Ard Biesheuvel <ardb at kernel.org> wrote:
>> 
>> On Tue, 23 Jun 2020 at 17:34, Daniel Kiss <Daniel.Kiss at arm.com> wrote:
>>> 
>>> Hi Will,
>>> 
>>> The CFI is correct for PowerPC[1] and X86[2] at least; I did not check others.
>>> so LLVM’s unwinder can unwind/backtrace sigreturn without any specific hack on those architectures.
>>> The a4eb355a3fda change implements the same CFI as PowerPC and X86 have so the generic unwind logic is able to process the sigreturn.
>> 
>> It most certainly did not implement the same CFI. The x86 and PowerPC
>> examples you quote have elaborate asm foo to emit the eh_frame by
>> hand.
>> 
>>> IMHO the kernel change was fine.
>>> 
>> 
>> It creates easily reproducible segfaults in the libgcc unwinder.
>> 
>>> I’m not comfortable to say the instruction sequence is the ABI that describe the unwinding information.
>>> I guess the pattern matching was implemented due to the CFI was wrong.
>>> 
>>> The .cfi_signal_frame could be useful once the entry is found, but needs the right .CFI annotations at the right place.
>>> 
>>> This change definitely a not good for LLVM’s unwinder.
>>> 
>> 
>> I agree that it would be better for the CFI to be correct, but that
>> takes a bit of work at the very least, and even then, the variable
>> nature of our sigframe may make it impossible to restore the FP/SIMD
>> register state reliably.
>> 
>> I did some tests with the below CFI directives, where
>> ARM64_SIGFRAME_REGS_OFFSET is emitted by asm-offsets as the offset
>> from the top of the sigframe to the regs[] array. This fixes the
>> segfaults, and seems to do the right thing if i single step through
>> the unwinder as it unwinds the stack in response to a call to
>> pthread_cancel(). But we need a lot of testing to ensure that this is
>> correct.
>> 
>> 
>> .cfi_def_cfa_offset ARM64_SIGFRAME_REGS_OFFSET
>> .irp r, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, \
>>    13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23,  \
>>    24, 25, 26, 27, 28, 29, 30, 31
>> .cfi_offset \r, \r * 8
>> .endr
>> 
>> .cfi_offset 96, 32 * 8 // regs->pc
>> .cfi_return_column 96
>