2
0
Эх сурвалжийг харах

Fix mutex use after free in llvm_shutdown (#5463) (#5477)

This patch is consolidated from the following upstream patches:

- llvm/llvm-project@3d5360a4398b
- llvm/llvm-project@928071ae4ef5
- llvm/llvm-project@56349e8b6d85

Fixes https://github.com/microsoft/DirectXShaderCompiler/issues/5119

Co-authored-by: Ellie Hermaszewska <[email protected]>
Chris B 2 жил өмнө
parent
commit
271b46a172

+ 69 - 41
include/llvm/Support/ManagedStatic.h

@@ -10,42 +10,77 @@
 // This file defines the ManagedStatic class and the llvm_shutdown() function.
 //
 //===----------------------------------------------------------------------===//
+//
+// The following changes have been backported from upstream llvm:
+//
+// 56349e8b6d85 Fix for memory leak reported by Valgrind
+// 928071ae4ef5 [Support] Replace sys::Mutex with their standard equivalents.
+// 3d5360a4398b Replace llvm::MutexGuard/UniqueLock with their standard equivalents
+// c06a470fc843 Try once more to ensure constant initializaton of ManagedStatics
+// 41fe3a54c261 Ensure that ManagedStatic is constant initialized in MSVC 2017 & 2019
+// 74de08031f5d [ManagedStatic] Avoid putting function pointers in template args.
+// f65e4ce2c48a [ADT, Support, TableGen] Fix some Clang-tidy modernize-use-default and Include What You Use warnings; other minor fixes (NFC).
+// 832d04207810 [ManagedStatic] Reimplement double-checked locking with std::atomic.
+// dd1463823a0c This is yet another attempt to re-instate r220932 as discussed in D19271.
+//
 
 #ifndef LLVM_SUPPORT_MANAGEDSTATIC_H
 #define LLVM_SUPPORT_MANAGEDSTATIC_H
 
-#include "llvm/Support/Atomic.h"
-#include "llvm/Support/Threading.h"
-#include "llvm/Support/Valgrind.h"
+#include <atomic>
+#include <cstddef>
 
 namespace llvm {
 
 /// object_creator - Helper method for ManagedStatic.
-template<class C>
-void* object_creator() {
-  return new C();
-}
+template <class C> struct object_creator {
+  static void *call() { return new C(); }
+};
 
 /// object_deleter - Helper method for ManagedStatic.
 ///
-template<typename T> struct object_deleter {
-  static void call(void * Ptr) { delete (T*)Ptr; }
+template <typename T> struct object_deleter {
+  static void call(void *Ptr) { delete (T *)Ptr; }
 };
-template<typename T, size_t N> struct object_deleter<T[N]> {
-  static void call(void * Ptr) { delete[] (T*)Ptr; }
+template <typename T, size_t N> struct object_deleter<T[N]> {
+  static void call(void *Ptr) { delete[](T *)Ptr; }
 };
 
+// ManagedStatic must be initialized to zero, and it must *not* have a dynamic
+// initializer because managed statics are often created while running other
+// dynamic initializers. In standard C++11, the best way to accomplish this is
+// with a constexpr default constructor. However, different versions of the
+// Visual C++ compiler have had bugs where, even though the constructor may be
+// constexpr, a dynamic initializer may be emitted depending on optimization
+// settings. For the affected versions of MSVC, use the old linker
+// initialization pattern of not providing a constructor and leaving the fields
+// uninitialized. See http://llvm.org/PR41367 for details.
+#if !defined(_MSC_VER) || (_MSC_VER >= 1925) || defined(__clang__)
+#define LLVM_USE_CONSTEXPR_CTOR
+#endif
+
 /// ManagedStaticBase - Common base class for ManagedStatic instances.
 class ManagedStaticBase {
 protected:
+#ifdef LLVM_USE_CONSTEXPR_CTOR
+  mutable std::atomic<void *> Ptr{};
+  mutable void (*DeleterFn)(void *) = nullptr;
+  mutable const ManagedStaticBase *Next = nullptr;
+#else
   // This should only be used as a static variable, which guarantees that this
   // will be zero initialized.
-  mutable void *Ptr;
-  mutable void (*DeleterFn)(void*);
+  mutable std::atomic<void *> Ptr;
+  mutable void (*DeleterFn)(void *);
   mutable const ManagedStaticBase *Next;
+#endif
 
   void RegisterManagedStatic(void *(*creator)(), void (*deleter)(void*)) const;
+
 public:
+#ifdef LLVM_USE_CONSTEXPR_CTOR
+  constexpr ManagedStaticBase() = default;
+#endif
+
   /// isConstructed - Return true if this object has not been created yet.
   bool isConstructed() const { return Ptr != nullptr; }
 
@@ -57,42 +92,35 @@ public:
 /// libraries that link in LLVM components) and for making destruction be
 /// explicit through the llvm_shutdown() function call.
 ///
-template<class C>
+template <class C, class Creator = object_creator<C>,
+          class Deleter = object_deleter<C>>
 class ManagedStatic : public ManagedStaticBase {
 public:
-
   // Accessors.
   C &operator*() {
-    void* tmp = Ptr;
-    if (llvm_is_multithreaded()) sys::MemoryFence();
-    if (!tmp) RegisterManagedStatic(object_creator<C>, object_deleter<C>::call);
-    TsanHappensAfter(this);
+    void *Tmp = Ptr.load(std::memory_order_acquire);
+    if (!Tmp)
+      RegisterManagedStatic(Creator::call, Deleter::call);
 
-    return *static_cast<C*>(Ptr);
+    return *static_cast<C *>(Ptr.load(std::memory_order_relaxed));
   }
-  C *operator->() {
-    void* tmp = Ptr;
-    if (llvm_is_multithreaded()) sys::MemoryFence();
-    if (!tmp) RegisterManagedStatic(object_creator<C>, object_deleter<C>::call);
-    TsanHappensAfter(this);
 
-    return static_cast<C*>(Ptr);
-  }
+  C *operator->() { return &**this; }
+
   const C &operator*() const {
-    void* tmp = Ptr;
-    if (llvm_is_multithreaded()) sys::MemoryFence();
-    if (!tmp) RegisterManagedStatic(object_creator<C>, object_deleter<C>::call);
-    TsanHappensAfter(this);
+    void *Tmp = Ptr.load(std::memory_order_acquire);
+    if (!Tmp)
+      RegisterManagedStatic(Creator::call, Deleter::call);
 
-    return *static_cast<C*>(Ptr);
+    return *static_cast<C *>(Ptr.load(std::memory_order_relaxed));
   }
-  const C *operator->() const {
-    void* tmp = Ptr;
-    if (llvm_is_multithreaded()) sys::MemoryFence();
-    if (!tmp) RegisterManagedStatic(object_creator<C>, object_deleter<C>::call);
-    TsanHappensAfter(this);
 
-    return static_cast<C*>(Ptr);
+  const C *operator->() const { return &**this; }
+
+  // Extract the instance, leaving the ManagedStatic uninitialized. The
+  // user is then responsible for the lifetime of the returned instance.
+  C *claim() {
+    return static_cast<C *>(Ptr.exchange(nullptr));
   }
 };
 
@@ -102,10 +130,10 @@ void llvm_shutdown();
 /// llvm_shutdown_obj - This is a simple helper class that calls
 /// llvm_shutdown() when it is destroyed.
 struct llvm_shutdown_obj {
-  llvm_shutdown_obj() { }
+  llvm_shutdown_obj() = default;
   ~llvm_shutdown_obj() { llvm_shutdown(); }
 };
 
-}
+} // end namespace llvm
 
-#endif
+#endif // LLVM_SUPPORT_MANAGEDSTATIC_H

+ 28 - 26
lib/Support/ManagedStatic.cpp

@@ -10,45 +10,46 @@
 // This file implements the ManagedStatic class and llvm_shutdown().
 //
 //===----------------------------------------------------------------------===//
+//
+// The following changes have been backported from upstream llvm:
+//
+// 56349e8b6d85 Fix for memory leak reported by Valgrind
+// 928071ae4ef5 [Support] Replace sys::Mutex with their standard equivalents.
+// 3d5360a4398b Replace llvm::MutexGuard/UniqueLock with their standard equivalents
+// c06a470fc843 Try once more to ensure constant initializaton of ManagedStatics
+// 41fe3a54c261 Ensure that ManagedStatic is constant initialized in MSVC 2017 & 2019
+// 74de08031f5d [ManagedStatic] Avoid putting function pointers in template args.
+// f65e4ce2c48a [ADT, Support, TableGen] Fix some Clang-tidy modernize-use-default and Include What You Use warnings; other minor fixes (NFC).
+// 832d04207810 [ManagedStatic] Reimplement double-checked locking with std::atomic.
+// dd1463823a0c This is yet another attempt to re-instate r220932 as discussed in D19271.
+//
 
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Config/config.h"
-#include "llvm/Support/Atomic.h"
-#include "llvm/Support/Mutex.h"
-#include "llvm/Support/MutexGuard.h"
+#include "llvm/Support/Threading.h"
 #include <cassert>
+#include <mutex>
 using namespace llvm;
 
 static const ManagedStaticBase *StaticList = nullptr;
 
-static sys::Mutex& getManagedStaticMutex() {
-  // We need to use a function local static here, since this can get called
-  // during a static constructor and we need to guarantee that it's initialized
-  // correctly.
-  static sys::Mutex ManagedStaticMutex;
-  return ManagedStaticMutex;
+static std::recursive_mutex *getManagedStaticMutex() {
+  static std::recursive_mutex m;
+  return &m;
 }
 
 void ManagedStaticBase::RegisterManagedStatic(void *(*Creator)(),
                                               void (*Deleter)(void*)) const {
   assert(Creator);
   if (llvm_is_multithreaded()) {
-    MutexGuard Lock(getManagedStaticMutex());
-
-    if (!Ptr) {
-      void* tmp = Creator();
+    std::lock_guard<std::recursive_mutex> Lock(*getManagedStaticMutex());
 
-      TsanHappensBefore(this);
-      sys::MemoryFence();
+    if (!Ptr.load(std::memory_order_relaxed)) {
+      void *Tmp = Creator();
 
-      // This write is racy against the first read in the ManagedStatic
-      // accessors. The race is benign because it does a second read after a
-      // memory fence, at which point it isn't possible to get a partial value.
-      TsanIgnoreWritesBegin();
-      Ptr = tmp;
-      TsanIgnoreWritesEnd();
+      Ptr.store(Tmp, std::memory_order_release);
       DeleterFn = Deleter;
-      
+
       // Add to list of managed statics.
       Next = StaticList;
       StaticList = this;
@@ -58,7 +59,7 @@ void ManagedStaticBase::RegisterManagedStatic(void *(*Creator)(),
            "Partially initialized ManagedStatic!?");
     Ptr = Creator();
     DeleterFn = Deleter;
-  
+
     // Add to list of managed statics.
     Next = StaticList;
     StaticList = this;
@@ -75,16 +76,17 @@ void ManagedStaticBase::destroy() const {
 
   // Destroy memory.
   DeleterFn(Ptr);
-  
+
   // Cleanup.
   Ptr = nullptr;
   DeleterFn = nullptr;
 }
 
 /// llvm_shutdown - Deallocate and destroy all ManagedStatic variables.
+/// IMPORTANT: it's only safe to call llvm_shutdown() in single thread,
+/// without any other threads executing LLVM APIs.
+/// llvm_shutdown() should be the last use of LLVM APIs.
 void llvm::llvm_shutdown() {
-  MutexGuard Lock(getManagedStaticMutex());
-
   while (StaticList)
     StaticList->destroy();
 }