[libc] Provide empty definitions for teardown_main_tls symbols

[petrhosek]

This avoids the GOT slot being generated which is undesirable when using static linking. A better solution would be to reorganize the code to avoid the use of weak symbols altogether.

See #173409

[llvmbot]

@llvm/pr-subscribers-libc

Author: Petr Hosek (petrhosek)

Changes

This avoids the GOT slot being generated which is undesirable when using static linking. A better solution would be to reorganize the code to avoid the use of weak symbols altogether.

Fixes #173409

---

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

3 Files Affected:

• (modified) libc/src/stdlib/atexit.cpp (+2-3)
• (modified) libc/src/stdlib/exit.cpp (+2-3)
• (modified) libc/src/stdlib/quick_exit.cpp (+2-3)

diff --git a/libc/src/stdlib/atexit.cpp b/libc/src/stdlib/atexit.cpp
index 799aad136bda5..fb45c3a7ea280 100644
--- a/libc/src/stdlib/atexit.cpp
+++ b/libc/src/stdlib/atexit.cpp
@@ -16,7 +16,7 @@ namespace LIBC_NAMESPACE_DECL {
 
 constinit ExitCallbackList atexit_callbacks;
 Mutex handler_list_mtx(false, false, false, false);
-[[gnu::weak]] extern void teardown_main_tls();
+[[gnu::weak]] void teardown_main_tls() {}
 
 extern "C" {
 
@@ -27,8 +27,7 @@ int __cxa_atexit(AtExitCallback *callback, void *payload, void *) {
 void __cxa_finalize(void *dso) {
   if (!dso) {
     call_exit_callbacks(atexit_callbacks);
-    if (teardown_main_tls)
-      teardown_main_tls();
+    teardown_main_tls();
   }
 }
 
diff --git a/libc/src/stdlib/exit.cpp b/libc/src/stdlib/exit.cpp
index f26d48ac00d7f..dd5211a504fef 100644
--- a/libc/src/stdlib/exit.cpp
+++ b/libc/src/stdlib/exit.cpp
@@ -20,15 +20,14 @@ extern "C" void __cxa_finalize(void *);
 //       as we have no way to ensure system libc will call the TLS destructors.
 //       We should run exit related tests in hermetic mode but this is currently
 //       blocked by https://github.com/llvm/llvm-project/issues/133925.
-extern "C" [[gnu::weak]] void __cxa_thread_finalize();
+extern "C" [[gnu::weak]] void __cxa_thread_finalize() {}
 
 // TODO: use recursive mutex to protect this routine.
 [[noreturn]] LLVM_LIBC_FUNCTION(void, exit, (int status)) {
 // FIXME: The NVPTX target does not support external weak symbols correctly
 //        despite being an ELF platform. Disable pending a future split.
 #if !defined(LIBC_TARGET_ARCH_IS_NVPTX)
-  if (__cxa_thread_finalize)
-    __cxa_thread_finalize();
+  __cxa_thread_finalize();
 #endif
   __cxa_finalize(nullptr);
   internal::exit(status);
diff --git a/libc/src/stdlib/quick_exit.cpp b/libc/src/stdlib/quick_exit.cpp
index 29110b33afcf5..181e8a6097cfb 100644
--- a/libc/src/stdlib/quick_exit.cpp
+++ b/libc/src/stdlib/quick_exit.cpp
@@ -16,12 +16,11 @@
 namespace LIBC_NAMESPACE_DECL {
 
 extern ExitCallbackList at_quick_exit_callbacks;
-[[gnu::weak]] extern void teardown_main_tls();
+[[gnu::weak]] void teardown_main_tls() {}
 
 [[noreturn]] LLVM_LIBC_FUNCTION(void, quick_exit, (int status)) {
   call_exit_callbacks(at_quick_exit_callbacks);
-  if (teardown_main_tls)
-    teardown_main_tls();
+  teardown_main_tls();
   internal::exit(status);
 }
 

[frobtech]

This is easy to approve for the teardown_main_tls symbol, but might require more discussion wrt __cxa_thread_finalize. So let's make it two separate changes, and use the second one to more thoroughly discuss issues with the __cxa_thread_finalize symbol name / ABI in particular.

[jhuber6]

Can probably remove this for NVPTX if it's defined now

[petrhosek]

This is easy to approve for the teardown_main_tls symbol, but might require more discussion wrt __cxa_thread_finalize. So let's make it two separate changes, and use the second one to more thoroughly discuss issues with the __cxa_thread_finalize symbol name / ABI in particular.

I updated this change to only cover teardown_main_tls and will make a separate pull request for __cxa_thread_finalize.

[frobtech]

Comments and clarity of intent aside, I think this is clearly a fine incremental change to the status quo.

[frobtech]

This (and any use of weak) merits a clear comment about what's going on here.
It should explain that the weak no-op is in the same TU with its caller for the case
where the real definition is not linked in. In particular, why it's OK to omit what it does when that specific TU is not linked in for other reasons, and what reasons would cause it to be linked in and thus have its strong definition take precedence. In this case it's also important to note in both atexit.cpp and quick_exit.cpp that each is doing its own weak no-op definition and why.

Since this is the first to set the example, we should also have a clear comment about the choice of weak no-op definition vs weak undefined with check. That's probably something we should write more formally into the libc style guide as a generic rule and rationale.

It's not entirely clear to me what the intended weak no-op scenario is for this particular one. The symbol is defined in libc's startup code, which will always be present in the statically-linked executable scenarios that I think are the only cases actually supported today.

[petrhosek]

Superseded by #174373 and #174374 after offline discussion with @frobtech.