[libc] Add returns_twice attribute to setjmp(3)

[alanzhao1]

This is to ensure that calls to setjmp(3) result in correct code generation that respects setjmp(3)'s returns_twice behavior. Otherwise, we might run into bugs (for example, Clang may perform tail-call optimization on this function if -fno-builtins is set (#122840)).

[llvmbot]

@llvm/pr-subscribers-libc

Author: Alan Zhao (alanzhao1)

Changes

This is to work around a clang bug where clang will incorrectly add a tail call optimization to setjmp(3) if -fno-builtins is set (#122840).

---

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

2 Files Affected:

• (modified) libc/include/__llvm-libc-common.h (+6)
• (modified) libc/include/setjmp.yaml (+2)

diff --git a/libc/include/__llvm-libc-common.h b/libc/include/__llvm-libc-common.h
index b5a23c5765f4d8..58006a1ffc24fc 100644
--- a/libc/include/__llvm-libc-common.h
+++ b/libc/include/__llvm-libc-common.h
@@ -45,6 +45,9 @@
 #define __NOEXCEPT throw()
 #endif
 
+#undef _Returns_twice
+#define _Returns_twice [[gnu::returns_twice]]
+
 #else // not __cplusplus
 
 #undef __BEGIN_C_DECLS
@@ -80,6 +83,9 @@
 #define __NOEXCEPT
 #endif
 
+#undef _Returns_twice
+#define _Returns_twice __attribute__((returns_twice))
+
 #endif // __cplusplus
 
 #endif // LLVM_LIBC_COMMON_H
diff --git a/libc/include/setjmp.yaml b/libc/include/setjmp.yaml
index 2c4f7fb6dfcc70..5fbb9eb2a47e5b 100644
--- a/libc/include/setjmp.yaml
+++ b/libc/include/setjmp.yaml
@@ -17,5 +17,7 @@ functions:
     standards:
       - stdc
     return_type: int
+    attributes:
+      - _Returns_twice
     arguments:
       - type: jmp_buf

[jyknight]

The commit message here isn't really accurate. First, TCO is not the only thing that can't be done in the presence of setjmp -- it affects a variety of things. But more importantly, there's a stronger reason to add the attribute than just "workaround".

Fundamentally, any function which has setjmp-like behaviors must have its declaration (and therefore the calls at the IR level) annotated with the returns_twice attribute -- without this, it will not generate correct code.

Clang has a builtin list of function names that get the attr applied automatically (unless -fno-builtin is specified), which usually covers things. But, llvm-libc actually defines and (at least in tests, calls) setjmp with a function name which is not in the list: LIBC_NAMESPACE::setjmp. That non-standard name will not ever get the magic behavior automatically.

libc/src/setjmp/setjmp_impl.h also needs the attribute applied.

[alanzhao1]

libc/src/setjmp/setjmp_impl.h also needs the attribute applied.

Done

[alanzhao1]

But, llvm-libc actually defines and (at least in tests, calls) setjmp with a function name which is not in the list: LIBC_NAMESPACE::setjmp. That non-standard name will not ever get the magic behavior automatically.

Isn't this intended behavior though? My understanding is that the special casing should only apply to the function named setjmp(...) provided by the standard library, and since LIBC_NAMESPACE::setjmp is in a namespace, it's not the specific function in question and therefore special casing shouldn't apply.

[jyknight]

Yes, Clang of course shouldn't special-case functions defined in arbitrary namespaces. But that's exactly the problem with having libc depend on a special-case list to apply the required attribute. That's why it's not a "workaround" for libc to set the required attribute, but rather, the correct thing to do!

[alanzhao1]

The commit message here isn't really accurate. First, TCO is not the only thing that can't be done in the presence of setjmp -- it affects a variety of things. But more importantly, there's a stronger reason to add the attribute than just "workaround".

Fundamentally, any function which has setjmp-like behaviors must have its declaration (and therefore the calls at the IR level) annotated with the returns_twice attribute -- without this, it will not generate correct code.

Clang has a builtin list of function names that get the attr applied automatically (unless -fno-builtin is specified), which usually covers things. But, llvm-libc actually defines and (at least in tests, calls) setjmp with a function name which is not in the list: LIBC_NAMESPACE::setjmp. That non-standard name will not ever get the magic behavior automatically.

libc/src/setjmp/setjmp_impl.h also needs the attribute applied.

PR description has been updated