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

Nick Desaulniers via libc-commits libc-commits at lists.llvm.org
Mon Apr 8 11:31:29 PDT 2024


================
@@ -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"
----------------
nickdesaulniers wrote:

No; `volatile` just keeps the inline asm blob around in case the outputs are DCE'd. In this case, because we write to the function parameter, we can't DCE the inline asm blob.  You can verify this by compiling this change and disassembling this function and observing that the inline asm is not discarded.

Theoretically, if were to inline `setjmp` (perhaps due to LTO) and someone accidentally called `setjmp` but then did not use the `jmp_buf` parameter ever then we could DCE this inline asm blob.  Which we would want.

But because this inline asm would not be safe to inline under LTO, and because this function also uses dangerous constructs like `__builtin_frame_address`, we should add `[[noinline]]` to this function definition.

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


More information about the libc-commits mailing list