[PATCH 0/6] Fix unwinding through sigreturn trampolines

[Ard Biesheuvel]

On Mon, 6 Jul 2020 at 12:29, Daniel Kiss <Daniel.Kiss at arm.com> wrote:
>
> 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.
>

I just copied that from libgcc's implementation, so using the existing
reserved register for that indeed seems like a much better approach.

The only remaining question is how to deal with FP/SIMD and SVE
registers, but given Szabolcs's assertion that exceptions thrown in
signal handlers cannot be caught outside of them anyway, this does not
seem like a huge deal, with the exception of the nested function case
for pthread cleanup handlers but perhaps the solution for that is
'don't do that'.




> 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
> >
>