[libc-commits] [libc] [libc][setjmp] fix setjmp test via naked fn attr (PR #88054)
Nick Desaulniers via libc-commits
libc-commits at lists.llvm.org
Thu Oct 10 11:02:07 PDT 2024
https://github.com/nickdesaulniers updated https://github.com/llvm/llvm-project/pull/88054
>From 7ffa468309e066d039b550b3ef1b5a535912381b 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/7] [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 | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/libc/src/setjmp/x86_64/setjmp.cpp b/libc/src/setjmp/x86_64/setjmp.cpp
index 62d9c13c68e4b6..1cbff4c5eff57f 100644
--- a/libc/src/setjmp/x86_64/setjmp.cpp
+++ b/libc/src/setjmp/x86_64/setjmp.cpp
@@ -16,23 +16,14 @@
namespace LIBC_NAMESPACE_DECL {
-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
+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
>From 352824b7dc95109002e39dfd1a9e2731805c4c90 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/7] use naked attribute to implement setjmp
---
libc/src/setjmp/x86_64/CMakeLists.txt | 8 ++---
libc/src/setjmp/x86_64/setjmp.cpp | 49 +++++++++++++--------------
2 files changed, 25 insertions(+), 32 deletions(-)
diff --git a/libc/src/setjmp/x86_64/CMakeLists.txt b/libc/src/setjmp/x86_64/CMakeLists.txt
index c789e5def7fe79..4f882a84405d87 100644
--- a/libc/src/setjmp/x86_64/CMakeLists.txt
+++ b/libc/src/setjmp/x86_64/CMakeLists.txt
@@ -8,12 +8,8 @@ add_entrypoint_object(
libc.hdr.types.jmp_buf
COMPILE_OPTIONS
-O3
- -fno-omit-frame-pointer
- # TODO: Remove once one of these lands:
- # https://github.com/llvm/llvm-project/pull/87837
- # https://github.com/llvm/llvm-project/pull/88054
- # https://github.com/llvm/llvm-project/pull/88157
- -ftrivial-auto-var-init=uninitialized
+ -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 1cbff4c5eff57f..89411450106d65 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/__support/macros/config.h"
#include "src/setjmp/setjmp_impl.h"
@@ -16,33 +17,29 @@
namespace LIBC_NAMESPACE_DECL {
+__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_DECL
>From 443e8aad4d9e8fa55be1d1a3abd848876add50dc 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/7] 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 89411450106d65..f16a0722c4e046 100644
--- a/libc/src/setjmp/x86_64/setjmp.cpp
+++ b/libc/src/setjmp/x86_64/setjmp.cpp
@@ -29,16 +29,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 bc80aec6322eb5d58112cb7a2db523e37edb8b17 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/7] 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 f16a0722c4e046..1d7722aa4aaeae 100644
--- a/libc/src/setjmp/x86_64/setjmp.cpp
+++ b/libc/src/setjmp/x86_64/setjmp.cpp
@@ -17,8 +17,7 @@
namespace LIBC_NAMESPACE_DECL {
-__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"
>From 7e1a505e86ec83e4703fb2a8adf7d1467dc9b0d6 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Thu, 20 Jun 2024 10:12:48 -0700
Subject: [PATCH 5/7] use [[]] attribute syntax
---
libc/src/setjmp/x86_64/setjmp.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libc/src/setjmp/x86_64/setjmp.cpp b/libc/src/setjmp/x86_64/setjmp.cpp
index 1d7722aa4aaeae..3c11edf4dfee21 100644
--- a/libc/src/setjmp/x86_64/setjmp.cpp
+++ b/libc/src/setjmp/x86_64/setjmp.cpp
@@ -17,7 +17,8 @@
namespace LIBC_NAMESPACE_DECL {
-__attribute__((naked)) LLVM_LIBC_FUNCTION(int, setjmp, (__jmp_buf * buf)) {
+[[gnu::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"
>From 0dec13e70debb5fd4fa8705b4e7045df6cdd95dd Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Thu, 10 Oct 2024 09:54:29 -0700
Subject: [PATCH 6/7] remove omit frame pointer command line flags
---
libc/src/setjmp/x86_64/CMakeLists.txt | 2 --
1 file changed, 2 deletions(-)
diff --git a/libc/src/setjmp/x86_64/CMakeLists.txt b/libc/src/setjmp/x86_64/CMakeLists.txt
index 4f882a84405d87..b5b0d9ba65599c 100644
--- a/libc/src/setjmp/x86_64/CMakeLists.txt
+++ b/libc/src/setjmp/x86_64/CMakeLists.txt
@@ -8,8 +8,6 @@ add_entrypoint_object(
libc.hdr.types.jmp_buf
COMPILE_OPTIONS
-O3
- -fomit-frame-pointer
- -momit-leaf-frame-pointer
)
add_entrypoint_object(
>From 734e044d3c76db2c9bb4b2b5ac8b0ee165ccffb4 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Thu, 10 Oct 2024 09:56:43 -0700
Subject: [PATCH 7/7] use raw string literal
---
libc/src/setjmp/x86_64/setjmp.cpp | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/libc/src/setjmp/x86_64/setjmp.cpp b/libc/src/setjmp/x86_64/setjmp.cpp
index 3c11edf4dfee21..c9ca578fb1e6db 100644
--- a/libc/src/setjmp/x86_64/setjmp.cpp
+++ b/libc/src/setjmp/x86_64/setjmp.cpp
@@ -19,21 +19,22 @@ namespace LIBC_NAMESPACE_DECL {
[[gnu::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"
- "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 (%%rsp), %%rax\n\t"
- "mov %%rax, %c[rip](%%rdi)\n\t"
-
- "xorl %%eax, %%eax\n\t"
- "retq" ::[rbx] "i"(offsetof(__jmp_buf, rbx)),
+ asm(R"(
+ mov %%rbx, %c[rbx](%%rdi)
+ mov %%rbp, %c[rbp](%%rdi)
+ mov %%r12, %c[r12](%%rdi)
+ mov %%r13, %c[r13](%%rdi)
+ mov %%r14, %c[r14](%%rdi)
+ mov %%r15, %c[r15](%%rdi)
+
+ lea 8(%%rsp), %%rax
+ mov %%rax, %c[rsp](%%rdi)
+
+ mov (%%rsp), %%rax
+ mov %%rax, %c[rip](%%rdi)
+
+ xorl %%eax, %%eax
+ 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)), [rsp] "i"(offsetof(__jmp_buf, rsp)),
More information about the libc-commits
mailing list