[libc-commits] [libc] [libc][setjmp] fix setjmp test (PR #88054)

[via libc-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

<details>
<summary>Changes</summary>

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 1d5c16d ("[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.

Use `naked` function attribute to avoid function prolog/epilog.


---
Full diff: https://github.com/llvm/llvm-project/pull/88054.diff


2 Files Affected:

- (modified) libc/src/setjmp/x86_64/CMakeLists.txt (+2-1) 
- (modified) libc/src/setjmp/x86_64/setjmp.cpp (+23-35) 


``````````diff
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 8b6981d4cc34a2..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,42 +16,29 @@
 
 namespace LIBC_NAMESPACE {
 
+__attribute__((naked))
 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
-
-  // 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

``````````

</details>


https://github.com/llvm/llvm-project/pull/88054