Skip to content

Commit

Permalink
ManagedStatic: remove from GDBRegistrationListener
Browse files Browse the repository at this point in the history
An earlier version of this change originally landed as part of
e6f1f06 (D129120), which caused a
Fuchsia buildbot regression in ExecutionEngine tests.

Careful review suggests that the issue was that in the earlier version,
the destructor of the JITDebugLock was run before the destructor of
GDBJITRegistrationListener. The new version of the change moves the lock
to a member variable of the (singleton!) GDBJITRegistartionListener so
that destructors are run in the right order.
  • Loading branch information
nhaehnle committed Jul 10, 2022
1 parent d559857 commit c4ccf60
Showing 1 changed file with 18 additions and 12 deletions.
30 changes: 18 additions & 12 deletions llvm/lib/ExecutionEngine/GDBRegistrationListener.cpp
Expand Up @@ -12,7 +12,6 @@
#include "llvm/Object/ObjectFile.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/ManagedStatic.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Mutex.h"
#include <mutex>
Expand Down Expand Up @@ -91,18 +90,31 @@ typedef llvm::DenseMap<JITEventListener::ObjectKey, RegisteredObjectInfo>
/// object files that are in executable memory managed by the client of this
/// class.
class GDBJITRegistrationListener : public JITEventListener {
/// Lock used to serialize all jit registration events, since they
/// modify global variables.
///
/// Only a single instance of GDBJITRegistrationListener is ever created,
/// and so the lock can be a member variable of that instance. This ensures
/// destructors are run in the correct order.
sys::Mutex JITDebugLock;

/// A map of in-memory object files that have been registered with the
/// JIT interface.
RegisteredObjectBufferMap ObjectBufferMap;

public:
/// Instantiates the JIT service.
GDBJITRegistrationListener() = default;

/// Unregisters each object that was previously registered and releases all
/// internal resources.
~GDBJITRegistrationListener() override;

public:
static GDBJITRegistrationListener &instance() {
static GDBJITRegistrationListener Instance;
return Instance;
}

/// Creates an entry in the JIT registry for the buffer @p Object,
/// which must contain an object file in executable memory with any
/// debug information for the debugger.
Expand All @@ -121,10 +133,6 @@ class GDBJITRegistrationListener : public JITEventListener {
void deregisterObjectInternal(RegisteredObjectBufferMap::iterator I);
};

/// Lock used to serialize all jit registration events, since they
/// modify global variables.
ManagedStatic<sys::Mutex> JITDebugLock;

/// Do the registration.
void NotifyDebugger(jit_code_entry* JITCodeEntry) {
__jit_debug_descriptor.action_flag = JIT_REGISTER_FN;
Expand All @@ -143,7 +151,7 @@ void NotifyDebugger(jit_code_entry* JITCodeEntry) {

GDBJITRegistrationListener::~GDBJITRegistrationListener() {
// Free all registered object files.
std::lock_guard<llvm::sys::Mutex> locked(*JITDebugLock);
std::lock_guard<llvm::sys::Mutex> locked(JITDebugLock);
for (RegisteredObjectBufferMap::iterator I = ObjectBufferMap.begin(),
E = ObjectBufferMap.end();
I != E; ++I) {
Expand All @@ -167,7 +175,7 @@ void GDBJITRegistrationListener::notifyObjectLoaded(
const char *Buffer = DebugObj.getBinary()->getMemoryBufferRef().getBufferStart();
size_t Size = DebugObj.getBinary()->getMemoryBufferRef().getBufferSize();

std::lock_guard<llvm::sys::Mutex> locked(*JITDebugLock);
std::lock_guard<llvm::sys::Mutex> locked(JITDebugLock);
assert(ObjectBufferMap.find(K) == ObjectBufferMap.end() &&
"Second attempt to perform debug registration.");
jit_code_entry* JITCodeEntry = new jit_code_entry();
Expand All @@ -186,7 +194,7 @@ void GDBJITRegistrationListener::notifyObjectLoaded(
}

void GDBJITRegistrationListener::notifyFreeingObject(ObjectKey K) {
std::lock_guard<llvm::sys::Mutex> locked(*JITDebugLock);
std::lock_guard<llvm::sys::Mutex> locked(JITDebugLock);
RegisteredObjectBufferMap::iterator I = ObjectBufferMap.find(K);

if (I != ObjectBufferMap.end()) {
Expand Down Expand Up @@ -228,14 +236,12 @@ void GDBJITRegistrationListener::deregisterObjectInternal(
JITCodeEntry = nullptr;
}

llvm::ManagedStatic<GDBJITRegistrationListener> GDBRegListener;

} // end namespace

namespace llvm {

JITEventListener* JITEventListener::createGDBRegistrationListener() {
return &*GDBRegListener;
return &GDBJITRegistrationListener::instance();
}

} // namespace llvm
Expand Down

0 comments on commit c4ccf60

Please sign in to comment.