Re: [PATCH 3/3] arm64: vdso: Fix CFI directives in sigreturn trampoline

From: Will Deacon <will@kernel.org>
To: Dave Martin <Dave.Martin@arm.com>
Cc: Tamas Zsoldos <Tamas.Zsoldos@arm.com>,
	Mark Brown <broonie@kernel.org>,
	"kernel-team@android.com" <kernel-team@android.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Daniel Kiss <Daniel.Kiss@arm.com>
Subject: Re: [PATCH 3/3] arm64: vdso: Fix CFI directives in sigreturn trampoline
Date: Tue, 19 May 2020 16:24:54 +0100	[thread overview]
Message-ID: <20200519152453.GA15811@willie-the-truck> (raw)
In-Reply-To: <20200519135537.GG5031@arm.com>

On Tue, May 19, 2020 at 02:55:37PM +0100, Dave Martin wrote:
> On Tue, May 19, 2020 at 02:39:41PM +0100, Will Deacon wrote:
> > The gas docs say:
> > 
> > 	"Mark current function as signal trampoline."
> > 
> > which is really informative.
> 
> Well, we've demonstrated that identifying the signal frame is a gross
> bodge.  The cfi annotation should provide a reliable way to identify the
> signal frame, but I guess it was too poorly specified or came too late
> to prevent the bodges from spreading.
> 
> Since this seems to be a nonstandard invention, I wouldn't hold out
> much hope of finding a usable spec.
> 
> Of course, something might be using it now, so I guess we have to leave
> it.

I had a quick look at libstdc++ (the horror!) and it looks like the DWARF
backend in there /does/ make use of this information as part of the
_Unwind_GetIPInfo() function:

https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/baselib--unwind-getipinfo.html

*ip_before_insn is set to 1 or 0 depending on whether or not the PC
corresponds to a function annotated with .cfi_signal_frame. So I think
the code in libstdc++-v3/libsupc++/eh_personality.cc doesn't need the
mysterious NOP at all!

Unfortunately, it looks like the LLVM libc++ doesn't use this, and instead
calls _Unwind_GetIP():

https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/baselib--unwind-getip.html

and unconditionally subtracts 1 in libcxxabi/src/cxa_personality.cpp,
meaning that the NOP is necessary.

So, after giving myself a splitting headache, it looks like:

  1. We need the mysterious NOP for LLVM
  2. We could drop .cfi_signal_frame but it's harmless to keep it
  3. We need the .cfi_def_cfa directive to locate the frame record, as
     .cfi_signal_frame doesn't do very much at all.

Make sense? If so, I'll spin a v2 of the patches along with a comment
trying to summarise some of this.

Cheers,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel