On Tue, Oct 5, 2021 at 5:22 AM Arnd Bergmann [off-list ref] wrote:

On Tue, Oct 5, 2021 at 9:15 AM Ard Biesheuvel [off-list ref] wrote:

quoted

Restructure the code and add the unwind annotations so that both the
frame pointer unwinder as well as the ELF unwind info based unwinder
will be able to follow the call stack through call_with_stack().

Note that the former cannot support GCC and Clang at the same time, as
they use a different idiom for the prologue/epilogue. So the code uses
the GCC idiom, adding full frame pointer based unwind support for GCC
while preserving the existing behavior of the Clang version, which
simply omits call_with_stack() from its call stack.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

I would like Nick to take a look at this for the clang support, he spent
some time on getting the frame pointer unwinder working with clang,
so he may have additional comments about this.

quoted

---
 arch/arm/lib/call_with_stack.S | 44 +++++++++++++++++---
 1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/arch/arm/lib/call_with_stack.S b/arch/arm/lib/call_with_stack.S
index 28b0341ae786..133dffa2404a 100644
--- a/arch/arm/lib/call_with_stack.S
+++ b/arch/arm/lib/call_with_stack.S

@@ -8,25 +8,57 @@

 #include <linux/linkage.h>
 #include <asm/assembler.h>
+#include <asm/unwind.h>

 /*
  * void call_with_stack(void (*fn)(void *), void *arg, void *sp)
  *
  * Change the stack to that pointed at by sp, then invoke fn(arg) with
  * the new stack.
+ *
+ * The sequence below follows the APCS frame convention for frame pointer
+ * unwinding, and implements the unwinder annotations needed by the EABI
+ * unwinder.
+ */
+
+#if defined(CONFIG_THUMB2_KERNEL) || \
+    (defined(CONFIG_UNWINDER_FRAME_POINTER) && defined(CONFIG_CC_IS_CLANG))

Doesn't clang use r11 (fp) as the frame pointer in ARM mode?
https://godbolt.org/z/1x4x99M1x
Or is this what you meant by "So the best we can do here is not touch
the frame pointer at all"?

quoted

+/*
+ * Thumb-2 builds must use R7 as the frame pointer due to the way our unwind
+ * info based unwinder is constructed.
+ *
+ * The code below uses the GCC idiom for managing the frame pointer in the
+ * function prologue and epilogue, which Clang does not support. So the best we

IIRC, it's only slightly different; it's just that FP points to the
previous FP in clang, rather than LR; at a fixed offset. At least when
looking through Doug's notes and diagrams:
https://lore.kernel.org/lkml/20210507135509.1.I5d969beafa0d7507f1e37fadaa6e4d88d428253d@changeid/ (local)
Though looking at the diagram, it looks like neither toolchain
implements APCS...did I understand that correctly?

There's also some documentation in
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/kernel/stacktrace.c#n11
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/lib/backtrace-clang.S#n31

I guess I'm more so curious about this code when built with clang,
both before and after this patch.  Was it broken for either unwinder
on ARM or THUMB2+UNWINDER_ARM?  Does it regress with this patch?
What's the best way to test/verify this?

quoted

+ * can do here is not touch the frame pointer at all: this will simply omit
+ * this frame when unwinding the call stack. So use R7 in this case as well,
+ * and leave R11 unmodified.
  */
+       fpreg   .req    r7

TIL about `.req`: https://sourceware.org/binutils/docs/as/ARM-Directives.html
This patch demonstrates the usage of quite a few of these!

quoted

+#else
+       fpreg   .req    fp
+#endif
+
 ENTRY(call_with_stack)
-       str     sp, [r2, #-4]!
-       str     lr, [r2, #-4]!
+UNWIND(        .fnstart                )
+UNWIND(        .movsp  ip              )
+       mov     ip, sp
+
+UNWIND(        .pad    #4              )
+UNWIND(        .save   {fpreg, ip, lr} )
+THUMB( sub     sp, #4          )
+       push    {fpreg, ip, lr ARM(, pc)}
+
+UNWIND(        .setfp  fpreg, ip, #-4  )
+       sub     fpreg, ip, #4

        mov     sp, r2
        mov     r2, r0
        mov     r0, r1

-       badr    lr, 1f
-       ret     r2
+       bl_r    r2

-1:     ldr     lr, [sp]
-       ldr     sp, [sp, #4]
+       ldmdb   fpreg, {fpreg, ip, lr}
+       mov     sp, ip
        ret     lr
+UNWIND(        .fnend                  )
 ENDPROC(call_with_stack)
--
2.30.2

-- 
Thanks,
~Nick Desaulniers

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