Reactor: Fix all memory leaks with rr::DebugInfo.
DebugInfo::NotifyObjectEmitted and NotifyFreeingObject
have become statics as the free-object callback happens
after the DebugInfo instance is destroyed. This isn't
an issue as the JITEventListener is a singleton in LLVM.
Bug: b/133399620
Change-Id: I93fa73d0e416ba3c09c6550cc0d3abd56354e862
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/31837
Tested-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
diff --git a/src/Reactor/LLVMReactor.cpp b/src/Reactor/LLVMReactor.cpp
index 55865a8..171f9b1 100644
--- a/src/Reactor/LLVMReactor.cpp
+++ b/src/Reactor/LLVMReactor.cpp
@@ -643,18 +643,12 @@
ObjLayer::NotifyLoadedFtor(),
[](llvm::orc::VModuleKey, const llvm::object::ObjectFile &Obj, const llvm::RuntimeDyld::LoadedObjectInfo &L) {
#ifdef ENABLE_RR_DEBUG_INFO
- if (debugInfo != nullptr)
- {
- debugInfo->NotifyObjectEmitted(Obj, L);
- }
+ DebugInfo::NotifyObjectEmitted(Obj, L);
#endif // ENABLE_RR_DEBUG_INFO
},
[](llvm::orc::VModuleKey, const llvm::object::ObjectFile &Obj) {
#ifdef ENABLE_RR_DEBUG_INFO
- if (debugInfo != nullptr)
- {
- debugInfo->NotifyFreeingObject(Obj);
- }
+ DebugInfo::NotifyFreeingObject(Obj);
#endif // ENABLE_RR_DEBUG_INFO
}
),
@@ -963,6 +957,10 @@
Nucleus::~Nucleus()
{
+#ifdef ENABLE_RR_DEBUG_INFO
+ debugInfo.reset(nullptr);
+#endif // ENABLE_RR_DEBUG_INFO
+
::reactorJIT->endSession();
::codegenMutex.unlock();
diff --git a/src/Reactor/LLVMReactorDebugInfo.cpp b/src/Reactor/LLVMReactorDebugInfo.cpp
index d021a5f..744ae42 100644
--- a/src/Reactor/LLVMReactorDebugInfo.cpp
+++ b/src/Reactor/LLVMReactorDebugInfo.cpp
@@ -29,6 +29,7 @@
#include <cctype>
#include <fstream>
+#include <mutex>
#include <regex>
#include <sstream>
#include <string>
@@ -45,6 +46,12 @@
{
return llvm::StringRef(path).rsplit('/');
}
+
+ // Note: createGDBRegistrationListener() returns a pointer to a singleton.
+ // Nothing is actually created.
+ auto jitEventListener = llvm::JITEventListener::createGDBRegistrationListener(); // guarded by jitEventListenerMutex
+ std::mutex jitEventListenerMutex;
+
} // anonymous namespaces
namespace rr
@@ -61,14 +68,13 @@
auto location = getCallerLocation();
auto fileAndDir = splitPath(location.function.file.c_str());
- diBuilder = new llvm::DIBuilder(*module);
+ diBuilder.reset(new llvm::DIBuilder(*module));
diCU = diBuilder->createCompileUnit(
llvm::dwarf::DW_LANG_C,
diBuilder->createFile(fileAndDir.first, fileAndDir.second),
"Reactor",
0, "", 0);
- jitEventListener = llvm::JITEventListener::createGDBRegistrationListener();
registerBasicTypes();
SmallVector<Metadata *, 8> EltTys;
@@ -94,11 +100,13 @@
builder->SetCurrentDebugLocation(diRootLocation);
}
+ DebugInfo::~DebugInfo() = default;
+
void DebugInfo::Finalize()
{
while (diScope.size() > 0)
{
- emitPending(diScope.back(), builder, diBuilder);
+ emitPending(diScope.back(), builder);
diScope.pop_back();
}
diBuilder->finalize();
@@ -127,7 +135,7 @@
void DebugInfo::Flush()
{
- emitPending(diScope.back(), builder, diBuilder);
+ emitPending(diScope.back(), builder);
}
void DebugInfo::syncScope(Backtrace const& backtrace)
@@ -141,7 +149,7 @@
int(diScope.size() - 1), scope.di,
scope.location.function.file.c_str(),
int(scope.location.line));
- emitPending(scope, builder, diBuilder);
+ emitPending(scope, builder);
diScope.pop_back();
}
};
@@ -172,7 +180,7 @@
auto di = diBuilder->createLexicalBlock(scope.di, file, newLocation.line, 0);
LOG(" STACK(%d): Jumped backwards %d -> %d. di: %p -> %p", int(i),
oldLocation.line, newLocation.line, scope.di, di);
- emitPending(scope, builder, diBuilder);
+ emitPending(scope, builder);
scope = {newLocation, di};
shrink(i+1);
break;
@@ -263,7 +271,7 @@
auto &scope = diScope[i];
if (scope.pending.location != location)
{
- emitPending(scope, builder, diBuilder);
+ emitPending(scope, builder);
}
auto value = V(variable);
@@ -297,7 +305,7 @@
}
}
- void DebugInfo::emitPending(Scope &scope, IRBuilder *builder, llvm::DIBuilder *diBuilder)
+ void DebugInfo::emitPending(Scope &scope, IRBuilder *builder)
{
auto const &pending = scope.pending;
if (pending.value == nullptr)
@@ -373,11 +381,13 @@
void DebugInfo::NotifyObjectEmitted(const llvm::object::ObjectFile &Obj, const llvm::LoadedObjectInfo &L)
{
+ std::unique_lock<std::mutex> lock(jitEventListenerMutex);
jitEventListener->NotifyObjectEmitted(Obj, static_cast<const llvm::RuntimeDyld::LoadedObjectInfo&>(L));
}
void DebugInfo::NotifyFreeingObject(const llvm::object::ObjectFile &Obj)
{
+ std::unique_lock<std::mutex> lock(jitEventListenerMutex);
jitEventListener->NotifyFreeingObject(Obj);
}
diff --git a/src/Reactor/LLVMReactorDebugInfo.hpp b/src/Reactor/LLVMReactorDebugInfo.hpp
index d8c83de..db743d7 100644
--- a/src/Reactor/LLVMReactorDebugInfo.hpp
+++ b/src/Reactor/LLVMReactorDebugInfo.hpp
@@ -71,6 +71,8 @@
llvm::Module *module,
llvm::Function *function);
+ ~DebugInfo();
+
// Finalize debug info generation. Must be called before the LLVM module
// is built.
void Finalize();
@@ -88,11 +90,11 @@
// NotifyObjectEmitted informs any attached debuggers of the JIT'd
// object.
- void NotifyObjectEmitted(const llvm::object::ObjectFile &Obj, const llvm::LoadedObjectInfo &L);
+ static void NotifyObjectEmitted(const llvm::object::ObjectFile &Obj, const llvm::LoadedObjectInfo &L);
// NotifyFreeingObject informs any attached debuggers that the JIT'd
// object is now invalid.
- void NotifyFreeingObject(const llvm::object::ObjectFile &Obj);
+ static void NotifyFreeingObject(const llvm::object::ObjectFile &Obj);
private:
struct Token
@@ -168,7 +170,7 @@
void registerBasicTypes();
- void emitPending(Scope &scope, IRBuilder *builder, llvm::DIBuilder *diBuilder);
+ void emitPending(Scope &scope, IRBuilder *builder);
// Returns the source location of the non-Reactor calling function.
Location getCallerLocation() const;
@@ -192,7 +194,7 @@
llvm::Module *module;
llvm::Function *function;
- llvm::DIBuilder *diBuilder;
+ std::unique_ptr<llvm::DIBuilder> diBuilder;
llvm::DICompileUnit *diCU;
llvm::DISubprogram *diSubprogram;
llvm::DILocation *diRootLocation;
@@ -200,7 +202,6 @@
std::unordered_map<std::string, llvm::DIFile*> diFiles;
std::unordered_map<llvm::Type*, llvm::DIType*> diTypes;
std::unordered_map<std::string, std::unique_ptr<LineTokens>> fileTokens;
- llvm::JITEventListener *jitEventListener;
std::vector<void const*> pushed;
};
