From 389f13012fc618bdc3eaf252d8e3c1aca4a44376 Mon Sep 17 00:00:00 2001 From: Dylan Noblesmith Date: Sat, 23 Aug 2014 23:07:14 +0000 Subject: [PATCH] Support: add llvm::unique_lock Based on the STL class of the same name, it guards a mutex while also allowing it to be unlocked conditionally before destruction. This eliminates the last naked usages of mutexes in LLVM and clang. It also uncovered and fixed a bug in callExternalFunction() when compiled without USE_LIBFFI, where the mutex would never be unlocked if the end of the function was reached. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@216338 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/IR/ValueMap.h | 11 ++- include/llvm/Support/UniqueLock.h | 67 +++++++++++++++++++ .../Interpreter/ExternalFunctions.cpp | 7 +- lib/Support/Unix/Signals.inc | 33 ++++----- 4 files changed, 93 insertions(+), 25 deletions(-) create mode 100644 include/llvm/Support/UniqueLock.h diff --git a/include/llvm/IR/ValueMap.h b/include/llvm/IR/ValueMap.h index 993694889a5..aa8a29dc771 100644 --- a/include/llvm/IR/ValueMap.h +++ b/include/llvm/IR/ValueMap.h @@ -29,6 +29,7 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/IR/ValueHandle.h" #include "llvm/Support/Mutex.h" +#include "llvm/Support/UniqueLock.h" #include "llvm/Support/type_traits.h" #include @@ -216,12 +217,11 @@ public: // Make a copy that won't get changed even when *this is destroyed. ValueMapCallbackVH Copy(*this); typename Config::mutex_type *M = Config::getMutex(Copy.Map->Data); + unique_lock Guard; if (M) - M->lock(); + Guard = unique_lock(*M); Config::onDelete(Copy.Map->Data, Copy.Unwrap()); // May destroy *this. Copy.Map->Map.erase(Copy); // Definitely destroys *this. - if (M) - M->unlock(); } void allUsesReplacedWith(Value *new_key) override { assert(isa(new_key) && @@ -229,8 +229,9 @@ public: // Make a copy that won't get changed even when *this is destroyed. ValueMapCallbackVH Copy(*this); typename Config::mutex_type *M = Config::getMutex(Copy.Map->Data); + unique_lock Guard; if (M) - M->lock(); + Guard = unique_lock(*M); KeyT typed_new_key = cast(new_key); // Can destroy *this: @@ -245,8 +246,6 @@ public: Copy.Map->insert(std::make_pair(typed_new_key, Target)); } } - if (M) - M->unlock(); } }; diff --git a/include/llvm/Support/UniqueLock.h b/include/llvm/Support/UniqueLock.h new file mode 100644 index 00000000000..5a4c273e83e --- /dev/null +++ b/include/llvm/Support/UniqueLock.h @@ -0,0 +1,67 @@ +//===-- Support/UniqueLock.h - Acquire/Release Mutex In Scope ---*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file defines a guard for a block of code that ensures a Mutex is locked +// upon construction and released upon destruction. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_SUPPORT_UNIQUE_LOCK_H +#define LLVM_SUPPORT_UNIQUE_LOCK_H + +#include "llvm/Support/Mutex.h" + +namespace llvm { + /// A pared-down imitation of std::unique_lock from C++11. Contrary to the + /// name, it's really more of a wrapper for a lock. It may or may not have + /// an associated mutex, which is guaranteed to be locked upon creation + /// and unlocked after destruction. unique_lock can also unlock the mutex + /// and re-lock it freely during its lifetime. + /// @brief Guard a section of code with a mutex. + template + class unique_lock { + MutexT *M; + bool locked; + + unique_lock(const unique_lock &) LLVM_DELETED_FUNCTION; + void operator=(const unique_lock &) LLVM_DELETED_FUNCTION; + public: + unique_lock() : M(nullptr), locked(false) {} + explicit unique_lock(MutexT &m) : M(&m), locked(true) { M->lock(); } + + void operator=(unique_lock &&o) { + if (owns_lock()) + M->unlock(); + M = o.M; + locked = o.locked; + o.M = nullptr; + o.locked = false; + } + + ~unique_lock() { if (owns_lock()) M->unlock(); } + + void lock() { + assert(!locked && "mutex already locked!"); + assert(M && "no associated mutex!"); + M->lock(); + locked = true; + } + + void unlock() { + assert(locked && "unlocking a mutex that isn't locked!"); + assert(M && "no associated mutex!"); + M->unlock(); + locked = false; + } + + bool owns_lock() { return locked; } + }; +} + +#endif // LLVM_SUPPORT_UNIQUE_LOCK_H diff --git a/lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp b/lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp index e4b89c60b46..0fdcc5fd361 100644 --- a/lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp +++ b/lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp @@ -28,6 +28,7 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/ManagedStatic.h" #include "llvm/Support/Mutex.h" +#include "llvm/Support/UniqueLock.h" #include #include #include @@ -248,14 +249,14 @@ GenericValue Interpreter::callExternalFunction(Function *F, const std::vector &ArgVals) { TheInterpreter = this; - FunctionsLock->lock(); + unique_lock Guard(*FunctionsLock); // Do a lookup to see if the function is in our cache... this should just be a // deferred annotation! std::map::iterator FI = ExportedFunctions->find(F); if (ExFunc Fn = (FI == ExportedFunctions->end()) ? lookupFunction(F) : FI->second) { - FunctionsLock->unlock(); + Guard.unlock(); return Fn(F->getFunctionType(), ArgVals); } @@ -273,7 +274,7 @@ GenericValue Interpreter::callExternalFunction(Function *F, RawFn = RF->second; } - FunctionsLock->unlock(); + Guard.unlock(); GenericValue Result; if (RawFn != 0 && ffiInvoke(RawFn, F, ArgVals, getDataLayout(), Result)) diff --git a/lib/Support/Unix/Signals.inc b/lib/Support/Unix/Signals.inc index 34c39c9ad92..c820553665d 100644 --- a/lib/Support/Unix/Signals.inc +++ b/lib/Support/Unix/Signals.inc @@ -15,6 +15,7 @@ #include "Unix.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Support/Mutex.h" +#include "llvm/Support/UniqueLock.h" #include #include #include @@ -162,25 +163,25 @@ static RETSIGTYPE SignalHandler(int Sig) { sigfillset(&SigMask); sigprocmask(SIG_UNBLOCK, &SigMask, nullptr); - SignalsMutex.lock(); - RemoveFilesToRemove(); - - if (std::find(IntSigs, IntSigsEnd, Sig) != IntSigsEnd) { - if (InterruptFunction) { - void (*IF)() = InterruptFunction; - SignalsMutex.unlock(); - InterruptFunction = nullptr; - IF(); // run the interrupt function. + { + unique_lock> Guard(SignalsMutex); + RemoveFilesToRemove(); + + if (std::find(IntSigs, IntSigsEnd, Sig) != IntSigsEnd) { + if (InterruptFunction) { + void (*IF)() = InterruptFunction; + Guard.unlock(); + InterruptFunction = nullptr; + IF(); // run the interrupt function. + return; + } + + Guard.unlock(); + raise(Sig); // Execute the default handler. return; - } - - SignalsMutex.unlock(); - raise(Sig); // Execute the default handler. - return; + } } - SignalsMutex.unlock(); - // Otherwise if it is a fault (like SEGV) run any handler. for (unsigned i = 0, e = CallBacksToRun.size(); i != e; ++i) CallBacksToRun[i].first(CallBacksToRun[i].second); -- 2.34.1