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

[Nikhil Benesch <nikhil.benesch%gmail.com@localhost>]

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

From: Nikhil Benesch <nikhil.benesch%gmail.com@localhost>
To: Kamil Rytarowski <kamil%netbsd.org@localhost>
Cc: gnats-bugs%netbsd.org@localhost, lib-bug-people%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost, 
	gnats-admin%netbsd.org@localhost, Andrew Cagney <andrew.cagney%gmail.com@localhost>, 
	=?UTF-8?B?TWljaGHFgiBHw7Nybnk=?= <mgorny%netbsd.org@localhost>
Subject: Re: lib/55719 (Unwind tables for signal trampoline on amd64 are incorrect)
Date: Sun, 11 Oct 2020 21:18:04 -0400

 --0000000000009eda3505b16f1257
 Content-Type: text/plain; charset="UTF-8"
 
 Sure thing! I can see if that works later tonight.
 
 One thing jumps out at me, though: it doesn't seem valid to redefine the
 CFA like that? As I understand it, the CFA on amd64 is defined to be the
 stack pointer in the caller's frame at the time of the call, and you're not
 allowed to define it otherwise. The reason to use multiple .cfi_def_cfa
 directives in one function would be to update the CFA *computation* to
 account for changes to the variables used in the computation (e.g. RSP),
 but not to actually change the CFA itself.
 
 That said, it's possible that changing the CFA as you propose works just
 fine in practice. Someone more knowledgeable than me would be better
 equipped to say for sure.
 
 I should note that I am far from an expert on this topic. Everything I know
 about unwind tables has been pieced together in the last two days while
 trying to fix this gccgo test.
 
 On Sun, Oct 11, 2020 at 8:46 PM Kamil Rytarowski <kamil%netbsd.org@localhost> wrote:
 
 > On 12.10.2020 01:54, kamil%NetBSD.org@localhost wrote:
 > > Synopsis: Unwind tables for signal trampoline on amd64 are incorrect
 > >
 > > Responsible-Changed-From-To: lib-bug-people->kamil
 > > Responsible-Changed-By: kamil%NetBSD.org@localhost
 > > Responsible-Changed-When: Mon, 12 Oct 2020 01:54:22 +0200
 > > Responsible-Changed-Why:
 > > Take.
 > >
 > >
 > >
 >
 > We ended up with something like this, but ran out of time:
 >
 > http://netbsd.org/~kamil/patch-00283-amd64-__sigtramp2-cfi-unwinding.txt
 >
 > Could you please check this code?
 >
 > This approach was proposed/inspired by Andrew Cagney and written by myself.
 >
 > We could combine this with the genassym.cf approach.
 >
 >
 
 --0000000000009eda3505b16f1257
 Content-Type: text/html; charset="UTF-8"
 Content-Transfer-Encoding: quoted-printable
 
 <div dir=3D"ltr"><div dir=3D"ltr">Sure thing! I can see if that works later=
  tonight.<div><br></div><div>One thing jumps out at me, though: it doesn&#3=
 9;t seem valid to redefine the CFA like that? As I understand it, the CFA o=
 n amd64 is defined to be the stack pointer in the caller&#39;s frame at the=
  time of the call, and you&#39;re not allowed to define it otherwise. The r=
 eason to use multiple .cfi_def_cfa directives in one function would be to u=
 pdate the CFA *computation* to account for changes to the variables used in=
  the computation (e.g. RSP), but not to actually change the CFA itself.</di=
 v><div><br></div><div>That said, it&#39;s possible that changing the CFA as=
  you propose=C2=A0works just fine in practice. Someone more knowledgeable t=
 han me would be better equipped to say for sure.</div><div><br></div><div>I=
  should note that I am far from an expert on this topic. Everything I know =
 about unwind tables has been pieced together in the last two days while try=
 ing to fix this gccgo test.</div></div></div><br><div class=3D"gmail_quote"=
 ><div dir=3D"ltr" class=3D"gmail_attr">On Sun, Oct 11, 2020 at 8:46 PM Kami=
 l Rytarowski &lt;<a href=3D"mailto:kamil%netbsd.org@localhost"; target=3D"_blank">kami=
 l%netbsd.org@localhost</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" styl=
 e=3D"margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid=
 ;border-left-color:rgb(204,204,204);padding-left:1ex">On 12.10.2020 01:54, =
 kamil%NetBSD.org@localhost wrote:<br>
 &gt; Synopsis: Unwind tables for signal trampoline on amd64 are incorrect<b=
 r>
 &gt; <br>
 &gt; Responsible-Changed-From-To: lib-bug-people-&gt;kamil<br>
 &gt; Responsible-Changed-By: kamil%NetBSD.org@localhost<br>
 &gt; Responsible-Changed-When: Mon, 12 Oct 2020 01:54:22 +0200<br>
 &gt; Responsible-Changed-Why:<br>
 &gt; Take.<br>
 &gt; <br>
 &gt; <br>
 &gt; <br>
 <br>
 We ended up with something like this, but ran out of time:<br>
 <br>
 <a href=3D"http://netbsd.org/~kamil/patch-00283-amd64-__sigtramp2-cfi-unwin=
 ding.txt" rel=3D"noreferrer" target=3D"_blank">http://netbsd.org/~kamil/pat=
 ch-00283-amd64-__sigtramp2-cfi-unwinding.txt</a><br>
 <br>
 Could you please check this code?<br>
 <br>
 This approach was proposed/inspired by Andrew Cagney and written by myself.=
 <br>
 <br>
 We could combine this with the <a href=3D"http://genassym.cf"; rel=3D"norefe=
 rrer" target=3D"_blank">genassym.cf</a> approach.<br>
 <br>
 </blockquote></div>
 
 --0000000000009eda3505b16f1257--