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; };