[libcxx-commits] [PATCH] D155278: [libc++][mingw] TLS cleanup on windows

[Karl-Johan Johnsson via Phabricator via libcxx-commits]

kalle-llvm created this revision.
kalle-llvm added a reviewer: mstorsjo.
Herald added a project: All.
kalle-llvm requested review of this revision.
Herald added projects: libc++, libc++abi.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.

On windows, when libc++ is statically linked into a dll the tls-key used for __cxa_eh_globals must be freed when the dll is unloaded.  Otherwise the callback registered for the tls-key will remain in place but point to non-existing code and the process will crash as soon as a thread exits.

This will happen if libc++ is statically linked into the dll or if the main application doesn't use libc++ (so that is is unloaded when the dll is unloaded).

I've made a function __libcpp_tls_destroy() that is registered with atexit() that frees the tls-key.  I did this conditionally on _LIBCPP_HAS_THREAD_API_WIN32 since I'm not sure how this should be handled for the other thread models.

I think a similar problem exists with the tls-key in __thread_specific_ptr, but a comment in the destructor (thread.h line 103) says that the missing cleanup is intentional in the pthreads case.  And this destructor is inlined so I guess adding a cleanup there would be an ABI break?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155278

Files:
  libcxx/include/__threading_support
  libcxx/src/support/win32/thread_win32.cpp
  libcxxabi/src/cxa_exception_storage.cpp


Index: libcxxabi/src/cxa_exception_storage.cpp
===================================================================
--- libcxxabi/src/cxa_exception_storage.cpp
+++ libcxxabi/src/cxa_exception_storage.cpp
@@ -64,9 +64,18 @@
             abort_message("cannot zero out thread value for __cxa_get_globals()");
     }
 
+#if defined(_LIBCPP_HAS_THREAD_API_WIN32)
+    void destroy_() {
+        std::__libcpp_tls_destroy(key_);
+    }
+#endif
+
     void construct_() {
         if (0 != std::__libcpp_tls_create(&key_, destruct_))
             abort_message("cannot create thread specific key for __cxa_get_globals()");
+#if defined(_LIBCPP_HAS_THREAD_API_WIN32)
+        atexit(destroy_);
+#endif
     }
 } // namespace
 
Index: libcxx/src/support/win32/thread_win32.cpp
===================================================================
--- libcxx/src/support/win32/thread_win32.cpp
+++ libcxx/src/support/win32/thread_win32.cpp
@@ -259,6 +259,11 @@
   return 0;
 }
 
+void __libcpp_tls_destroy(__libcpp_tls_key __key)
+{
+  FlsFree(__key);
+}
+
 void *__libcpp_tls_get(__libcpp_tls_key __key)
 {
   return FlsGetValue(__key);
Index: libcxx/include/__threading_support
===================================================================
--- libcxx/include/__threading_support
+++ libcxx/include/__threading_support
@@ -240,6 +240,9 @@
 int __libcpp_tls_create(__libcpp_tls_key* __key,
                         void(_LIBCPP_TLS_DESTRUCTOR_CC* __at_exit)(void*));
 
+_LIBCPP_THREAD_ABI_VISIBILITY
+void __libcpp_tls_destroy(__libcpp_tls_key __key);
+
 _LIBCPP_THREAD_ABI_VISIBILITY
 void *__libcpp_tls_get(__libcpp_tls_key __key);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D155278.540326.patch
Type: text/x-patch
Size: 1630 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20230714/0dd24a39/attachment.bin>