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

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

The following reply was made to PR lib/55719; it has been noted by GNATS.

From: Kamil Rytarowski <n54%gmx.com@localhost>
To: gnats-bugs%netbsd.org@localhost, kamil%netbsd.org@localhost, gnats-admin%netbsd.org@localhost,
 netbsd-bugs%netbsd.org@localhost, nikhil.benesch%gmail.com@localhost,
 Andrew Cagney <andrew.cagney%gmail.com@localhost>
Cc: 
Subject: Re: lib/55719 (Unwind tables for signal trampoline on amd64 are
 incorrect)
Date: Mon, 19 Oct 2020 13:38:29 +0200

 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-a=
 dmin%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:
 >
 >  =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 >  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.
 >  >
 >  >
 >  >
 >
 >