[libc][arm] implement a basic setjmp/longjmp #93220
Conversation
Note: our baremetal arm configuration compiles this as `--target=arm-none-eabi`, so this code is built in -marm mode. It could be smaller with `--target=armv7-none-eabi -mthumb`. The assembler is valid ARMv5, or THUMB2, but not THUMB(1). Unclear yet if we need to support THUMB(1); may depend on whether downstream users are using llvm-libc's cmake.
|
@llvm/pr-subscribers-libc Author: Nick Desaulniers (paternity leave) (nickdesaulniers) ChangesNote: our baremetal arm configuration compiles this as Full diff: https://github.com/llvm/llvm-project/pull/93220.diff 9 Files Affected:
diff --git a/libc/config/baremetal/arm/entrypoints.txt b/libc/config/baremetal/arm/entrypoints.txt
index 4e3d1cb9f5337..ff192edfffe3d 100644
--- a/libc/config/baremetal/arm/entrypoints.txt
+++ b/libc/config/baremetal/arm/entrypoints.txt
@@ -26,6 +26,10 @@ set(TARGET_LIBC_ENTRYPOINTS
# errno.h entrypoints
libc.src.errno.errno
+ # setjmp.h entrypoints
+ libc.src.setjmp.longjmp
+ libc.src.setjmp.setjmp
+
# string.h entrypoints
libc.src.string.bcmp
libc.src.string.bcopy
diff --git a/libc/config/baremetal/arm/headers.txt b/libc/config/baremetal/arm/headers.txt
index 3608364e45bde..28fd1ce9128d2 100644
--- a/libc/config/baremetal/arm/headers.txt
+++ b/libc/config/baremetal/arm/headers.txt
@@ -1,13 +1,14 @@
set(TARGET_PUBLIC_HEADERS
libc.include.assert
libc.include.ctype
- libc.include.fenv
libc.include.errno
+ libc.include.fenv
libc.include.float
- libc.include.stdint
libc.include.inttypes
libc.include.math
+ libc.include.setjmp
libc.include.stdfix
+ libc.include.stdint
libc.include.stdio
libc.include.stdlib
libc.include.string
diff --git a/libc/config/linux/arm/entrypoints.txt b/libc/config/linux/arm/entrypoints.txt
index 335981ff7dc7c..af824d294d7d7 100644
--- a/libc/config/linux/arm/entrypoints.txt
+++ b/libc/config/linux/arm/entrypoints.txt
@@ -20,6 +20,10 @@ set(TARGET_LIBC_ENTRYPOINTS
# errno.h entrypoints
libc.src.errno.errno
+ # setjmp.h entrypoints
+ libc.src.setjmp.longjmp
+ libc.src.setjmp.setjmp
+
# string.h entrypoints
libc.src.string.bcmp
libc.src.string.bcopy
diff --git a/libc/config/linux/arm/headers.txt b/libc/config/linux/arm/headers.txt
index 1180564fe458c..84078a947031a 100644
--- a/libc/config/linux/arm/headers.txt
+++ b/libc/config/linux/arm/headers.txt
@@ -1,19 +1,20 @@
set(TARGET_PUBLIC_HEADERS
libc.include.ctype
- libc.include.fenv
libc.include.errno
+ libc.include.fenv
libc.include.float
- libc.include.stdint
libc.include.inttypes
libc.include.math
- libc.include.stdckdint
+ libc.include.search
+ libc.include.setjmp
libc.include.stdbit
+ libc.include.stdckdint
+ libc.include.stdint
libc.include.stdlib
libc.include.string
libc.include.strings
- libc.include.search
- libc.include.wchar
libc.include.uchar
+ libc.include.wchar
# Disabled due to epoll_wait syscalls not being available on this platform.
# libc.include.sys_epoll
diff --git a/libc/include/llvm-libc-types/jmp_buf.h b/libc/include/llvm-libc-types/jmp_buf.h
index 29a1df9ad6823..a5379336865de 100644
--- a/libc/include/llvm-libc-types/jmp_buf.h
+++ b/libc/include/llvm-libc-types/jmp_buf.h
@@ -32,6 +32,9 @@ typedef struct {
#elif defined(__riscv_float_abi_single)
#error "__jmp_buf not available for your target architecture."
#endif
+#elif defined(__arm__)
+ // r4, r5, r6, r7, r8, r9, r10, r11, r12, lr
+ long opaque [10];
#else
#error "__jmp_buf not available for your target architecture."
#endif
diff --git a/libc/include/setjmp.h.def b/libc/include/setjmp.h.def
index 670bc1ac0fe24..cb083b8cd023e 100644
--- a/libc/include/setjmp.h.def
+++ b/libc/include/setjmp.h.def
@@ -10,6 +10,7 @@
#define LLVM_LIBC_SETJMP_H
#include "__llvm-libc-common.h"
+#include "llvm-libc-types/jmp_buf.h"
%%public_api()
diff --git a/libc/src/setjmp/arm/CMakeLists.txt b/libc/src/setjmp/arm/CMakeLists.txt
new file mode 100644
index 0000000000000..da97b79c9fea0
--- /dev/null
+++ b/libc/src/setjmp/arm/CMakeLists.txt
@@ -0,0 +1,19 @@
+add_entrypoint_object(
+ setjmp
+ SRCS
+ setjmp.cpp
+ HDRS
+ ../setjmp_impl.h
+ DEPENDS
+ libc.include.setjmp
+)
+
+add_entrypoint_object(
+ longjmp
+ SRCS
+ longjmp.cpp
+ HDRS
+ ../longjmp.h
+ DEPENDS
+ libc.include.setjmp
+)
diff --git a/libc/src/setjmp/arm/longjmp.cpp b/libc/src/setjmp/arm/longjmp.cpp
new file mode 100644
index 0000000000000..752318edfa580
--- /dev/null
+++ b/libc/src/setjmp/arm/longjmp.cpp
@@ -0,0 +1,31 @@
+
+//===-- Implementation of longjmp -----------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/setjmp/longjmp.h"
+#include "src/__support/common.h"
+#include "src/__support/macros/properties/architectures.h"
+
+#if !defined(LIBC_TARGET_ARCH_IS_ARM)
+#error "Invalid file include"
+#endif
+
+namespace LIBC_NAMESPACE {
+
+[[gnu::naked]]
+LLVM_LIBC_FUNCTION(void, longjmp, (__jmp_buf * buf, int val)) {
+ asm("ldm.w r0!, {r4, r5, r6, r7, r8, r9, r10, r11, r12, lr}\n\t"
+ "mov sp, r12\n\t"
+ "movs r0, r1\n\t"
+ "it eq\n\t"
+ "moveq r0, 1\n\t"
+ "bx lr"
+ );
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/setjmp/arm/setjmp.cpp b/libc/src/setjmp/arm/setjmp.cpp
new file mode 100644
index 0000000000000..9304ff6b47050
--- /dev/null
+++ b/libc/src/setjmp/arm/setjmp.cpp
@@ -0,0 +1,27 @@
+//===-- Implementation of setjmp ------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/__support/common.h"
+#include "src/setjmp/setjmp_impl.h"
+
+#if !defined(LIBC_TARGET_ARCH_IS_ARM)
+#error "Invalid file include"
+#endif
+
+namespace LIBC_NAMESPACE {
+
+[[gnu::naked]]
+LLVM_LIBC_FUNCTION(int, setjmp, (__jmp_buf * buf)) {
+ asm("mov r12, sp\n\t"
+ "stm.w r0!, {r4, r5, r6, r7, r8, r9, r10, r11, r12, lr}\n\t"
+ "mov.w r0, 0\n\t"
+ "bx lr\n\t"
+ );
+}
+
+} // namespace LIBC_NAMESPACE
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
This can be addressed separately later, but I think we need to rethink the approach to setjmp/longjmp in libc. In particular, I think implementations like this (and the jmp_buf ABI) cannot be universal per machine. This is a great implementation for bare metal, but not necessarily correct or sufficient for any given operating system. For POSIX systems, there's the issue of stack vs signal mask atomicity in (sig)longjmp. For various OS ABIs, there are other ABI elements to be concerned with, like shadow-call-stack or safestack ABIs. For various contexts there are additional concerns like pointer mangling. For various more complete ABIs, there are non-integer registers that need to be considered.
libc/src/setjmp/arm/longjmp.cpp
Outdated
|
|
||
| [[gnu::naked]] | ||
| LLVM_LIBC_FUNCTION(void, longjmp, (__jmp_buf * buf, int val)) { | ||
| asm("ldm.w r0!, {r4, r5, r6, r7, r8, r9, r10, r11, r12, lr}\n\t" |
There was a problem hiding this comment.
I think the norm is to use __asm__. Consider using a raw string with natural newlines.
The ! is not really useful here, since r0 is not used again.
The syntax allows {r4-r12, lr} which IMHO is more readable.
It's unusual to use the .w suffix rather than letting the assembler decide what encoding to use. I'd just use ldm r0, {r4-r12, lr} here.
There was a problem hiding this comment.
I think the norm is to use asm.
Ugly. No.
Consider using a raw string with natural newlines.
Ah, good idea. 96d8f14
The syntax allows {r4-r12, lr} which IMHO is more readable.
It's unusual to use the .w suffix rather than letting the assembler decide what encoding to use. I'd just use ldm r0, {r4-r12, lr} here.
Yes, these definitely can get way more complex. Perhaps I should retitle the PR to "implement simple setjmp/longjmp". |
There are two issues here:
|
|
Oh, I forgot there's machines that ONLY support Thumb(1) such as cortex-m0. cc @smithp35 I guess a fundamental question we should answer for llvm-libc is "we claim to support 32b ARM, but we don't specify which minimum target architecture." I think it would be handy to draw the line at armv6t2+, or even just armv7, but others (@petrhosek @smithp35 ) may feel differently? |
nickdesaulniers
requested review from
smithp35,
efriedma-quic,
lntue and
michaelrj-google
nickdesaulniers
changed the title
Cortex-M0+ is hugely popular, we use in several of our products (some of which already use LLVM libc) and it's also used by popular boards such as Raspberry Pi Pico, so I don't think we can realistically draw the line at Thumb2 unless we're fine excluding significant share of the embedded market. |
|
@petrhosek mentioned to me offline the need to support |
|
|
||
| #if defined(__thumb__) && __ARM_ARCH_ISA_THUMB == 1 | ||
|
|
||
| [[gnu::naked, gnu::target("thumb")]] |
There was a problem hiding this comment.
just curious, does arm/aarch64 also support multi-versioned symbols?
There was a problem hiding this comment.
If multi-versioned symbols is referring to function multi-versioning then there is a beta version defined in https://arm-software.github.io/acle/main/acle.html#function-multi-versioning AFAIK it has been concentrating on AArch64 though.
Unlikely to be used in bare-metal cases like cortex-m0 though.
There was a problem hiding this comment.
Right, depends on loader support, which we currently lack. Also, not sure yet we want to support a libc with mixed arm/thumb mode support. Frankly, I'd suspect that folks that care about libc support on 32b ARM only really care about Thumb mode (1 or 2) (unless they're running on some variant with only ARM mode support).
|
Apologies for the delay in responding, yes Thumb 1 only is important. There is also armv8-m.base (cortex-m23) which is the Arm v8 version of Thumb 1 only v6-m. I've asked our C-library expert if he can take a look at the Arm specific details. The main problem LLVM-libc will have v6-m is that it doesn't have any load/store exclusive instructions so it is unsuitable for multithreaded implementations. Strictly speaking interrupts can be disabled, but this is a privileged instruction. |
|
Not sure you're aware of it so I'll mention it anyway. There is some ABI documentation about requirements for Arm/Thumb setjmp/longjmp in the clib32 https://github.com/ARM-software/abi-aa/blob/main/clibabi32/clibabi32.rst#setjmph |
|
I know this is titled basic implementation, so these are likely some TODOs for a full implementation:
|
Thanks for the tips! 6bed9ef added some TODOs that I've filed. As @frobtech notes, there's even more that we can be doing here, too. Looking at Bionic's implementation, it quotes the AAPC32 to claim that only d8-d15 need be preserved? https://github.com/ARM-software/abi-aa/blob/main/aapcs32/aapcs32.rst#vfp-register-usage-conventions says d8-d15, q4-q7 though let's discuss that in #94061 |
|
right on, thanks for the review @statham-arm !! Would you have cycles to help review other 32b ARM related patches I may have in the near future? #93738 comes to mind. Also, I'll be on paternity leave for the next two weeks (on again, off again). |
I was not! Thanks for the reference. Here's some more TODO's I filed after reading that section: |
|
Thanks again for the reviews @statham-arm and @frobtech ! |
Note: our baremetal arm configuration compiles this as `--target=arm-none-eabi`, so this code is built in -marm mode. It could be smaller with `--target=armv7-none-eabi -mthumb`. The assembler is valid ARMv5, or THUMB2, but not THUMB(1).
Note: our baremetal arm configuration compiles this as
--target=arm-none-eabi, so this code is built in -marm mode. It could besmaller with
--target=armv7-none-eabi -mthumb. The assembler is valid ARMv5,or THUMB2, but not THUMB(1). Unclear yet if we need to support THUMB(1); may
depend on whether downstream users are using llvm-libc's cmake.