Re: lib/55719 (Unwind tables for signal trampoline on amd64 are incorrect)

[Kamil Rytarowski <n54%gmx.com@localhost>]

Thanks for the patch. I've applied it and repeated it in i386.

I was wondering whether we need to restore the stack pointer too.

While there, I'm getting the following backtrace for NetBSD/i386:

Backtrace 6 stack frames.
0x8048972 <handler+0x2d> at ./a.out
0xfbd93010 <__sigtramp_siginfo_2> at /usr/lib/i386/libc.so.12
0x804899e <run1> at ./a.out
0x80489a5 <run3> at ./a.out
0x80489aa <run3+0x5> at ./a.out
0x80489de <__x86.get_pc_thunk.ax> at ./a.out

for "gcc -fomit-frame-pointer  -O3 test.c -lexecinfo -m32". Is this
correct? I don't see <main> over there.

http://netbsd.org/~kamil/backtrace/libc-i386-PR55719-multiple-frames.txt

On 18.10.2020 18:05, Nikhil Benesch wrote:
> The following reply was made to PR lib/55719; it has been noted by GNATS.
>
> From: Nikhil Benesch <nikhil.benesch%gmail.com@localhost>
> To: gnats-bugs%netbsd.org@localhost
> Cc: Kamil Rytarowski <kamil%netbsd.org@localhost>, netbsd-bugs%netbsd.org@localhost, gnats-admin%netbsd.org@localhost
> Subject: Re: lib/55719 (Unwind tables for signal trampoline on amd64 are incorrect)
> Date: Sun, 18 Oct 2020 12:01:08 -0400
>
>  Ah, unfortunately I introduced a buglet with the revisions. Apologies
>  for not noticing sooner. The problem only presents when backtracing
>  through *multiple* frames before a signal handler and when those
>  frames don't use a frame pointer. I've included a modified version of
>  Kamil's test program that surfaces the bug here:
>
>  https://gist.github.com/benesch/2b2b4ffb76019d7a2cda6c6002bb2aa0
>
>  With -O0, the backtrace works perfectly on NetBSD-current:
>
>  $ ./test
>  Backtrace 4 stack frames.
>  0x400bb0 <handler+0x30> at ./test
>  0x7a64384a7870 <__sigtramp_siginfo_2> at /usr/lib/libc.so.12
>  0x400be9 <run+0x4> at ./test
>  0x400c0e <main+0x23> at ./test
>
>  With -O2 the backtrace gets stuck at the run function:
>
>  $ ./test
>  Backtrace 3 stack frames.
>  0x400ba0 <handler+0x20> at ./test
>  0x73959f0a7870 <__sigtramp_siginfo_2> at /usr/lib/libc.so.12
>  0x400bc8 <run> at ./test
>
>  Looking at the generated CFI for the test program makes the issue
>  clear. Without optimizations, the CFA is computed via the base
>  pointer. With optimizations, it is computed via the stack pointer. So
>  the CFI we added to NetBSD's sigtramp mishandles restoration of RSP.
>  The fix is quite straightforward:
>
>  ===================================================================
>  RCS file: /cvsroot/src/lib/libc/arch/x86_64/sys/__sigtramp2.S,v
>  retrieving revision 1.8
>  diff -u -r1.8 __sigtramp2.S
>  --- arch/x86_64/sys/__sigtramp2.S       12 Oct 2020 17:55:54 -0000      1.8
>  +++ arch/x86_64/sys/__sigtramp2.S       18 Oct 2020 15:51:51 -0000
>  @@ -58,7 +58,7 @@
>          .cfi_offset rsi, UC_GREGS_RSI
>          .cfi_offset rdi, UC_GREGS_RDI
>          .cfi_offset rbp, UC_GREGS_RBP
>  -       /* The unwinder will use the CFA to restore RSP. */
>  +       .cfi_offset rsp, UC_GREGS_RSP
>          .cfi_offset r8,  UC_GREGS_R8
>          .cfi_offset r9,  UC_GREGS_R9
>          .cfi_offset r10, UC_GREGS_R10
>
>  (My email client has a tendency to misformat patches, so I've included
>  another copy of the patch in the GitHub Gist I linked to above.)
>
>  Basically, because we changed the CFI for the sigtramp to *not* use
>  RSP as the CFA, we need to explicitly restore RSP to the value stored
>  in the context. Otherwise the unwinder will implicitly set RSP to the
>  CFA, which is not correct in this case. (Of course, without
>  optimizations things work out ok, since the CFI doesn't refer to the
>  mishandled RSP.)
>
>  On Mon, Oct 12, 2020 at 1:58 PM <kamil%netbsd.org@localhost> wrote:
>  >
>  > Synopsis: Unwind tables for signal trampoline on amd64 are incorrect
>  >
>  > State-Changed-From-To: open->closed
>  > State-Changed-By: kamil%NetBSD.org@localhost
>  > State-Changed-When: Mon, 12 Oct 2020 19:58:00 +0200
>  > State-Changed-Why:
>  > Fix committe.
>  >
>  >
>  >
>
>