diff --git a/interpreter/cling/include/cling/Utils/OrderedMap.h b/interpreter/cling/include/cling/Utils/OrderedMap.h new file mode 100644 index 0000000000000..412f9bc788df4 --- /dev/null +++ b/interpreter/cling/include/cling/Utils/OrderedMap.h @@ -0,0 +1,115 @@ +//--------------------------------------------------------------------*- C++ -*- +// CLING - the C++ LLVM-based InterpreterG :) +// author: Roman Zulak +// +// This file is dual-licensed: you can choose to license it under the University +// of Illinois Open Source License or the GNU Lesser General Public License. See +// LICENSE.TXT for details. +//------------------------------------------------------------------------------ + +#ifndef CLING_ORDERED_MAP_H +#define CLING_ORDERED_MAP_H + +#include +#include + +namespace cling { +namespace utils { + +///\brief Thin wrapper class for tracking the order of insertion into a +/// std::unordered_map. +/// +/// Only supports 'emplace' and '[Key]' for insertion of values, and adds an +/// additional parameter to 'erase' so that a mapped value can be moved into +/// local storage before erasing the iterator occurs. +/// +template class OrderedMap { + typedef std::unordered_map map_t; + // Would this be faster as a std::unoredered_map for erasure? + typedef std::vector order_t; + + map_t m_Map; + order_t m_Order; + +public: + typedef typename map_t::iterator iterator; + typedef typename map_t::const_iterator const_iterator; + typedef typename map_t::mapped_type mapped_type; + + template + std::pair emplace(Args&&... args) { + auto Rval = m_Map.emplace(std::forward(args)...); + if (Rval.second) m_Order.emplace_back(Rval.first); + return Rval; + } + + Value& operator[](const Key& K) { + iterator Itr = find(K); + if (Itr == end()) { + Itr = m_Map.emplace(K, Value()).first; + m_Order.emplace_back(Itr); + } + return Itr->second; + } + + Value& operator[](Key&& K) { + iterator Itr = find(K); + if (Itr == end()) { + Itr = m_Map.emplace(K, Value()).first; + m_Order.emplace_back(Itr); + } + return Itr->second; + } + + ///\brief Erase a mapping from this object. + /// + ///\param [in] Itr - The iterator to erase. + ///\param [out] Move - Move the mapped object to this pointer before erasing. + /// + void erase(const_iterator Itr, mapped_type* Move = nullptr) { + assert(std::find(m_Order.begin(), m_Order.end(), Itr) != m_Order.end()); + for (auto Otr = m_Order.begin(), End = m_Order.end(); Otr != End; ++Otr) { + if (Itr == *Otr) { + m_Order.erase(Otr); + break; + } + } + assert(std::find(m_Order.begin(), m_Order.end(), Itr) == m_Order.end()); + if (Move) *Move = std::move(Itr->second); + m_Map.erase(Itr); + } + + iterator find(const Key& K) { return m_Map.find(K); } + const_iterator find(const Key& K) const { return m_Map.find(K); } + + iterator end() { return m_Map.end(); } + const_iterator end() const { return m_Map.end(); } + + void swap(OrderedMap& Other) { + m_Map.swap(Other.m_Map); + m_Order.swap(Other.m_Order); + } + + void clear() { + m_Map.clear(); + m_Order.clear(); + } + + bool empty() const { + assert(m_Map.empty() == m_Order.empty() && "Not synchronized"); + return m_Order.empty(); + } + + void size() const { + assert(m_Map.size() == m_Order.size() && "Not synchronized"); + return m_Order.size(); + } + + const order_t& ordered() const { return m_Order; } + order_t& ordered() { return m_Order; } +}; + +} // namespace utils +} // namespace cling + +#endif // CLING_PLATFORM_H diff --git a/interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp b/interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp index 1b1b2c79b77d3..25439f85e9856 100644 --- a/interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp +++ b/interpreter/cling/lib/Interpreter/IncrementalExecutor.cpp @@ -92,10 +92,6 @@ IncrementalExecutor::IncrementalExecutor(clang::DiagnosticsEngine& diags, // MSVC doesn't support m_AtExitFuncsSpinLock=ATOMIC_FLAG_INIT; in the class definition std::atomic_flag_clear( &m_AtExitFuncsSpinLock ); - // No need to protect this access of m_AtExitFuncs, since nobody - // can use this object yet. - m_AtExitFuncs.reserve(256); - std::unique_ptr TM(CreateHostTargetMachine(CI)); m_BackendPasses.reset(new BackendPasses(CI.getCodeGenOpts(), CI.getTargetOpts(), @@ -108,11 +104,23 @@ IncrementalExecutor::IncrementalExecutor(clang::DiagnosticsEngine& diags, IncrementalExecutor::~IncrementalExecutor() {} void IncrementalExecutor::shuttingDown() { - // No need to protect this access, since hopefully there is no concurrent - // shutdown request. - for (size_t I = 0, N = m_AtExitFuncs.size(); I < N; ++I) { - const CXAAtExitElement& AEE = m_AtExitFuncs[N - I - 1]; - (*AEE.m_Func)(AEE.m_Arg); + // It is legal to register an atexit handler from within another atexit + // handler and furthor-more the standard says they need to run in reverse + // order, so this function must be recursion safe. + AtExitFunctions Local; + { + cling::internal::SpinLockGuard slg(m_AtExitFuncsSpinLock); + // Check this case first, to avoid the swap all-together. + if (m_AtExitFuncs.empty()) + return; + Local.swap(m_AtExitFuncs); + } + for (auto&& Ordered : llvm::reverse(Local.ordered())) { + for (auto&& AtExit : llvm::reverse(Ordered->second)) + AtExit(); + // The standard says that they need to run in reverse order, which means + // anything added from 'AtExit()' must now be run! + shuttingDown(); } } @@ -120,7 +128,7 @@ void IncrementalExecutor::AddAtExitFunc(void (*func) (void*), void* arg, llvm::Module* M) { // Register a CXAAtExit function cling::internal::SpinLockGuard slg(m_AtExitFuncsSpinLock); - m_AtExitFuncs.push_back(CXAAtExitElement(func, arg, M)); + m_AtExitFuncs[M].emplace_back(func, arg); } void unresolvedSymbol() @@ -288,25 +296,19 @@ IncrementalExecutor::runStaticInitializersOnce(const Transaction& T) { void IncrementalExecutor::runAndRemoveStaticDestructors(Transaction* T) { assert(T && "Must be set"); // Collect all the dtors bound to this transaction. - AtExitFunctions boundToT; - + AtExitFunctions::mapped_type Local; { cling::internal::SpinLockGuard slg(m_AtExitFuncsSpinLock); - for (AtExitFunctions::iterator I = m_AtExitFuncs.begin(); - I != m_AtExitFuncs.end();) - if (I->m_FromM == T->getModule()) { - boundToT.push_back(*I); - I = m_AtExitFuncs.erase(I); - } - else - ++I; + auto Itr = m_AtExitFuncs.find(T->getModule()); + if (Itr == m_AtExitFuncs.end()) return; + m_AtExitFuncs.erase(Itr, &Local); } // end of spin lock lifetime block. - // 'Unload' the cxa_atexit entities. - for (AtExitFunctions::reverse_iterator I = boundToT.rbegin(), - E = boundToT.rend(); I != E; ++I) { - const CXAAtExitElement& AEE = *I; - (*AEE.m_Func)(AEE.m_Arg); + // 'Unload' the cxa_atexit, atexit entities. + for (auto&& AtExit : llvm::reverse(Local)) { + AtExit(); + // Run anything that was just registered in 'AtExit()' + runAndRemoveStaticDestructors(T); } } @@ -358,18 +360,17 @@ bool IncrementalExecutor::diagnoseUnresolvedSymbols(llvm::StringRef trigger, return false; llvm::SmallVector funcsToFree; - for (std::set::const_iterator i = m_unresolvedSymbols.begin(), - e = m_unresolvedSymbols.end(); i != e; ++i) { + for (const std::string& sym : m_unresolvedSymbols) { #if 0 // FIXME: This causes a lot of test failures, for some reason it causes // the call to HandleMissingFunction to be elided. unsigned diagID = m_Diags.getCustomDiagID(clang::DiagnosticsEngine::Error, "%0 unresolved while jitting %1"); (void)diagID; - //m_Diags.Report(diagID) << *i << funcname; // TODO: demangle the names. + //m_Diags.Report(diagID) << sym << funcname; // TODO: demangle the names. #endif - cling::errs() << "IncrementalExecutor::executeFunction: symbol '" << *i + cling::errs() << "IncrementalExecutor::executeFunction: symbol '" << sym << "' unresolved while linking "; if (trigger.find(utils::Synthesize::UniquePrefix) != llvm::StringRef::npos) cling::errs() << "[cling interface function]"; @@ -383,7 +384,7 @@ bool IncrementalExecutor::diagnoseUnresolvedSymbols(llvm::StringRef trigger, cling::errs() << "!\n"; // Be helpful, demangle! - std::string demangledName = platform::Demangle(*i); + std::string demangledName = platform::Demangle(sym); if (!demangledName.empty()) { cling::errs() << "You are probably missing the definition of " diff --git a/interpreter/cling/lib/Interpreter/IncrementalExecutor.h b/interpreter/cling/lib/Interpreter/IncrementalExecutor.h index 5b00ce627c091..bfe8c9fd1fb7d 100644 --- a/interpreter/cling/lib/Interpreter/IncrementalExecutor.h +++ b/interpreter/cling/lib/Interpreter/IncrementalExecutor.h @@ -16,16 +16,17 @@ #include "cling/Interpreter/Transaction.h" #include "cling/Interpreter/Value.h" #include "cling/Utils/Casting.h" +#include "cling/Utils/OrderedMap.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringRef.h" -#include -#include +#include #include #include -#include +#include +#include namespace clang { class DiagnosticsEngine; @@ -64,7 +65,16 @@ namespace cling { /// The object is registered first as an CXAAtExitElement and then cling /// takes the control of it's destruction. /// - struct CXAAtExitElement { + class CXAAtExitElement { + ///\brief The function to be called. + /// + void (*m_Func)(void*); + + ///\brief The single argument passed to the function. + /// + void* m_Arg; + + public: ///\brief Constructs an element, whose destruction time will be managed by /// the interpreter. (By registering a function to be called by exit /// or when a shared library is unloaded.) @@ -83,22 +93,10 @@ namespace cling { ///\param [in] fromT - The unloading of this transaction will trigger the /// atexit function. /// - CXAAtExitElement(void (*func) (void*), void* arg, - const llvm::Module* fromM): - m_Func(func), m_Arg(arg), m_FromM(fromM) {} - - ///\brief The function to be called. - /// - void (*m_Func)(void*); - - ///\brief The single argument passed to the function. - /// - void* m_Arg; + CXAAtExitElement(void (*func)(void*), void* arg) + : m_Func(func), m_Arg(arg) {} - ///\brief The module whose unloading will trigger the call to this atexit - /// function. - /// - const llvm::Module* m_FromM; + void operator()() const { (*m_Func)(m_Arg); } }; ///\brief Atomic used as a spin lock to protect the access to m_AtExitFuncs @@ -109,10 +107,11 @@ namespace cling { /// again multiple conccurent access. std::atomic_flag m_AtExitFuncsSpinLock; // MSVC doesn't support = ATOMIC_FLAG_INIT; - typedef llvm::SmallVector AtExitFunctions; - ///\brief Static object, which are bound to unloading of certain declaration - /// to be destructed. + ///\brief Function registered via __cxa_atexit, atexit, or one of + /// it's C++ overloads that should be run when a module is unloaded. /// + typedef utils::OrderedMap> + AtExitFunctions; AtExitFunctions m_AtExitFuncs; ///\brief Modules to emit upon the next call to the JIT. @@ -126,7 +125,7 @@ namespace cling { ///\brief Set of the symbols that the JIT couldn't resolve. /// - std::set m_unresolvedSymbols; + std::unordered_set m_unresolvedSymbols; #if 0 // See FIXME in IncrementalExecutor.cpp ///\brief The diagnostics engine, printing out issues coming from the diff --git a/interpreter/cling/test/CodeUnloading/AtExit.C b/interpreter/cling/test/CodeUnloading/AtExit.C index 7febfcef3a7d3..953ff380c7dc3 100644 --- a/interpreter/cling/test/CodeUnloading/AtExit.C +++ b/interpreter/cling/test/CodeUnloading/AtExit.C @@ -32,10 +32,28 @@ atexit(atexit_1); at_quick_exit(atexit_2); .undo -// Make sure at_quick_exit is resolved correctly (mangling issues on gcc < 5) -// CHECK-NEXT: atexit_2 + // Make sure at_quick_exit is resolved correctly (mangling issues on gcc < + // 5) + // CHECK-NEXT: atexit_2 -atexit(atexit_3); + // Test reverse ordering in a single transaction. + static void + atexitA() { + printf("atexitA\n"); +} +static void atexitB() { printf("atexitB\n"); } +static void atexitC() { printf("atexitC\n"); } +{ + std::atexit(atexitA); + std::atexit(atexitB); + std::atexit(atexitC); +} +.undo + // CHECK-NEXT: atexitC + // CHECK-NEXT: atexitB + // CHECK-NEXT: atexitA + + atexit(atexit_3); cling::Interpreter * gChild = 0; { @@ -53,9 +71,52 @@ static void atexit_f() { } at_quick_exit(atexit_f); +void atExit0() { printf("atExit0\n"); } +void atExit1() { printf("atExit1\n"); } +void atExit2() { printf("atExit2\n"); } +void atExitA() { + printf("atExitA\n"); + std::atexit(&atExit0); +} +void atExitB() { + printf("atExitB\n"); + std::atexit(&atExit1); + std::atexit(&atExit2); +} +// Recursion in a Transaction +{ + std::atexit(&atExitA); + std::atexit(&atExitB); +} +.undo + // CHECK-NEXT: atExitB + // CHECK-NEXT: atExit2 + // CHECK-NEXT: atExit1 + // CHECK-NEXT: atExitA + // CHECK-NEXT: atExit0 + + // Recusion at shutdown + struct ShutdownRecursion { + static void DtorAtExit0() { printf("ShutdownRecursion0\n"); } + static void DtorAtExit1() { printf("ShutdownRecursion1\n"); } + static void DtorAtExit2() { printf("ShutdownRecursion2\n"); } + ~ShutdownRecursion() { + printf("~ShutdownRecursion\n"); + atexit(&DtorAtExit0); + atexit(&DtorAtExit1); + atexit(&DtorAtExit2); + } +} Out; + // expected-no-diagnostics .q -// Reversed registration order -// CHECK-NEXT: atexit_f true -// CHECK-NEXT: atexit_3 + // Reversed registration order + + // CHECK-NEXT: ~ShutdownRecursion + // CHECK-NEXT: ShutdownRecursion2 + // CHECK-NEXT: ShutdownRecursion1 + // CHECK-NEXT: ShutdownRecursion0 + + // CHECK-NEXT: atexit_f true + // CHECK-NEXT: atexit_3