Fix registration of other atexit functions during an atexit handler. Recursive registration of atexit handlers is legal and should be handled, not ignored.

This commit is contained in:
Frederich Munch 2017-07-12 15:04:11 -04:00 committed by sftnight
parent 5c41b2957c
commit 38c4b902cf
4 changed files with 216 additions and 32 deletions

View File

@ -0,0 +1,116 @@
//--------------------------------------------------------------------*- 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 <unordered_map>
#include <cassert>
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

View File

@ -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(),
@ -107,18 +103,33 @@ IncrementalExecutor::IncrementalExecutor(clang::DiagnosticsEngine& diags,
// Keep in source: ~unique_ptr<ClingJIT> needs ClingJIT
IncrementalExecutor::~IncrementalExecutor() {}
void IncrementalExecutor::shuttingDown() {
// No need to protect this access, since hopefully there is no concurrent
// shutdown request.
for (auto& AtExit : llvm::reverse(m_AtExitFuncs))
AtExit();
// 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()
@ -286,23 +297,21 @@ 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->getModule() == 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 (auto&& AtExit : llvm::reverse(boundToT))
// 'Unload' the cxa_atexit, atexit entities.
for (auto&& AtExit : llvm::reverse(Local)) {
AtExit();
// Run anything that was just registered in 'AtExit()'
runAndRemoveStaticDestructors(T);
}
}
void

View File

@ -16,6 +16,7 @@
#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"
@ -73,10 +74,6 @@ namespace cling {
///
void* m_Arg;
///\brief The module whose unloading will trigger the call to this atexit
/// function.
///
const llvm::Module* m_FromM;
public:
///\brief Constructs an element, whose destruction time will be managed by
/// the interpreter. (By registering a function to be called by exit
@ -96,12 +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) {}
CXAAtExitElement(void (*func) (void*), void* arg):
m_Func(func), m_Arg(arg) {}
void operator () () const { (*m_Func)(m_Arg); }
const llvm::Module* getModule() const { return m_FromM; }
};
///\brief Atomic used as a spin lock to protect the access to m_AtExitFuncs
@ -112,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.

View File

@ -35,6 +35,20 @@ at_quick_exit(atexit_2);
// Make sure at_quick_exit is resolved correctly (mangling issues on gcc < 5)
// CHECK-NEXT: atexit_2
// 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 +67,58 @@ 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: ~ShutdownRecursion
// CHECK-NEXT: ShutdownRecursion2
// CHECK-NEXT: ShutdownRecursion1
// CHECK-NEXT: ShutdownRecursion0
// CHECK-NEXT: atexit_f true
// CHECK-NEXT: atexit_3