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

[Will Deacon <will@kernel.org>]

On Tue, May 19, 2020 at 03:30:57PM +0000, Daniel Kiss wrote:
> > On 19 May 2020, at 15:55, Dave Martin <Dave.Martin@arm.com> wrote:
> > On Tue, May 19, 2020 at 02:39:41PM +0100, Will Deacon wrote:
> >> On Tue, May 19, 2020 at 02:09:31PM +0100, Dave P Martin wrote:
> >>> On Tue, May 19, 2020 at 01:18:18PM +0100, Will Deacon wrote:
> >>>> Daniel reports that the .cfi_startproc is misplaced for the sigreturn
> >>>> trampoline, which causes LLVM's unwinder to misbehave:
> >>>> 
> >>>>  | I run into this with LLVM’s unwinder.
> >>>>  | This combination was always broken.
> >>>> 
> >>>> This prompted Dave to realise that our CFI directives are contradictory,
> >>>> as we specify both .cfi_signal_frame *and* .cfi_def_cfa, with the latter
> >>>> unconditionally identifying the interrupted context as opposed to the
> >>>> values in the sigcontext.
> >>>> 
> >>>> Rework the CFI directives so that we only use .cfi_signal_frame, and
> >>>> include the "mysterious NOP" as part of the .cfi_{start,end}proc block.
> >>>> 
> >>>> Cc: Tamas Zsoldos <tamas.zsoldos@arm.com>
> >>>> Reported-by: Dave Martin <dave.martin@arm.com>
> >>>> Reported-by: Daniel Kiss <daniel.kiss@arm.com>
> >>>> Signed-off-by: Will Deacon <will@kernel.org>
> >>>> ---
> >>>> arch/arm64/kernel/vdso/sigreturn.S | 8 +++-----
> >>>> 1 file changed, 3 insertions(+), 5 deletions(-)
> >>>> 
> >>>> diff --git a/arch/arm64/kernel/vdso/sigreturn.S b/arch/arm64/kernel/vdso/sigreturn.S
> >>>> index 7853fa9692f6..28b33f7d0604 100644
> >>>> --- a/arch/arm64/kernel/vdso/sigreturn.S
> >>>> +++ b/arch/arm64/kernel/vdso/sigreturn.S
> >>>> @@ -14,6 +14,9 @@
> >>>> 
> >>>> 	.text
> >>>> 
> >>>> +/* Ensure that the mysterious NOP can be associated with a function. */
> >>>> +	.cfi_startproc
> >>>> +	.cfi_signal_frame
> >>>> /*
> >>>>  * This mysterious NOP is required for some unwinders that subtract one from
> >>>>  * the return address in order to identify the calling function.
> >>>> @@ -28,11 +31,6 @@
> >>>>  * is perfectly fine.
> >>>>  */
> >>>> SYM_START(__kernel_rt_sigreturn, SYM_L_GLOBAL, SYM_A_ALIGN)
> >>>> -	.cfi_startproc
> >>>> -	.cfi_signal_frame
> >>>> -	.cfi_def_cfa	x29, 0
> >>>> -	.cfi_offset	x29, 0 * 8
> >>>> -	.cfi_offset	x30, 1 * 8
> LLVM’s unwinder does not like this version of the CFI. It needs a bit more information, 
> the cfi_signal_frame is not used for finding the frame.

Thanks, Daniel. That is, at least, aligned with my current understanding
of how this is supposed to work.

I'll send out a v2 in a bit.

Will

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