[PATCH v2 2/2] arm64: vdso: Fix CFI directives in sigreturn trampoline

From: Will Deacon <will@kernel•org>
To: linux-arm-kernel@lists•infradead.org
Cc: Will Deacon <will@kernel•org>,
	Tamas Zsoldos <tamas.zsoldos@arm•com>,
	Mark Brown <broonie@kernel•org>,
	kernel-team@android•com, Dave Martin <dave.martin@arm•com>,
	Daniel Kiss <daniel.kiss@arm•com>
Subject: [PATCH v2 2/2] arm64: vdso: Fix CFI directives in sigreturn trampoline
Date: Tue, 19 May 2020 17:28:21 +0100	[thread overview]
Message-ID: <20200519162821.16857-3-will@kernel.org> (raw)
In-Reply-To: <20200519162821.16857-1-will@kernel.org>

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 question our use of CFI directives more generally,
and I ended up going down a rabbit hole trying to figure out how this
very poorly documented stuff gets used.

Move the CFI directives so that the "mysterious NOP" is included in
the .cfi_{start,end}proc block and add a bunch of comments so that I
can save myself another headache in future.

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 | 40 ++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kernel/vdso/sigreturn.S b/arch/arm64/kernel/vdso/sigreturn.S
index 0c921130002a..cb47dfb3bd5a 100644
--- a/arch/arm64/kernel/vdso/sigreturn.S
+++ b/arch/arm64/kernel/vdso/sigreturn.S
@@ -1,7 +1,11 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
  * Sigreturn trampoline for returning from a signal when the SA_RESTORER
- * flag is not set.
+ * flag is not set. It serves primarily as a hall of shame for crappy
+ * unwinders and features an exciting but mysterious NOP instruction.
+ *
+ * It's also fragile as hell, so please think twice before changing anything
+ * in here.
  *
  * Copyright (C) 2012 ARM Limited
  *
@@ -14,7 +18,34 @@
 
 	.text
 
-	nop
+/* Ensure that the mysterious NOP can be associated with a function. */
+	.cfi_startproc
+
+/*
+ * .cfi_signal_frame causes the corresponding Frame Description Entry in the
+ * .eh_frame section to be annotated as a signal frame. This allows DWARF
+ * unwinders (e.g. libstdc++) to implement _Unwind_GetIPInfo(), which permits
+ * unwinding out of the signal trampoline without the need for the mysterious
+ * NOP.
+ */
+	.cfi_signal_frame
+
+/*
+ * Tell the unwinder where to locate the frame record linking back to the
+ * interrupted context.
+ */
+	.cfi_def_cfa    x29, 0
+	.cfi_offset     x29, 0 * 8
+	.cfi_offset     x29, 1 * 8
+
+/*
+ * This mysterious NOP is required for some unwinders (e.g. libc++) that
+ * unconditionally subtract one from the result of _Unwind_GetIP() in order to
+ * identify the calling function.
+ * Hack borrowed from arch/powerpc/kernel/vdso64/sigtramp.S.
+ */
+	nop	// Mysterious NOP
+
 /*
  * GDB relies on being able to identify the sigreturn instruction sequence to
  * unwind from signal handlers. We cannot, therefore, use SYM_FUNC_START()
@@ -23,11 +54,6 @@
  * is perfectly fine.
  */
 SYM_CODE_START(__kernel_rt_sigreturn)
-	.cfi_startproc
-	.cfi_signal_frame
-	.cfi_def_cfa	x29, 0
-	.cfi_offset	x29, 0 * 8
-	.cfi_offset	x30, 1 * 8
 	mov	x8, #__NR_rt_sigreturn
 	svc	#0
 	.cfi_endproc
-- 
2.26.2.761.g0e0b3e54be-goog


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