[libc-commits] [libc] [libc][setjmp] fix setjmp test (PR #88054)
Nick Desaulniers via libc-commits
libc-commits at lists.llvm.org
Tue Apr 9 14:29:23 PDT 2024
https://github.com/nickdesaulniers updated https://github.com/llvm/llvm-project/pull/88054
>From 9e95992f9ac7c5ed3d53cd720626e791e2b07004 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Fri, 5 Apr 2024 15:40:55 -0700
Subject: [PATCH 1/4] [libc][setjmp] fix setjmp test
This would consistently fail for me locally, to the point where I could not run
`ninja libc-unit-tests` without `ninja libc_setjmp_unittests` failing.
Turns out that since I enabled -ftrivial-auto-var-init=pattern in
commit 1d5c16d78058 ("[libc] default enable -ftrivial-auto-var-init=pattern (#78776)")
this has been a problem. Our x86_64 setjmp definition disabled -Wuninitialized,
so we wound up clobbering these registers and instead backing up
0xAAAAAAAAAAAAAAAA rather than the actual register value.
---
libc/src/setjmp/x86_64/setjmp.cpp | 23 +++++++----------------
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/libc/src/setjmp/x86_64/setjmp.cpp b/libc/src/setjmp/x86_64/setjmp.cpp
index 8b6981d4cc34a2..bbc08d42bfd617 100644
--- a/libc/src/setjmp/x86_64/setjmp.cpp
+++ b/libc/src/setjmp/x86_64/setjmp.cpp
@@ -16,22 +16,13 @@
namespace LIBC_NAMESPACE {
LLVM_LIBC_FUNCTION(int, setjmp, (__jmp_buf * buf)) {
- register __UINT64_TYPE__ rbx __asm__("rbx");
- register __UINT64_TYPE__ r12 __asm__("r12");
- register __UINT64_TYPE__ r13 __asm__("r13");
- register __UINT64_TYPE__ r14 __asm__("r14");
- register __UINT64_TYPE__ r15 __asm__("r15");
-
- // We want to store the register values as is. So, we will suppress the
- // compiler warnings about the uninitialized variables declared above.
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wuninitialized"
- LIBC_INLINE_ASM("mov %1, %0\n\t" : "=m"(buf->rbx) : "r"(rbx) :);
- LIBC_INLINE_ASM("mov %1, %0\n\t" : "=m"(buf->r12) : "r"(r12) :);
- LIBC_INLINE_ASM("mov %1, %0\n\t" : "=m"(buf->r13) : "r"(r13) :);
- LIBC_INLINE_ASM("mov %1, %0\n\t" : "=m"(buf->r14) : "r"(r14) :);
- LIBC_INLINE_ASM("mov %1, %0\n\t" : "=m"(buf->r15) : "r"(r15) :);
-#pragma GCC diagnostic pop
+ asm("mov %%rbx, %[rbx]\n\t"
+ "mov %%r12, %[r12]\n\t"
+ "mov %%r13, %[r13]\n\t"
+ "mov %%r14, %[r14]\n\t"
+ "mov %%r15, %[r15]"
+ : [rbx] "=m"(buf->rbx), [r12] "=m"(buf->r12), [r13] "=m"(buf->r13),
+ [r14] "=m"(buf->r14), [r15] "=m"(buf->r15));
// We want the rbp of the caller, which is what __builtin_frame_address(1)
// should return. But, compilers generate a warning that calling
>From 56505cb5281beb94cea5b6146e2aeb5df8962fa1 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Mon, 8 Apr 2024 14:48:34 -0700
Subject: [PATCH 2/4] use naked attribute to implement setjmp
---
libc/src/setjmp/x86_64/CMakeLists.txt | 3 +-
libc/src/setjmp/x86_64/setjmp.cpp | 49 +++++++++++++--------------
2 files changed, 25 insertions(+), 27 deletions(-)
diff --git a/libc/src/setjmp/x86_64/CMakeLists.txt b/libc/src/setjmp/x86_64/CMakeLists.txt
index 9899c00e7c4a65..d247dc7bda8ded 100644
--- a/libc/src/setjmp/x86_64/CMakeLists.txt
+++ b/libc/src/setjmp/x86_64/CMakeLists.txt
@@ -8,7 +8,8 @@ add_entrypoint_object(
libc.include.setjmp
COMPILE_OPTIONS
-O3
- -fno-omit-frame-pointer
+ -fomit-frame-pointer
+ -momit-leaf-frame-pointer
)
add_entrypoint_object(
diff --git a/libc/src/setjmp/x86_64/setjmp.cpp b/libc/src/setjmp/x86_64/setjmp.cpp
index bbc08d42bfd617..1380777b8e81af 100644
--- a/libc/src/setjmp/x86_64/setjmp.cpp
+++ b/libc/src/setjmp/x86_64/setjmp.cpp
@@ -6,6 +6,7 @@
//
//===----------------------------------------------------------------------===//
+#include "include/llvm-libc-macros/offsetof-macro.h"
#include "src/__support/common.h"
#include "src/setjmp/setjmp_impl.h"
@@ -15,33 +16,29 @@
namespace LIBC_NAMESPACE {
+__attribute__((naked))
LLVM_LIBC_FUNCTION(int, setjmp, (__jmp_buf * buf)) {
- asm("mov %%rbx, %[rbx]\n\t"
- "mov %%r12, %[r12]\n\t"
- "mov %%r13, %[r13]\n\t"
- "mov %%r14, %[r14]\n\t"
- "mov %%r15, %[r15]"
- : [rbx] "=m"(buf->rbx), [r12] "=m"(buf->r12), [r13] "=m"(buf->r13),
- [r14] "=m"(buf->r14), [r15] "=m"(buf->r15));
-
- // We want the rbp of the caller, which is what __builtin_frame_address(1)
- // should return. But, compilers generate a warning that calling
- // __builtin_frame_address with non-zero argument is unsafe. So, we use
- // the knowledge of the x86_64 ABI to fetch the callers rbp. As per the ABI,
- // the rbp of the caller is pushed on to the stack and then new top is saved
- // in this function's rbp. So, we fetch it from location at which this
- // functions's rbp is pointing.
- buf->rbp = *reinterpret_cast<__UINTPTR_TYPE__ *>(__builtin_frame_address(0));
-
- // The callers stack address is exactly 2 pointer widths ahead of the current
- // frame pointer - between the current frame pointer and the rsp of the caller
- // are the return address (pushed by the x86_64 call instruction) and the
- // previous stack pointer as required by the x86_64 ABI.
- // The stack pointer is ahead because the stack grows down on x86_64.
- buf->rsp = reinterpret_cast<__UINTPTR_TYPE__>(__builtin_frame_address(0)) +
- sizeof(__UINTPTR_TYPE__) * 2;
- buf->rip = reinterpret_cast<__UINTPTR_TYPE__>(__builtin_return_address(0));
- return 0;
+ asm("mov %%rbx, %c[rbx](%%rdi)\n\t"
+ "mov %%rbp, %c[rbp](%%rdi)\n\t"
+ "mov %%r12, %c[r12](%%rdi)\n\t"
+ "mov %%r13, %c[r13](%%rdi)\n\t"
+ "mov %%r14, %c[r14](%%rdi)\n\t"
+ "mov %%r15, %c[r15](%%rdi)\n\t"
+
+ "lea 8(%%rsp), %%rax\n\t"
+ "mov %%rax, %c[rsp](%%rdi)\n\t"
+
+ "mov %[read_rip], %c[rip](%%rdi)\n\t"
+
+ "xorl %%eax, %%eax\n\t"
+ "retq"
+ ::
+ [rbx] "i"(offsetof(__jmp_buf, rbx)), [r12] "i"(offsetof(__jmp_buf, r12)),
+ [r13] "i"(offsetof(__jmp_buf, r13)), [r14] "i"(offsetof(__jmp_buf, r14)),
+ [r15] "i"(offsetof(__jmp_buf, r15)), [rbp] "i"(offsetof(__jmp_buf, rbp)),
+ [rsp] "i"(offsetof(__jmp_buf, rsp)), [rip] "i"(offsetof(__jmp_buf, rip)),
+ [read_rip] "r" (reinterpret_cast<__UINTPTR_TYPE__>(__builtin_return_address(0)))
+ : "rax");
}
} // namespace LIBC_NAMESPACE
>From d58bc75990995fdef6b67c1989da4a0c9431e15f Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Tue, 9 Apr 2024 14:21:19 -0700
Subject: [PATCH 3/4] avoid __builtin_return_address
---
libc/src/setjmp/x86_64/setjmp.cpp | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/libc/src/setjmp/x86_64/setjmp.cpp b/libc/src/setjmp/x86_64/setjmp.cpp
index 1380777b8e81af..f000188b4ad07c 100644
--- a/libc/src/setjmp/x86_64/setjmp.cpp
+++ b/libc/src/setjmp/x86_64/setjmp.cpp
@@ -28,16 +28,15 @@ LLVM_LIBC_FUNCTION(int, setjmp, (__jmp_buf * buf)) {
"lea 8(%%rsp), %%rax\n\t"
"mov %%rax, %c[rsp](%%rdi)\n\t"
- "mov %[read_rip], %c[rip](%%rdi)\n\t"
+ "mov (%%rsp), %%rax\n\t"
+ "mov %%rax, %c[rip](%%rdi)\n\t"
"xorl %%eax, %%eax\n\t"
- "retq"
- ::
- [rbx] "i"(offsetof(__jmp_buf, rbx)), [r12] "i"(offsetof(__jmp_buf, r12)),
+ "retq" ::[rbx] "i"(offsetof(__jmp_buf, rbx)),
+ [rbp] "i"(offsetof(__jmp_buf, rbp)), [r12] "i"(offsetof(__jmp_buf, r12)),
[r13] "i"(offsetof(__jmp_buf, r13)), [r14] "i"(offsetof(__jmp_buf, r14)),
- [r15] "i"(offsetof(__jmp_buf, r15)), [rbp] "i"(offsetof(__jmp_buf, rbp)),
- [rsp] "i"(offsetof(__jmp_buf, rsp)), [rip] "i"(offsetof(__jmp_buf, rip)),
- [read_rip] "r" (reinterpret_cast<__UINTPTR_TYPE__>(__builtin_return_address(0)))
+ [r15] "i"(offsetof(__jmp_buf, r15)), [rsp] "i"(offsetof(__jmp_buf, rsp)),
+ [rip] "i"(offsetof(__jmp_buf, rip))
: "rax");
}
>From b60a733693cbc492a8233fff55e69ac9c21ee8ce Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Tue, 9 Apr 2024 14:29:11 -0700
Subject: [PATCH 4/4] damned linter
---
libc/src/setjmp/x86_64/setjmp.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/libc/src/setjmp/x86_64/setjmp.cpp b/libc/src/setjmp/x86_64/setjmp.cpp
index f000188b4ad07c..e7f1807dd825a2 100644
--- a/libc/src/setjmp/x86_64/setjmp.cpp
+++ b/libc/src/setjmp/x86_64/setjmp.cpp
@@ -16,8 +16,7 @@
namespace LIBC_NAMESPACE {
-__attribute__((naked))
-LLVM_LIBC_FUNCTION(int, setjmp, (__jmp_buf * buf)) {
+__attribute__((naked)) LLVM_LIBC_FUNCTION(int, setjmp, (__jmp_buf * buf)) {
asm("mov %%rbx, %c[rbx](%%rdi)\n\t"
"mov %%rbp, %c[rbp](%%rdi)\n\t"
"mov %%r12, %c[r12](%%rdi)\n\t"
More information about the libc-commits
mailing list