Cling MR

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 <cassert>
+#include <unordered_map>
+
+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 <typename Key, typename Value> class OrderedMap {
+  typedef std::unordered_map<Key, Value> map_t;
+  // Would this be faster as a std::unoredered_map<Key, size_t> for erasure?
+  typedef std::vector<typename map_t::const_iterator> 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 <typename... Args>
+  std::pair<iterator, bool> emplace(Args&&... args) {
+    auto Rval = m_Map.emplace(std::forward<Args>(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

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<TargetMachine> TM(CreateHostTargetMachine(CI));
   m_BackendPasses.reset(new BackendPasses(CI.getCodeGenOpts(),
                                           CI.getTargetOpts(),
@@ -108,19 +104,31 @@ 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();
   }
 }
 
 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<llvm::Function*, 128> funcsToFree;
-  for (std::set<std::string>::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 "

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 <vector>
-#include <set>
+#include <atomic>
 #include <map>
 #include <memory>
-#include <atomic>
+#include <unordered_set>
+#include <vector>
 
 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<CXAAtExitElement, 128> 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<llvm::Module*, std::vector<CXAAtExitElement>>
+        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<std::string> m_unresolvedSymbols;
+    std::unordered_set<std::string> m_unresolvedSymbols;
 
 #if 0 // See FIXME in IncrementalExecutor.cpp
     ///\brief The diagnostics engine, printing out issues coming from the