From c3e3b158600f14994dec66aed32a664d8c49ff83 Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Mon, 11 Aug 2025 14:40:26 +0200 Subject: [PATCH 1/3] add mutex per interpreter per thread --- lib/CppInterOp/CppInterOp.cpp | 215 ++++++++++++++++++++++++++++++---- 1 file changed, 194 insertions(+), 21 deletions(-) diff --git a/lib/CppInterOp/CppInterOp.cpp b/lib/CppInterOp/CppInterOp.cpp index e1ed3601c..2492630df 100755 --- a/lib/CppInterOp/CppInterOp.cpp +++ b/lib/CppInterOp/CppInterOp.cpp @@ -62,10 +62,12 @@ #include #include #include +#include #include #include #include #include +#include // Stream redirect. #ifdef _WIN32 @@ -88,9 +90,76 @@ using namespace clang; using namespace llvm; using namespace std; +#define LOCK(InterpInfo) \ + LockRAII interop_lock = \ + LockRAII((InterpInfo).InterpreterLock, (InterpInfo).ThreadIdLock, \ + (InterpInfo).CurrentThreadId) + +namespace { +struct LockRAII { + LockRAII(std::mutex& resource_lock, std::mutex& thread_id_lock, + std::optional& current_thread_id) + : ResourceLock(resource_lock), ThreadIdLock(thread_id_lock), + CurrentThreadId(current_thread_id) { + ThreadIdLock.lock(); + if (!ResourceLock.try_lock()) { + if (CurrentThreadId) { + if ((*CurrentThreadId) != std::this_thread::get_id()) { + ThreadIdLock.unlock(); + ResourceLock.lock(); // blocking + bool res = ThreadIdLock.try_lock(); // should be free + assert(res && "Internal Error: Interpreter lock not held, but " + "ThreadId lock held.\nPlease report this as a bug.\n"); + ThreadIdLock.unlock(); + return; + } + owned = false; + ThreadIdLock.unlock(); + return; + } + assert(false && + "Internal Error: A thread holds lock for the current " + "interpreter, but no information available on which thread " + "holds the lock.\nPlease report this as a bug.\n"); + } + assert(!CurrentThreadId && + "Internal Error: Lock released but thread id not cleared.\nPlease " + "report this as a bug.\n"); + CurrentThreadId = std::this_thread::get_id(); + ThreadIdLock.unlock(); + } + + ~LockRAII() { + assert(!ResourceLock.try_lock() && + "Internal Error: Resource lock not held"); + if (!owned) + return; + ThreadIdLock.lock(); + CurrentThreadId = std::nullopt; + ThreadIdLock.unlock(); + ResourceLock.unlock(); + } + + LockRAII(LockRAII&&) = delete; + LockRAII(const LockRAII&) = delete; + LockRAII& operator=(const LockRAII&) = delete; + LockRAII& operator=(LockRAII&&) = delete; + +private: + bool owned = true; + std::mutex& ResourceLock; + std::mutex& ThreadIdLock; + std::optional& CurrentThreadId; +}; +} // namespace + struct InterpreterInfo { compat::Interpreter* Interpreter = nullptr; bool isOwned = true; + std::mutex InterpreterLock; + std::mutex ThreadIdLock; + std::optional CurrentThreadId = std::nullopt; + InterpreterInfo(compat::Interpreter* I, bool Owned) : Interpreter(I), isOwned(Owned) {} @@ -127,8 +196,17 @@ struct InterpreterInfo { // std::deque avoids relocations and calling the dtor of InterpreterInfo. static llvm::ManagedStatic> sInterpreters; +static std::mutex InterpreterStackLock; + +static InterpreterInfo& getInterpInfo() { + std::unique_lock Lock(InterpreterStackLock); + assert(!sInterpreters->empty() && + "Interpreter instance must be set before calling this!"); + return sInterpreters->back(); +} static compat::Interpreter& getInterp() { + std::unique_lock Lock(InterpreterStackLock); assert(!sInterpreters->empty() && "Interpreter instance must be set before calling this!"); return *sInterpreters->back().Interpreter; @@ -226,11 +304,18 @@ std::string Demangle(const std::string& mangled_name) { return demangle; } -void EnableDebugOutput(bool value /* =true*/) { llvm::DebugFlag = value; } +void EnableDebugOutput(bool value /* =true*/) { + LOCK(getInterpInfo()); + llvm::DebugFlag = value; +} -bool IsDebugOutputEnabled() { return llvm::DebugFlag; } +bool IsDebugOutputEnabled() { + LOCK(getInterpInfo()); + return llvm::DebugFlag; +} static void InstantiateFunctionDefinition(Decl* D) { + LOCK(getInterpInfo()); compat::SynthesizingCodeRAII RAII(&getInterp()); if (auto* FD = llvm::dyn_cast_or_null(D)) { getSema().InstantiateFunctionDefinition(SourceLocation(), FD, @@ -240,6 +325,7 @@ static void InstantiateFunctionDefinition(Decl* D) { } bool IsAggregate(TCppScope_t scope) { + LOCK(getInterpInfo()); Decl* D = static_cast(scope); // Aggregates are only arrays or tag decls. @@ -275,6 +361,7 @@ bool IsFunctionPointerType(TCppType_t type) { } bool IsClassPolymorphic(TCppScope_t klass) { + LOCK(getInterpInfo()); Decl* D = static_cast(klass); if (auto* CXXRD = llvm::dyn_cast(D)) if (auto* CXXRDD = CXXRD->getDefinition()) @@ -283,6 +370,7 @@ bool IsClassPolymorphic(TCppScope_t klass) { } static SourceLocation GetValidSLoc(Sema& semaRef) { + LOCK(getInterpInfo()); auto& SM = semaRef.getSourceManager(); return SM.getLocForStartOfFile(SM.getMainFileID()); } @@ -294,6 +382,7 @@ bool IsComplete(TCppScope_t scope) { Decl* D = static_cast(scope); + LOCK(getInterpInfo()); if (isa(D)) { QualType QT = QualType::getFromOpaquePtr(GetTypeFromScope(scope)); clang::Sema& S = getSema(); @@ -318,6 +407,7 @@ size_t SizeOf(TCppScope_t scope) { if (!IsComplete(scope)) return 0; + LOCK(getInterpInfo()); if (auto* RD = dyn_cast(static_cast(scope))) { ASTContext& Context = RD->getASTContext(); const ASTRecordLayout& Layout = Context.getASTRecordLayout(RD); @@ -352,8 +442,10 @@ bool IsTypedefed(TCppScope_t handle) { bool IsAbstract(TCppType_t klass) { auto* D = (clang::Decl*)klass; - if (auto* CXXRD = llvm::dyn_cast_or_null(D)) + if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { + LOCK(getInterpInfo()); return CXXRD->isAbstract(); + } return false; } @@ -374,6 +466,7 @@ bool IsEnumType(TCppType_t type) { } static bool isSmartPointer(const RecordType* RT) { + LOCK(getInterpInfo()); auto IsUseCountPresent = [](const RecordDecl* Record) { ASTContext& C = Record->getASTContext(); return !Record->lookup(&C.Idents.get("use_count")).empty(); @@ -453,6 +546,7 @@ TCppType_t GetIntegerTypeFromEnumType(TCppType_t enum_type) { std::vector GetEnumConstants(TCppScope_t handle) { auto* D = (clang::Decl*)handle; + LOCK(getInterpInfo()); if (auto* ED = llvm::dyn_cast_or_null(D)) { std::vector enum_constants; for (auto* ECD : ED->enumerators()) { @@ -486,6 +580,7 @@ TCppIndex_t GetEnumConstantValue(TCppScope_t handle) { } size_t GetSizeOfType(TCppType_t type) { + LOCK(getInterpInfo()); QualType QT = QualType::getFromOpaquePtr(type); if (const TagType* TT = QT->getAs()) return SizeOf(TT->getDecl()); @@ -594,6 +689,7 @@ std::string GetQualifiedCompleteName(TCppType_t klass) { } std::vector GetUsingNamespaces(TCppScope_t scope) { + LOCK(getInterpInfo()); auto* D = (clang::Decl*)scope; if (auto* DC = llvm::dyn_cast_or_null(D)) { @@ -683,6 +779,7 @@ TCppScope_t GetScopeFromCompleteName(const std::string& name) { TCppScope_t GetNamed(const std::string& name, TCppScope_t parent /*= nullptr*/) { + LOCK(getInterpInfo()); clang::DeclContext* Within = 0; if (parent) { auto* D = (clang::Decl*)parent; @@ -718,6 +815,7 @@ TCppScope_t GetParentScope(TCppScope_t scope) { } TCppIndex_t GetNumBases(TCppScope_t klass) { + LOCK(getInterpInfo()); auto* D = (Decl*)klass; if (auto* CTSD = llvm::dyn_cast_or_null(D)) @@ -732,6 +830,7 @@ TCppIndex_t GetNumBases(TCppScope_t klass) { } TCppScope_t GetBaseClass(TCppScope_t klass, TCppIndex_t ibase) { + LOCK(getInterpInfo()); auto* D = (Decl*)klass; auto* CXXRD = llvm::dyn_cast_or_null(D); if (!CXXRD || CXXRD->getNumBases() <= ibase) @@ -771,6 +870,7 @@ bool IsSubclass(TCppScope_t derived, TCppScope_t base) { static unsigned ComputeBaseOffset(const ASTContext& Context, const CXXRecordDecl* DerivedRD, const CXXBasePath& Path) { + LOCK(getInterpInfo()); CharUnits NonVirtualOffset = CharUnits::Zero(); unsigned NonVirtualStart = 0; @@ -824,6 +924,9 @@ int64_t GetBaseClassOffset(TCppScope_t derived, TCppScope_t base) { return -1; CXXRecordDecl* DCXXRD = cast(DD); CXXRecordDecl* BCXXRD = cast(BD); + + LOCK(getInterpInfo()); + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, /*DetectVirtual=*/false); DCXXRD->isDerivedFrom(BCXXRD, Paths); @@ -838,6 +941,7 @@ static void GetClassDecls(TCppScope_t klass, if (!klass) return; + LOCK(getInterpInfo()); auto* D = (clang::Decl*)klass; if (auto* TD = dyn_cast(D)) @@ -895,6 +999,7 @@ void GetFunctionTemplatedDecls(TCppScope_t klass, bool HasDefaultConstructor(TCppScope_t scope) { auto* D = (clang::Decl*)scope; + LOCK(getInterpInfo()); if (auto* CXXRD = llvm::dyn_cast_or_null(D)) return CXXRD->hasDefaultConstructor(); @@ -906,6 +1011,7 @@ TCppFunction_t GetDefaultConstructor(compat::Interpreter& interp, if (!HasDefaultConstructor(scope)) return nullptr; + LOCK(getInterpInfo()); auto* CXXRD = (clang::CXXRecordDecl*)scope; return interp.getCI()->getSema().LookupDefaultConstructor(CXXRD); } @@ -918,6 +1024,7 @@ TCppFunction_t GetDestructor(TCppScope_t scope) { auto* D = (clang::Decl*)scope; if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { + LOCK(getInterpInfo()); getSema().ForceDeclarationOfImplicitMembers(CXXRD); return CXXRD->getDestructor(); } @@ -937,6 +1044,8 @@ std::vector GetFunctionsUsingName(TCppScope_t scope, if (!scope || name.empty()) return {}; + LOCK(getInterpInfo()); + D = GetUnderlyingScope(D); std::vector funcs; @@ -961,6 +1070,8 @@ std::vector GetFunctionsUsingName(TCppScope_t scope, } TCppType_t GetFunctionReturnType(TCppFunction_t func) { + LOCK(getInterpInfo()); + auto* D = (clang::Decl*)func; if (auto* FD = llvm::dyn_cast_or_null(D)) { QualType Type = FD->getReturnType(); @@ -1010,8 +1121,9 @@ TCppIndex_t GetFunctionRequiredArgs(TCppConstFunction_t func) { } TCppType_t GetFunctionArgType(TCppFunction_t func, TCppIndex_t iarg) { - auto* D = (clang::Decl*)func; + LOCK(getInterpInfo()); + auto* D = (clang::Decl*)func; if (auto* FD = llvm::dyn_cast_or_null(D)) { if (iarg < FD->getNumParams()) { auto* PVD = FD->getParamDecl(iarg); @@ -1090,6 +1202,8 @@ bool ExistsFunctionTemplate(const std::string& name, TCppScope_t parent) { Within = llvm::dyn_cast(D); } + LOCK(getInterpInfo()); + auto* ND = Cpp_utils::Lookup::Named(&getSema(), name, Within); if ((intptr_t)ND == (intptr_t)0) @@ -1109,6 +1223,8 @@ void LookupConstructors(const std::string& name, TCppScope_t parent, auto* D = (Decl*)parent; if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { + LOCK(getInterpInfo()); + getSema().ForceDeclarationOfImplicitMembers(CXXRD); DeclContextLookupResult Result = getSema().LookupConstructors(CXXRD); // Obtaining all constructors when we intend to lookup a method under a @@ -1126,6 +1242,8 @@ bool GetClassTemplatedMethods(const std::string& name, TCppScope_t parent, if (!D && name.empty()) return false; + LOCK(getInterpInfo()); + // Accumulate constructors LookupConstructors(name, parent, funcs); auto& S = getSema(); @@ -1167,6 +1285,8 @@ TCppFunction_t BestOverloadFunctionMatch(const std::vector& candidates, const std::vector& explicit_types, const std::vector& arg_types) { + LOCK(getInterpInfo()); + auto& S = getSema(); auto& C = S.getASTContext(); @@ -1288,6 +1408,8 @@ bool IsDestructor(TCppConstFunction_t method) { } bool IsStaticMethod(TCppConstFunction_t method) { + LOCK(getInterpInfo()); + const auto* D = static_cast(method); if (auto* CXXMD = llvm::dyn_cast_or_null(D)) { return CXXMD->isStatic(); @@ -1341,6 +1463,7 @@ TCppFuncAddr_t GetFunctionAddress(TCppFunction_t method) { } bool IsVirtualMethod(TCppFunction_t method) { + LOCK(getInterpInfo()); auto* D = (Decl*)method; if (auto* CXXMD = llvm::dyn_cast_or_null(D)) { return CXXMD->isVirtual(); @@ -1353,6 +1476,8 @@ void GetDatamembers(TCppScope_t scope, std::vector& datamembers) { auto* D = (Decl*)scope; if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { + LOCK(getInterpInfo()); + getSema().ForceDeclarationOfImplicitMembers(CXXRD); if (CXXRD->hasDefinition()) CXXRD = CXXRD->getDefinition(); @@ -1398,6 +1523,8 @@ void GetStaticDatamembers(TCppScope_t scope, void GetEnumConstantDatamembers(TCppScope_t scope, std::vector& datamembers, bool include_enum_class) { + LOCK(getInterpInfo()); + std::vector EDs; GetClassDecls(scope, EDs); for (TCppScope_t i : EDs) { @@ -1419,6 +1546,7 @@ TCppScope_t LookupDatamember(const std::string& name, TCppScope_t parent) { Within = llvm::dyn_cast(D); } + LOCK(getInterpInfo()); auto* ND = Cpp_utils::Lookup::Named(&getSema(), name, Within); if (ND && ND != (clang::NamedDecl*)-1) { if (llvm::isa_and_nonnull(ND)) { @@ -1464,6 +1592,8 @@ intptr_t GetVariableOffset(compat::Interpreter& I, Decl* D, if (!D) return 0; + LOCK(getInterpInfo()); + auto& C = I.getSema().getASTContext(); if (auto* FD = llvm::dyn_cast(D)) { @@ -1626,11 +1756,7 @@ bool IsPrivateVariable(TCppScope_t var) { bool IsStaticVariable(TCppScope_t var) { auto* D = (Decl*)var; - if (llvm::isa_and_nonnull(D)) { - return true; - } - - return false; + return llvm::isa_and_nonnull(D); } bool IsConstVariable(TCppScope_t var) { @@ -1864,6 +1990,7 @@ TCppType_t GetType(const std::string& name) { if (!builtin.isNull()) return builtin.getAsOpaquePtr(); + LOCK(getInterpInfo()); auto* D = (Decl*)GetNamed(name, /* Within= */ 0); if (auto* TD = llvm::dyn_cast_or_null(D)) { return QualType(TD->getTypeForDecl(), 0).getAsOpaquePtr(); @@ -1875,6 +2002,7 @@ TCppType_t GetType(const std::string& name) { TCppType_t GetComplexType(TCppType_t type) { QualType QT = QualType::getFromOpaquePtr(type); + LOCK(getInterpInfo()); return getASTContext().getComplexType(QT).getAsOpaquePtr(); } @@ -1914,6 +2042,7 @@ void* compile_wrapper(compat::Interpreter& I, const std::string& wrapper_name, const std::string& wrapper, bool withAccessControl = true) { LLVM_DEBUG(dbgs() << "Compiling '" << wrapper_name << "'\n"); + LOCK(getInterpInfo()); return I.compileFunction(wrapper_name, wrapper, false /*ifUnique*/, withAccessControl); } @@ -1922,8 +2051,10 @@ void get_type_as_string(QualType QT, std::string& type_name, ASTContext& C, PrintingPolicy Policy) { // TODO: Implement cling desugaring from utils::AST // cling::utils::Transform::GetPartiallyDesugaredType() - if (!QT->isTypedefNameType() || QT->isBuiltinType()) + if (!QT->isTypedefNameType() || QT->isBuiltinType()) { + LOCK(getInterpInfo()); QT = QT.getDesugaredType(C); + } Policy.SuppressElaboration = true; Policy.SuppressTagKeyword = !QT->isEnumeralType(); Policy.FullyQualifiedName = true; @@ -2272,12 +2403,14 @@ void make_narg_call(const FunctionDecl* FD, const std::string& return_type, // a simple way of determining whether a viable copy constructor exists, // so check for the most common case: the trivial one, but not uniquely // available, while there is a move constructor. - - // include utility header if not already included for std::move - DeclarationName DMove = &getASTContext().Idents.get("move"); - auto result = getSema().getStdNamespace()->lookup(DMove); - if (result.empty()) - Cpp::Declare("#include "); + { + LOCK(getInterpInfo()); + // include utility header if not already included for std::move + DeclarationName DMove = &getASTContext().Idents.get("move"); + auto result = getSema().getStdNamespace()->lookup(DMove); + if (result.empty()) + Cpp::Declare("#include "); + } // move construction as needed for classes (note that this is implicit) callbuf << "std::move(*(" << type_name.c_str() << "*)args[" << i << "])"; @@ -2488,6 +2621,8 @@ void make_narg_call_with_return(compat::Interpreter& I, const FunctionDecl* FD, int get_wrapper_code(compat::Interpreter& I, const FunctionDecl* FD, std::string& wrapper_name, std::string& wrapper) { assert(FD && "generate_wrapper called without a function decl!"); + LOCK(getInterpInfo()); + ASTContext& Context = FD->getASTContext(); // // Get the class or namespace name. @@ -2908,6 +3043,8 @@ JitCall::GenericCall make_wrapper(compat::Interpreter& I, const FunctionDecl* FD) { static std::map gWrapperStore; + LOCK(getInterpInfo()); + auto R = gWrapperStore.find(FD); if (R != gWrapperStore.end()) return (JitCall::GenericCall)R->second; @@ -3000,6 +3137,8 @@ static JitCall::DestructorCall make_dtor_wrapper(compat::Interpreter& interp, static map gDtorWrapperStore; + LOCK(getInterpInfo()); + auto I = gDtorWrapperStore.find(D); if (I != gDtorWrapperStore.end()) return (JitCall::DestructorCall)I->second; @@ -3179,6 +3318,7 @@ static std::string MakeResourcesPath() { TInterp_t CreateInterpreter(const std::vector& Args /*={}*/, const std::vector& GpuArgs /*={}*/) { + std::unique_lock Lock(InterpreterStackLock); std::string MainExecutableName = sys::fs::getMainExecutable(nullptr, nullptr); std::string ResourceDir = MakeResourcesPath(); std::vector ClingArgv = {"-resource-dir", ResourceDir.c_str(), @@ -3260,6 +3400,8 @@ TInterp_t CreateInterpreter(const std::vector& Args /*={}*/, } bool DeleteInterpreter(TInterp_t I /*=nullptr*/) { + std::unique_lock Lock(InterpreterStackLock); + if (!I) { sInterpreters->pop_back(); return true; @@ -3271,11 +3413,14 @@ bool DeleteInterpreter(TInterp_t I /*=nullptr*/) { if (found == sInterpreters->end()) return false; // failure + LOCK(*found); sInterpreters->erase(found); return true; } bool ActivateInterpreter(TInterp_t I) { + std::unique_lock Lock(InterpreterStackLock); + if (!I) return false; @@ -3292,18 +3437,21 @@ bool ActivateInterpreter(TInterp_t I) { } TInterp_t GetInterpreter() { + std::unique_lock Lock(InterpreterStackLock); if (sInterpreters->empty()) return nullptr; return sInterpreters->back().Interpreter; } void UseExternalInterpreter(TInterp_t I) { + std::unique_lock Lock(InterpreterStackLock); assert(sInterpreters->empty() && "sInterpreter already in use!"); sInterpreters->emplace_back(static_cast(I), /*isOwned=*/false); } void AddSearchPath(const char* dir, bool isUser, bool prepend) { + LOCK(getInterpInfo()); getInterp().getDynamicLibraryManager()->addSearchPath(dir, isUser, prepend); } @@ -3367,7 +3515,10 @@ void DetectSystemCompilerIncludePaths(std::vector& Paths, exec(cmd.c_str(), Paths); } -void AddIncludePath(const char* dir) { getInterp().AddIncludePath(dir); } +void AddIncludePath(const char* dir) { + LOCK(getInterpInfo()); + getInterp().AddIncludePath(dir); +} void GetIncludePaths(std::vector& IncludePaths, bool withSystem, bool withFlags) { @@ -3404,10 +3555,14 @@ int Declare(compat::Interpreter& I, const char* code, bool silent) { } int Declare(const char* code, bool silent) { + LOCK(getInterpInfo()); return Declare(getInterp(), code, silent); } -int Process(const char* code) { return getInterp().process(code); } +int Process(const char* code) { + LOCK(getInterpInfo()); + return getInterp().process(code); +} intptr_t Evaluate(const char* code, bool* HadError /*=nullptr*/) { #ifdef CPPINTEROP_USE_CLING @@ -3419,6 +3574,7 @@ intptr_t Evaluate(const char* code, bool* HadError /*=nullptr*/) { if (HadError) *HadError = false; + LOCK(getInterpInfo()); auto res = getInterp().evaluate(code, V); if (res != 0) { // 0 is success if (HadError) @@ -3435,6 +3591,7 @@ std::string LookupLibrary(const char* lib_name) { } bool LoadLibrary(const char* lib_stem, bool lookup) { + LOCK(getInterpInfo()); compat::Interpreter::CompilationResult res = getInterp().loadLibrary(lib_stem, lookup); @@ -3442,11 +3599,13 @@ bool LoadLibrary(const char* lib_stem, bool lookup) { } void UnloadLibrary(const char* lib_stem) { + LOCK(getInterpInfo()); getInterp().getDynamicLibraryManager()->unloadLibrary(lib_stem); } std::string SearchLibrariesForSymbol(const char* mangled_name, bool search_system /*true*/) { + LOCK(getInterpInfo()); auto* DLM = getInterp().getDynamicLibraryManager(); return DLM->searchLibrariesForSymbol(mangled_name, search_system); } @@ -3532,16 +3691,19 @@ bool InsertOrReplaceJitSymbol(compat::Interpreter& I, bool InsertOrReplaceJitSymbol(const char* linker_mangled_name, uint64_t address) { + LOCK(getInterpInfo()); return InsertOrReplaceJitSymbol(getInterp(), linker_mangled_name, address); } std::string ObjToString(const char* type, void* obj) { + LOCK(getInterpInfo()); return getInterp().toString(type, obj); } -static Decl* InstantiateTemplate(TemplateDecl* TemplateD, - TemplateArgumentListInfo& TLI, Sema& S, - bool instantiate_body) { +Decl* InstantiateTemplate(TemplateDecl* TemplateD, + TemplateArgumentListInfo& TLI, Sema& S, + bool instantiate_body) { + LOCK(getInterpInfo()); // This is not right but we don't have a lot of options to choose from as a // template instantiation requires a valid source location. SourceLocation fakeLoc = GetValidSLoc(S); @@ -3592,6 +3754,7 @@ static Decl* InstantiateTemplate(TemplateDecl* TemplateD, Decl* InstantiateTemplate(TemplateDecl* TemplateD, ArrayRef TemplateArgs, Sema& S, bool instantiate_body) { + LOCK(getInterpInfo()); // Create a list of template arguments. TemplateArgumentListInfo TLI{}; for (auto TA : TemplateArgs) @@ -3636,12 +3799,14 @@ TCppScope_t InstantiateTemplate(TCppScope_t tmpl, const TemplateArgInfo* template_args, size_t template_args_size, bool instantiate_body) { + LOCK(getInterpInfo()); return InstantiateTemplate(getInterp(), tmpl, template_args, template_args_size, instantiate_body); } void GetClassTemplateInstantiationArgs(TCppScope_t templ_instance, std::vector& args) { + LOCK(getInterpInfo()); auto* CTSD = static_cast(templ_instance); for (const auto& TA : CTSD->getTemplateInstantiationArgs().asArray()) { switch (TA.getKind()) { @@ -3674,6 +3839,7 @@ InstantiateTemplateFunctionFromString(const char* function_template) { std::string id = "__Cppyy_GetMethTmpl_" + std::to_string(var_count++); std::string instance = "auto " + id + " = " + function_template + ";\n"; + LOCK(getInterpInfo()); if (!Cpp::Declare(instance.c_str(), /*silent=*/false)) { VarDecl* VD = (VarDecl*)Cpp::GetNamed(id, 0); DeclRefExpr* DRE = (DeclRefExpr*)VD->getInit()->IgnoreImpCasts(); @@ -3687,6 +3853,7 @@ void GetAllCppNames(TCppScope_t scope, std::set& names) { clang::DeclContext* DC; clang::DeclContext::decl_iterator decl; + LOCK(getInterpInfo()); if (auto* TD = dyn_cast_or_null(D)) { DC = clang::TagDecl::castToDeclContext(TD); decl = DC->decls_begin(); @@ -3716,6 +3883,7 @@ void GetEnums(TCppScope_t scope, std::vector& Result) { auto* DC = llvm::dyn_cast(D); + LOCK(getInterpInfo()); llvm::SmallVector DCs; DC->collectAllContexts(DCs); @@ -3732,6 +3900,7 @@ void GetEnums(TCppScope_t scope, std::vector& Result) { // FIXME: On the CPyCppyy side the receiver is of type // vector instead of vector std::vector GetDimensions(TCppType_t type) { + LOCK(getInterpInfo()); QualType Qual = QualType::getFromOpaquePtr(type); if (Qual.isNull()) return {}; @@ -3758,6 +3927,7 @@ std::vector GetDimensions(TCppType_t type) { } bool IsTypeDerivedFrom(TCppType_t derived, TCppType_t base) { + LOCK(getInterpInfo()); auto& S = getSema(); auto fakeLoc = GetValidSLoc(S); auto derivedType = clang::QualType::getFromOpaquePtr(derived); @@ -3771,6 +3941,7 @@ bool IsTypeDerivedFrom(TCppType_t derived, TCppType_t base) { std::string GetFunctionArgDefault(TCppFunction_t func, TCppIndex_t param_index) { + LOCK(getInterpInfo()); auto* D = (clang::Decl*)func; clang::ParmVarDecl* PI = nullptr; @@ -3868,6 +4039,7 @@ OperatorArity GetOperatorArity(TCppFunction_t op) { void GetOperator(TCppScope_t scope, Operator op, std::vector& operators, OperatorArity kind) { + LOCK(getInterpInfo()); Decl* D = static_cast(scope); if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { auto fn = [&operators, kind, op](const RecordDecl* RD) { @@ -4053,6 +4225,7 @@ std::string EndStdStreamCapture() { void CodeComplete(std::vector& Results, const char* code, unsigned complete_line /* = 1U */, unsigned complete_column /* = 1U */) { + LOCK(getInterpInfo()); compat::codeComplete(Results, getInterp(), code, complete_line, complete_column); } From 8e0c18f7745d4f2836d3bc0a1575af0ed3285043 Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Tue, 12 Aug 2025 10:18:58 +0200 Subject: [PATCH 2/3] use `std::recursive_mutex` instead of recreating it --- lib/CppInterOp/CppInterOp.cpp | 67 ++--------------------------------- 1 file changed, 3 insertions(+), 64 deletions(-) diff --git a/lib/CppInterOp/CppInterOp.cpp b/lib/CppInterOp/CppInterOp.cpp index 2492630df..3aed81306 100755 --- a/lib/CppInterOp/CppInterOp.cpp +++ b/lib/CppInterOp/CppInterOp.cpp @@ -91,74 +91,13 @@ using namespace llvm; using namespace std; #define LOCK(InterpInfo) \ - LockRAII interop_lock = \ - LockRAII((InterpInfo).InterpreterLock, (InterpInfo).ThreadIdLock, \ - (InterpInfo).CurrentThreadId) - -namespace { -struct LockRAII { - LockRAII(std::mutex& resource_lock, std::mutex& thread_id_lock, - std::optional& current_thread_id) - : ResourceLock(resource_lock), ThreadIdLock(thread_id_lock), - CurrentThreadId(current_thread_id) { - ThreadIdLock.lock(); - if (!ResourceLock.try_lock()) { - if (CurrentThreadId) { - if ((*CurrentThreadId) != std::this_thread::get_id()) { - ThreadIdLock.unlock(); - ResourceLock.lock(); // blocking - bool res = ThreadIdLock.try_lock(); // should be free - assert(res && "Internal Error: Interpreter lock not held, but " - "ThreadId lock held.\nPlease report this as a bug.\n"); - ThreadIdLock.unlock(); - return; - } - owned = false; - ThreadIdLock.unlock(); - return; - } - assert(false && - "Internal Error: A thread holds lock for the current " - "interpreter, but no information available on which thread " - "holds the lock.\nPlease report this as a bug.\n"); - } - assert(!CurrentThreadId && - "Internal Error: Lock released but thread id not cleared.\nPlease " - "report this as a bug.\n"); - CurrentThreadId = std::this_thread::get_id(); - ThreadIdLock.unlock(); - } - - ~LockRAII() { - assert(!ResourceLock.try_lock() && - "Internal Error: Resource lock not held"); - if (!owned) - return; - ThreadIdLock.lock(); - CurrentThreadId = std::nullopt; - ThreadIdLock.unlock(); - ResourceLock.unlock(); - } - - LockRAII(LockRAII&&) = delete; - LockRAII(const LockRAII&) = delete; - LockRAII& operator=(const LockRAII&) = delete; - LockRAII& operator=(LockRAII&&) = delete; - -private: - bool owned = true; - std::mutex& ResourceLock; - std::mutex& ThreadIdLock; - std::optional& CurrentThreadId; -}; -} // namespace + std::lock_guard interop_lock( \ + (InterpInfo).InterpreterLock) struct InterpreterInfo { compat::Interpreter* Interpreter = nullptr; bool isOwned = true; - std::mutex InterpreterLock; - std::mutex ThreadIdLock; - std::optional CurrentThreadId = std::nullopt; + std::recursive_mutex InterpreterLock; InterpreterInfo(compat::Interpreter* I, bool Owned) : Interpreter(I), isOwned(Owned) {} From 2c2b317200e1742bdbaf0219553c3a69de759de3 Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Wed, 13 Aug 2025 13:27:37 +0200 Subject: [PATCH 3/3] using hashmap to identify Sema & AST for individual Decl We need to identify which interpreter a Decl belongs to, when using multiple interpreter. We do it by checking which `clang::ASTContext` the `clang::Decl` belongs We maintain a map: `clang::ASTContext -> Cpp::InterpreterInfo`. Using this map, be identify the correct interpreter. There are 2 usecases for this: 1. We can now lock the correct interpreter making it thread safe. 2. User of `libCppInterOp` need not set the correct active interpreter using `Cpp::ActivateInterpreter`, this information can be retrived using the map. --- lib/CppInterOp/CppInterOp.cpp | 334 ++++++++++++++--------- unittests/CppInterOp/InterpreterTest.cpp | 19 ++ 2 files changed, 223 insertions(+), 130 deletions(-) diff --git a/lib/CppInterOp/CppInterOp.cpp b/lib/CppInterOp/CppInterOp.cpp index 3aed81306..666103cd9 100755 --- a/lib/CppInterOp/CppInterOp.cpp +++ b/lib/CppInterOp/CppInterOp.cpp @@ -134,24 +134,60 @@ struct InterpreterInfo { }; // std::deque avoids relocations and calling the dtor of InterpreterInfo. -static llvm::ManagedStatic> sInterpreters; +static llvm::ManagedStatic>> + sInterpreters; +static llvm::ManagedStatic< + std::unordered_map>> + sInterpreterASTMap; static std::mutex InterpreterStackLock; static InterpreterInfo& getInterpInfo() { std::unique_lock Lock(InterpreterStackLock); assert(!sInterpreters->empty() && "Interpreter instance must be set before calling this!"); - return sInterpreters->back(); + return *sInterpreters->back(); +} +static InterpreterInfo& getInterpInfo(const void* D) { + if (!D) + return getInterpInfo(); + for (auto& item : *sInterpreterASTMap) { + if (item.first->getAllocator().identifyObject(D)) + return *item.second.lock(); + } + llvm_unreachable( + "This pointer does not belong to any interpreter instance.\n"); } static compat::Interpreter& getInterp() { std::unique_lock Lock(InterpreterStackLock); assert(!sInterpreters->empty() && "Interpreter instance must be set before calling this!"); - return *sInterpreters->back().Interpreter; + return *sInterpreters->back()->Interpreter; +} +static compat::Interpreter& getInterp(const void* D) { + if (!D) + return getInterp(); + for (auto& item : *sInterpreterASTMap) { + if (item.first->getAllocator().identifyObject(D)) + return *item.second.lock()->Interpreter; + } + llvm_unreachable( + "This pointer does not belong to any interpreter instance.\n"); } + static clang::Sema& getSema() { return getInterp().getCI()->getSema(); } +static clang::Sema& getSema(const void* D) { + if (!D) + return getSema(); + return getInterpInfo(D).Interpreter->getSema(); +} + static clang::ASTContext& getASTContext() { return getSema().getASTContext(); } +static clang::ASTContext& getASTContext(const void* D) { + if (!D) + return getASTContext(); + return getSema(D).getASTContext(); +} #define DEBUG_TYPE "jitcall" bool JitCall::AreArgumentsValid(void* result, ArgList args, void* self, @@ -244,28 +280,30 @@ std::string Demangle(const std::string& mangled_name) { } void EnableDebugOutput(bool value /* =true*/) { + // TODO: This should be locked under a global LLVM lock LOCK(getInterpInfo()); llvm::DebugFlag = value; } bool IsDebugOutputEnabled() { + // TODO: This should be locked under a global LLVM lock LOCK(getInterpInfo()); return llvm::DebugFlag; } static void InstantiateFunctionDefinition(Decl* D) { - LOCK(getInterpInfo()); - compat::SynthesizingCodeRAII RAII(&getInterp()); if (auto* FD = llvm::dyn_cast_or_null(D)) { - getSema().InstantiateFunctionDefinition(SourceLocation(), FD, - /*Recursive=*/true, - /*DefinitionRequired=*/true); + LOCK(getInterpInfo(FD)); + compat::SynthesizingCodeRAII RAII(&getInterp(FD)); + getSema(FD).InstantiateFunctionDefinition(SourceLocation(), FD, + /*Recursive=*/true, + /*DefinitionRequired=*/true); } } bool IsAggregate(TCppScope_t scope) { - LOCK(getInterpInfo()); Decl* D = static_cast(scope); + LOCK(getInterpInfo(D)); // Aggregates are only arrays or tag decls. if (ValueDecl* ValD = dyn_cast(D)) @@ -300,16 +338,16 @@ bool IsFunctionPointerType(TCppType_t type) { } bool IsClassPolymorphic(TCppScope_t klass) { - LOCK(getInterpInfo()); Decl* D = static_cast(klass); - if (auto* CXXRD = llvm::dyn_cast(D)) + if (auto* CXXRD = llvm::dyn_cast(D)) { + LOCK(getInterpInfo(CXXRD)); if (auto* CXXRDD = CXXRD->getDefinition()) return CXXRDD->isPolymorphic(); + } return false; } static SourceLocation GetValidSLoc(Sema& semaRef) { - LOCK(getInterpInfo()); auto& SM = semaRef.getSourceManager(); return SM.getLocForStartOfFile(SM.getMainFileID()); } @@ -321,13 +359,13 @@ bool IsComplete(TCppScope_t scope) { Decl* D = static_cast(scope); - LOCK(getInterpInfo()); + LOCK(getInterpInfo(D)); if (isa(D)) { QualType QT = QualType::getFromOpaquePtr(GetTypeFromScope(scope)); - clang::Sema& S = getSema(); + clang::Sema& S = getSema(D); SourceLocation fakeLoc = GetValidSLoc(S); #ifdef CPPINTEROP_USE_CLING - cling::Interpreter::PushTransactionRAII RAII(&getInterp()); + cling::Interpreter::PushTransactionRAII RAII(&getInterp(D)); #endif // CPPINTEROP_USE_CLING return S.isCompleteType(fakeLoc, QT); } @@ -346,8 +384,8 @@ size_t SizeOf(TCppScope_t scope) { if (!IsComplete(scope)) return 0; - LOCK(getInterpInfo()); if (auto* RD = dyn_cast(static_cast(scope))) { + LOCK(getInterpInfo(RD)); ASTContext& Context = RD->getASTContext(); const ASTRecordLayout& Layout = Context.getASTRecordLayout(RD); return Layout.getSize().getQuantity(); @@ -382,7 +420,7 @@ bool IsTypedefed(TCppScope_t handle) { bool IsAbstract(TCppType_t klass) { auto* D = (clang::Decl*)klass; if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo()); + LOCK(getInterpInfo(CXXRD)); return CXXRD->isAbstract(); } @@ -405,7 +443,6 @@ bool IsEnumType(TCppType_t type) { } static bool isSmartPointer(const RecordType* RT) { - LOCK(getInterpInfo()); auto IsUseCountPresent = [](const RecordDecl* Record) { ASTContext& C = Record->getASTContext(); return !Record->lookup(&C.Idents.get("use_count")).empty(); @@ -419,6 +456,7 @@ static bool isSmartPointer(const RecordType* RT) { }; const RecordDecl* Record = RT->getDecl(); + LOCK(getInterpInfo(Record)); if (IsUseCountPresent(Record)) return true; @@ -485,8 +523,8 @@ TCppType_t GetIntegerTypeFromEnumType(TCppType_t enum_type) { std::vector GetEnumConstants(TCppScope_t handle) { auto* D = (clang::Decl*)handle; - LOCK(getInterpInfo()); if (auto* ED = llvm::dyn_cast_or_null(D)) { + LOCK(getInterpInfo(ED)); std::vector enum_constants; for (auto* ECD : ED->enumerators()) { enum_constants.push_back((TCppScope_t)ECD); @@ -519,13 +557,13 @@ TCppIndex_t GetEnumConstantValue(TCppScope_t handle) { } size_t GetSizeOfType(TCppType_t type) { - LOCK(getInterpInfo()); QualType QT = QualType::getFromOpaquePtr(type); if (const TagType* TT = QT->getAs()) return SizeOf(TT->getDecl()); // FIXME: Can we get the size of a non-tag type? - auto TI = getSema().getASTContext().getTypeInfo(QT); + auto TI = getSema().getASTContext().getTypeInfo( + QT); // FIXME: is this the corrent sema? size_t TypeSize = TI.Width; return TypeSize / 8; } @@ -550,8 +588,8 @@ std::string GetName(TCppType_t klass) { } std::string GetCompleteName(TCppType_t klass) { - auto& C = getSema().getASTContext(); auto* D = (Decl*)klass; + auto& C = getSema(D).getASTContext(); PrintingPolicy Policy = C.getPrintingPolicy(); Policy.SuppressUnwrittenScope = true; @@ -601,8 +639,8 @@ std::string GetQualifiedName(TCppType_t klass) { // FIXME: Figure out how to merge with GetCompleteName. std::string GetQualifiedCompleteName(TCppType_t klass) { - auto& C = getSema().getASTContext(); auto* D = (Decl*)klass; + auto& C = getSema(D).getASTContext(); if (auto* ND = llvm::dyn_cast_or_null(D)) { if (auto* TD = llvm::dyn_cast(ND)) { @@ -628,10 +666,10 @@ std::string GetQualifiedCompleteName(TCppType_t klass) { } std::vector GetUsingNamespaces(TCppScope_t scope) { - LOCK(getInterpInfo()); auto* D = (clang::Decl*)scope; if (auto* DC = llvm::dyn_cast_or_null(D)) { + LOCK(getInterpInfo(D)); std::vector namespaces; for (auto UD : DC->using_directives()) { namespaces.push_back((TCppScope_t)UD->getNominatedNamespace()); @@ -718,15 +756,15 @@ TCppScope_t GetScopeFromCompleteName(const std::string& name) { TCppScope_t GetNamed(const std::string& name, TCppScope_t parent /*= nullptr*/) { - LOCK(getInterpInfo()); clang::DeclContext* Within = 0; + auto* D = (clang::Decl*)parent; + LOCK(getInterpInfo(D)); if (parent) { - auto* D = (clang::Decl*)parent; D = GetUnderlyingScope(D); Within = llvm::dyn_cast(D); } - auto* ND = Cpp_utils::Lookup::Named(&getSema(), name, Within); + auto* ND = Cpp_utils::Lookup::Named(&getSema(D), name, Within); if (ND && ND != (clang::NamedDecl*)-1) { return (TCppScope_t)(ND->getCanonicalDecl()); } @@ -754,13 +792,15 @@ TCppScope_t GetParentScope(TCppScope_t scope) { } TCppIndex_t GetNumBases(TCppScope_t klass) { - LOCK(getInterpInfo()); auto* D = (Decl*)klass; - if (auto* CTSD = llvm::dyn_cast_or_null(D)) + if (auto* CTSD = llvm::dyn_cast_or_null(D)) { + LOCK(getInterpInfo(CTSD)); if (!CTSD->hasDefinition()) - compat::InstantiateClassTemplateSpecialization(getInterp(), CTSD); + compat::InstantiateClassTemplateSpecialization(getInterp(CTSD), CTSD); + } if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { + LOCK(getInterpInfo(CXXRD)); if (CXXRD->hasDefinition()) return CXXRD->getNumBases(); } @@ -769,10 +809,13 @@ TCppIndex_t GetNumBases(TCppScope_t klass) { } TCppScope_t GetBaseClass(TCppScope_t klass, TCppIndex_t ibase) { - LOCK(getInterpInfo()); auto* D = (Decl*)klass; auto* CXXRD = llvm::dyn_cast_or_null(D); - if (!CXXRD || CXXRD->getNumBases() <= ibase) + if (!CXXRD) + return 0; + + LOCK(getInterpInfo(CXXRD)); + if (CXXRD->getNumBases() <= ibase) return 0; auto type = (CXXRD->bases_begin() + ibase)->getType(); @@ -809,7 +852,6 @@ bool IsSubclass(TCppScope_t derived, TCppScope_t base) { static unsigned ComputeBaseOffset(const ASTContext& Context, const CXXRecordDecl* DerivedRD, const CXXBasePath& Path) { - LOCK(getInterpInfo()); CharUnits NonVirtualOffset = CharUnits::Zero(); unsigned NonVirtualStart = 0; @@ -864,14 +906,14 @@ int64_t GetBaseClassOffset(TCppScope_t derived, TCppScope_t base) { CXXRecordDecl* DCXXRD = cast(DD); CXXRecordDecl* BCXXRD = cast(BD); - LOCK(getInterpInfo()); + LOCK(getInterpInfo(DD)); CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, /*DetectVirtual=*/false); DCXXRD->isDerivedFrom(BCXXRD, Paths); // FIXME: We might want to cache these requests as they seem expensive. - return ComputeBaseOffset(getSema().getASTContext(), DCXXRD, Paths.front()); + return ComputeBaseOffset(getSema(DD).getASTContext(), DCXXRD, Paths.front()); } template @@ -880,8 +922,8 @@ static void GetClassDecls(TCppScope_t klass, if (!klass) return; - LOCK(getInterpInfo()); auto* D = (clang::Decl*)klass; + LOCK(getInterpInfo(D)); if (auto* TD = dyn_cast(D)) D = GetScopeFromType(TD->getUnderlyingType()); @@ -891,11 +933,11 @@ static void GetClassDecls(TCppScope_t klass, auto* CXXRD = dyn_cast(D); #ifdef CPPINTEROP_USE_CLING - cling::Interpreter::PushTransactionRAII RAII(&getInterp()); + cling::Interpreter::PushTransactionRAII RAII(&getInterp(CXXRD)); #endif // CPPINTEROP_USE_CLING if (CXXRD->hasDefinition()) CXXRD = CXXRD->getDefinition(); - getSema().ForceDeclarationOfImplicitMembers(CXXRD); + getSema(CXXRD).ForceDeclarationOfImplicitMembers(CXXRD); for (Decl* DI : CXXRD->decls()) { if (auto* MD = dyn_cast(DI)) methods.push_back(MD); @@ -921,7 +963,7 @@ static void GetClassDecls(TCppScope_t klass, // Result is appended to the decls, i.e. CXXRD, iterator // non-shadowed decl will be push_back later // methods.push_back(Result); - getSema().findInheritingConstructor(SourceLocation(), CXXCD, CUSD); + getSema(CXXRD).findInheritingConstructor(SourceLocation(), CXXCD, CUSD); } } } @@ -938,9 +980,10 @@ void GetFunctionTemplatedDecls(TCppScope_t klass, bool HasDefaultConstructor(TCppScope_t scope) { auto* D = (clang::Decl*)scope; - LOCK(getInterpInfo()); - if (auto* CXXRD = llvm::dyn_cast_or_null(D)) + if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { + LOCK(getInterpInfo(CXXRD)); return CXXRD->hasDefaultConstructor(); + } return false; } @@ -950,21 +993,22 @@ TCppFunction_t GetDefaultConstructor(compat::Interpreter& interp, if (!HasDefaultConstructor(scope)) return nullptr; - LOCK(getInterpInfo()); auto* CXXRD = (clang::CXXRecordDecl*)scope; + LOCK(getInterpInfo(CXXRD)); return interp.getCI()->getSema().LookupDefaultConstructor(CXXRD); } TCppFunction_t GetDefaultConstructor(TCppScope_t scope) { - return GetDefaultConstructor(getInterp(), scope); + auto* CXXRD = (clang::CXXRecordDecl*)scope; + return GetDefaultConstructor(getInterp(CXXRD), scope); } TCppFunction_t GetDestructor(TCppScope_t scope) { auto* D = (clang::Decl*)scope; if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo()); - getSema().ForceDeclarationOfImplicitMembers(CXXRD); + LOCK(getInterpInfo(CXXRD)); + getSema(CXXRD).ForceDeclarationOfImplicitMembers(CXXRD); return CXXRD->getDestructor(); } @@ -983,14 +1027,14 @@ std::vector GetFunctionsUsingName(TCppScope_t scope, if (!scope || name.empty()) return {}; - LOCK(getInterpInfo()); + LOCK(getInterpInfo(D)); D = GetUnderlyingScope(D); std::vector funcs; llvm::StringRef Name(name); - auto& S = getSema(); - DeclarationName DName = &getASTContext().Idents.get(name); + auto& S = getSema(D); + DeclarationName DName = &S.getASTContext().Idents.get(name); clang::LookupResult R(S, DName, SourceLocation(), Sema::LookupOrdinaryName, For_Visible_Redeclaration); @@ -1009,10 +1053,10 @@ std::vector GetFunctionsUsingName(TCppScope_t scope, } TCppType_t GetFunctionReturnType(TCppFunction_t func) { - LOCK(getInterpInfo()); auto* D = (clang::Decl*)func; if (auto* FD = llvm::dyn_cast_or_null(D)) { + LOCK(getInterpInfo(FD)); QualType Type = FD->getReturnType(); if (Type->isUndeducedAutoType()) { bool needInstantiation = false; @@ -1031,8 +1075,10 @@ TCppType_t GetFunctionReturnType(TCppFunction_t func) { return Type.getAsOpaquePtr(); } - if (auto* FD = llvm::dyn_cast_or_null(D)) + if (auto* FD = llvm::dyn_cast_or_null(D)) { + LOCK(getInterpInfo(FD)); return (FD->getTemplatedDecl())->getReturnType().getAsOpaquePtr(); + } return 0; } @@ -1060,10 +1106,10 @@ TCppIndex_t GetFunctionRequiredArgs(TCppConstFunction_t func) { } TCppType_t GetFunctionArgType(TCppFunction_t func, TCppIndex_t iarg) { - LOCK(getInterpInfo()); auto* D = (clang::Decl*)func; if (auto* FD = llvm::dyn_cast_or_null(D)) { + LOCK(getInterpInfo(FD)); if (iarg < FD->getNumParams()) { auto* PVD = FD->getParamDecl(iarg); return PVD->getOriginalType().getAsOpaquePtr(); @@ -1089,7 +1135,7 @@ std::string GetFunctionSignature(TCppFunction_t func) { std::string Signature; raw_string_ostream SS(Signature); - PrintingPolicy Policy = getASTContext().getPrintingPolicy(); + PrintingPolicy Policy = getASTContext(D).getPrintingPolicy(); // Skip printing the body Policy.TerseOutput = true; Policy.FullyQualifiedName = true; @@ -1136,14 +1182,14 @@ bool IsTemplatedFunction(TCppFunction_t func) { // the template function exists and >1 means overloads bool ExistsFunctionTemplate(const std::string& name, TCppScope_t parent) { DeclContext* Within = 0; + auto* D = (Decl*)parent; if (parent) { - auto* D = (Decl*)parent; Within = llvm::dyn_cast(D); } - LOCK(getInterpInfo()); + LOCK(getInterpInfo(D)); - auto* ND = Cpp_utils::Lookup::Named(&getSema(), name, Within); + auto* ND = Cpp_utils::Lookup::Named(&getSema(D), name, Within); if ((intptr_t)ND == (intptr_t)0) return false; @@ -1162,10 +1208,10 @@ void LookupConstructors(const std::string& name, TCppScope_t parent, auto* D = (Decl*)parent; if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo()); + LOCK(getInterpInfo(CXXRD)); - getSema().ForceDeclarationOfImplicitMembers(CXXRD); - DeclContextLookupResult Result = getSema().LookupConstructors(CXXRD); + getSema(CXXRD).ForceDeclarationOfImplicitMembers(CXXRD); + DeclContextLookupResult Result = getSema(CXXRD).LookupConstructors(CXXRD); // Obtaining all constructors when we intend to lookup a method under a // scope can lead to crashes. We avoid that by accumulating constructors // only if the Decl matches the lookup name. @@ -1181,14 +1227,14 @@ bool GetClassTemplatedMethods(const std::string& name, TCppScope_t parent, if (!D && name.empty()) return false; - LOCK(getInterpInfo()); + LOCK(getInterpInfo(D)); // Accumulate constructors LookupConstructors(name, parent, funcs); - auto& S = getSema(); + auto& S = getSema(D); D = GetUnderlyingScope(D); llvm::StringRef Name(name); - DeclarationName DName = &getASTContext().Idents.get(name); + DeclarationName DName = &S.getASTContext().Idents.get(name); clang::LookupResult R(S, DName, SourceLocation(), Sema::LookupOrdinaryName, For_Visible_Redeclaration); auto* DC = clang::Decl::castToDeclContext(D); @@ -1224,13 +1270,16 @@ TCppFunction_t BestOverloadFunctionMatch(const std::vector& candidates, const std::vector& explicit_types, const std::vector& arg_types) { - LOCK(getInterpInfo()); + if (candidates.empty()) + return nullptr; + InterpreterInfo& II = getInterpInfo((clang::Decl*)candidates[0]); + LOCK(II); - auto& S = getSema(); + auto& S = II.Interpreter->getSema(); auto& C = S.getASTContext(); #ifdef CPPINTEROP_USE_CLING - cling::Interpreter::PushTransactionRAII RAII(&getInterp()); + cling::Interpreter::PushTransactionRAII RAII(II.Interpreter); #endif // The overload resolution interfaces in Sema require a list of expressions. @@ -1347,10 +1396,9 @@ bool IsDestructor(TCppConstFunction_t method) { } bool IsStaticMethod(TCppConstFunction_t method) { - LOCK(getInterpInfo()); - const auto* D = static_cast(method); if (auto* CXXMD = llvm::dyn_cast_or_null(D)) { + LOCK(getInterpInfo(D)); return CXXMD->isStatic(); } @@ -1370,7 +1418,7 @@ TCppFuncAddr_t GetFunctionAddress(const char* mangled_name) { static TCppFuncAddr_t GetFunctionAddress(const FunctionDecl* FD) { const auto get_mangled_name = [](const FunctionDecl* FD) { - auto MangleCtxt = getASTContext().createMangleContext(); + auto MangleCtxt = getASTContext(FD).createMangleContext(); if (!MangleCtxt->shouldMangleDeclName(FD)) { return FD->getNameInfo().getName().getAsString(); @@ -1402,9 +1450,9 @@ TCppFuncAddr_t GetFunctionAddress(TCppFunction_t method) { } bool IsVirtualMethod(TCppFunction_t method) { - LOCK(getInterpInfo()); auto* D = (Decl*)method; if (auto* CXXMD = llvm::dyn_cast_or_null(D)) { + LOCK(getInterpInfo(CXXMD)); return CXXMD->isVirtual(); } @@ -1415,9 +1463,9 @@ void GetDatamembers(TCppScope_t scope, std::vector& datamembers) { auto* D = (Decl*)scope; if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { - LOCK(getInterpInfo()); + LOCK(getInterpInfo(CXXRD)); - getSema().ForceDeclarationOfImplicitMembers(CXXRD); + getSema(CXXRD).ForceDeclarationOfImplicitMembers(CXXRD); if (CXXRD->hasDefinition()) CXXRD = CXXRD->getDefinition(); @@ -1462,10 +1510,13 @@ void GetStaticDatamembers(TCppScope_t scope, void GetEnumConstantDatamembers(TCppScope_t scope, std::vector& datamembers, bool include_enum_class) { - LOCK(getInterpInfo()); - std::vector EDs; GetClassDecls(scope, EDs); + if (EDs.empty()) + return; + + LOCK(getInterpInfo((clang::Decl*)EDs[0])); + for (TCppScope_t i : EDs) { auto* ED = static_cast(i); @@ -1480,13 +1531,13 @@ void GetEnumConstantDatamembers(TCppScope_t scope, TCppScope_t LookupDatamember(const std::string& name, TCppScope_t parent) { clang::DeclContext* Within = 0; + auto* D = (clang::Decl*)parent; if (parent) { - auto* D = (clang::Decl*)parent; Within = llvm::dyn_cast(D); } - LOCK(getInterpInfo()); - auto* ND = Cpp_utils::Lookup::Named(&getSema(), name, Within); + LOCK(getInterpInfo(D)); + auto* ND = Cpp_utils::Lookup::Named(&getSema(D), name, Within); if (ND && ND != (clang::NamedDecl*)-1) { if (llvm::isa_and_nonnull(ND)) { return (TCppScope_t)ND; @@ -1528,11 +1579,6 @@ TCppType_t GetVariableType(TCppScope_t var) { intptr_t GetVariableOffset(compat::Interpreter& I, Decl* D, CXXRecordDecl* BaseCXXRD) { - if (!D) - return 0; - - LOCK(getInterpInfo()); - auto& C = I.getSema().getASTContext(); if (auto* FD = llvm::dyn_cast(D)) { @@ -1607,9 +1653,9 @@ intptr_t GetVariableOffset(compat::Interpreter& I, Decl* D, if (!address) { if (!VD->hasInit()) { #ifdef CPPINTEROP_USE_CLING - cling::Interpreter::PushTransactionRAII RAII(&getInterp()); + cling::Interpreter::PushTransactionRAII RAII(&getInterp(VD)); #endif // CPPINTEROP_USE_CLING - getSema().InstantiateVariableDefinition(SourceLocation(), VD); + getSema(VD).InstantiateVariableDefinition(SourceLocation(), VD); VD = VD->getDefinition(); } if (VD->hasInit() && @@ -1671,8 +1717,11 @@ intptr_t GetVariableOffset(compat::Interpreter& I, Decl* D, intptr_t GetVariableOffset(TCppScope_t var, TCppScope_t parent) { auto* D = static_cast(var); + if (!D) + return 0; + LOCK(getInterpInfo(D)); auto* RD = llvm::dyn_cast_or_null(static_cast(parent)); - return GetVariableOffset(getInterp(), D, RD); + return GetVariableOffset(getInterp(D), D, RD); } // Check if the Access Specifier of the variable matches the provided value. @@ -1719,7 +1768,7 @@ bool IsPODType(TCppType_t type) { if (QT.isNull()) return false; - return QT.isPODType(getASTContext()); + return QT.isPODType(getASTContext()); // FIXME: which ASTContext? } bool IsPointerType(TCppType_t type) { @@ -1751,13 +1800,17 @@ bool IsRValueReferenceType(TCppType_t type) { TCppType_t GetPointerType(TCppType_t type) { QualType QT = QualType::getFromOpaquePtr(type); - return getASTContext().getPointerType(QT).getAsOpaquePtr(); + return getASTContext() + .getPointerType(QT) + .getAsOpaquePtr(); // FIXME: which ASTContext? } TCppType_t GetReferencedType(TCppType_t type, bool rvalue) { QualType QT = QualType::getFromOpaquePtr(type); if (rvalue) - return getASTContext().getRValueReferenceType(QT).getAsOpaquePtr(); + return getASTContext() + .getRValueReferenceType(QT) + .getAsOpaquePtr(); // FIXME: which ASTContext? return getASTContext().getLValueReferenceType(QT).getAsOpaquePtr(); } @@ -1941,8 +1994,10 @@ TCppType_t GetType(const std::string& name) { TCppType_t GetComplexType(TCppType_t type) { QualType QT = QualType::getFromOpaquePtr(type); - LOCK(getInterpInfo()); - return getASTContext().getComplexType(QT).getAsOpaquePtr(); + LOCK(getInterpInfo()); // FIXME: Which interpreter to lock? + return getASTContext() + .getComplexType(QT) + .getAsOpaquePtr(); // FIXME: which ASTContext? } TCppType_t GetTypeFromScope(TCppScope_t klass) { @@ -1950,7 +2005,7 @@ TCppType_t GetTypeFromScope(TCppScope_t klass) { return 0; auto* D = (Decl*)klass; - ASTContext& C = getASTContext(); + ASTContext& C = getASTContext(D); if (ValueDecl* VD = dyn_cast(D)) return VD->getType().getAsOpaquePtr(); @@ -1981,7 +2036,6 @@ void* compile_wrapper(compat::Interpreter& I, const std::string& wrapper_name, const std::string& wrapper, bool withAccessControl = true) { LLVM_DEBUG(dbgs() << "Compiling '" << wrapper_name << "'\n"); - LOCK(getInterpInfo()); return I.compileFunction(wrapper_name, wrapper, false /*ifUnique*/, withAccessControl); } @@ -1990,10 +2044,8 @@ void get_type_as_string(QualType QT, std::string& type_name, ASTContext& C, PrintingPolicy Policy) { // TODO: Implement cling desugaring from utils::AST // cling::utils::Transform::GetPartiallyDesugaredType() - if (!QT->isTypedefNameType() || QT->isBuiltinType()) { - LOCK(getInterpInfo()); + if (!QT->isTypedefNameType() || QT->isBuiltinType()) QT = QT.getDesugaredType(C); - } Policy.SuppressElaboration = true; Policy.SuppressTagKeyword = !QT->isEnumeralType(); Policy.FullyQualifiedName = true; @@ -2342,14 +2394,12 @@ void make_narg_call(const FunctionDecl* FD, const std::string& return_type, // a simple way of determining whether a viable copy constructor exists, // so check for the most common case: the trivial one, but not uniquely // available, while there is a move constructor. - { - LOCK(getInterpInfo()); - // include utility header if not already included for std::move - DeclarationName DMove = &getASTContext().Idents.get("move"); - auto result = getSema().getStdNamespace()->lookup(DMove); - if (result.empty()) - Cpp::Declare("#include "); - } + + // include utility header if not already included for std::move + DeclarationName DMove = &getASTContext(FD).Idents.get("move"); + auto result = getSema(FD).getStdNamespace()->lookup(DMove); + if (result.empty()) + Cpp::Declare("#include "); // move construction as needed for classes (note that this is implicit) callbuf << "std::move(*(" << type_name.c_str() << "*)args[" << i << "])"; @@ -2560,7 +2610,7 @@ void make_narg_call_with_return(compat::Interpreter& I, const FunctionDecl* FD, int get_wrapper_code(compat::Interpreter& I, const FunctionDecl* FD, std::string& wrapper_name, std::string& wrapper) { assert(FD && "generate_wrapper called without a function decl!"); - LOCK(getInterpInfo()); + LOCK(getInterpInfo(FD)); ASTContext& Context = FD->getASTContext(); // @@ -2982,7 +3032,7 @@ JitCall::GenericCall make_wrapper(compat::Interpreter& I, const FunctionDecl* FD) { static std::map gWrapperStore; - LOCK(getInterpInfo()); + LOCK(getInterpInfo(FD)); auto R = gWrapperStore.find(FD); if (R != gWrapperStore.end()) @@ -3076,7 +3126,7 @@ static JitCall::DestructorCall make_dtor_wrapper(compat::Interpreter& interp, static map gDtorWrapperStore; - LOCK(getInterpInfo()); + LOCK(getInterpInfo(D)); auto I = gDtorWrapperStore.find(D); if (I != gDtorWrapperStore.end()) @@ -3226,7 +3276,8 @@ CPPINTEROP_API JitCall MakeFunctionCallable(TInterp_t I, } CPPINTEROP_API JitCall MakeFunctionCallable(TCppConstFunction_t func) { - return MakeFunctionCallable(&getInterp(), func); + auto* D = (clang::Decl*)func; + return MakeFunctionCallable(&getInterp(D), func); } namespace { @@ -3333,7 +3384,11 @@ TInterp_t CreateInterpreter(const std::vector& Args /*={}*/, } // namespace __internal_CppInterOp )"); - sInterpreters->emplace_back(I, /*Owned=*/true); + sInterpreters->emplace_back( + std::make_shared(I, /*Owned=*/true)); + sInterpreterASTMap->insert( + {&sInterpreters->back()->Interpreter->getSema().getASTContext(), + sInterpreters->back()}); return I; } @@ -3342,17 +3397,29 @@ bool DeleteInterpreter(TInterp_t I /*=nullptr*/) { std::unique_lock Lock(InterpreterStackLock); if (!I) { + auto foundAST = + std::find_if(sInterpreterASTMap->begin(), sInterpreterASTMap->end(), + [](const auto& Item) { + return Item.second.lock() == sInterpreters->back(); + }); + assert(foundAST != sInterpreterASTMap->end()); + sInterpreterASTMap->erase(foundAST); sInterpreters->pop_back(); return true; } auto found = std::find_if(sInterpreters->begin(), sInterpreters->end(), - [&I](const auto& Info) { return Info.Interpreter == I; }); + [&I](const auto& Info) { return Info->Interpreter == I; }); if (found == sInterpreters->end()) return false; // failure - LOCK(*found); + LOCK(**found); + auto foundAST = std::find_if( + sInterpreterASTMap->begin(), sInterpreterASTMap->end(), + [&found](const auto& Item) { return Item.second.lock() == *found; }); + assert(foundAST != sInterpreterASTMap->end()); + sInterpreterASTMap->erase(foundAST); sInterpreters->erase(found); return true; } @@ -3365,7 +3432,7 @@ bool ActivateInterpreter(TInterp_t I) { auto found = std::find_if(sInterpreters->begin(), sInterpreters->end(), - [&I](const auto& Info) { return Info.Interpreter == I; }); + [&I](const auto& Info) { return Info->Interpreter == I; }); if (found == sInterpreters->end()) return false; @@ -3379,14 +3446,18 @@ TInterp_t GetInterpreter() { std::unique_lock Lock(InterpreterStackLock); if (sInterpreters->empty()) return nullptr; - return sInterpreters->back().Interpreter; + return sInterpreters->back()->Interpreter; } void UseExternalInterpreter(TInterp_t I) { std::unique_lock Lock(InterpreterStackLock); assert(sInterpreters->empty() && "sInterpreter already in use!"); - sInterpreters->emplace_back(static_cast(I), - /*isOwned=*/false); + sInterpreters->emplace_back( + std::make_shared(static_cast(I), + /*isOwned=*/false)); + sInterpreterASTMap->insert( + {&sInterpreters->back()->Interpreter->getSema().getASTContext(), + sInterpreters->back()}); } void AddSearchPath(const char* dir, bool isUser, bool prepend) { @@ -3635,7 +3706,8 @@ bool InsertOrReplaceJitSymbol(const char* linker_mangled_name, } std::string ObjToString(const char* type, void* obj) { - LOCK(getInterpInfo()); + LOCK(getInterpInfo()); // FIXME: not enough information to lock the corrent + // interpreter return getInterp().toString(type, obj); } @@ -3693,7 +3765,6 @@ Decl* InstantiateTemplate(TemplateDecl* TemplateD, Decl* InstantiateTemplate(TemplateDecl* TemplateD, ArrayRef TemplateArgs, Sema& S, bool instantiate_body) { - LOCK(getInterpInfo()); // Create a list of template arguments. TemplateArgumentListInfo TLI{}; for (auto TA : TemplateArgs) @@ -3738,15 +3809,16 @@ TCppScope_t InstantiateTemplate(TCppScope_t tmpl, const TemplateArgInfo* template_args, size_t template_args_size, bool instantiate_body) { - LOCK(getInterpInfo()); - return InstantiateTemplate(getInterp(), tmpl, template_args, + auto* D = static_cast(tmpl); + LOCK(getInterpInfo(D)); + return InstantiateTemplate(getInterp(D), tmpl, template_args, template_args_size, instantiate_body); } void GetClassTemplateInstantiationArgs(TCppScope_t templ_instance, std::vector& args) { - LOCK(getInterpInfo()); auto* CTSD = static_cast(templ_instance); + LOCK(getInterpInfo(CTSD)); for (const auto& TA : CTSD->getTemplateInstantiationArgs().asArray()) { switch (TA.getKind()) { default: @@ -3792,7 +3864,7 @@ void GetAllCppNames(TCppScope_t scope, std::set& names) { clang::DeclContext* DC; clang::DeclContext::decl_iterator decl; - LOCK(getInterpInfo()); + LOCK(getInterpInfo(D)); if (auto* TD = dyn_cast_or_null(D)) { DC = clang::TagDecl::castToDeclContext(TD); decl = DC->decls_begin(); @@ -3822,7 +3894,7 @@ void GetEnums(TCppScope_t scope, std::vector& Result) { auto* DC = llvm::dyn_cast(D); - LOCK(getInterpInfo()); + LOCK(getInterpInfo(D)); llvm::SmallVector DCs; DC->collectAllContexts(DCs); @@ -3839,7 +3911,6 @@ void GetEnums(TCppScope_t scope, std::vector& Result) { // FIXME: On the CPyCppyy side the receiver is of type // vector instead of vector std::vector GetDimensions(TCppType_t type) { - LOCK(getInterpInfo()); QualType Qual = QualType::getFromOpaquePtr(type); if (Qual.isNull()) return {}; @@ -3866,24 +3937,25 @@ std::vector GetDimensions(TCppType_t type) { } bool IsTypeDerivedFrom(TCppType_t derived, TCppType_t base) { - LOCK(getInterpInfo()); - auto& S = getSema(); - auto fakeLoc = GetValidSLoc(S); auto derivedType = clang::QualType::getFromOpaquePtr(derived); auto baseType = clang::QualType::getFromOpaquePtr(base); + auto* CXXRD = baseType->getAsRecordDecl(); + LOCK(getInterpInfo(CXXRD)); + auto& S = getSema(CXXRD); + auto fakeLoc = GetValidSLoc(S); #ifdef CPPINTEROP_USE_CLING - cling::Interpreter::PushTransactionRAII RAII(&getInterp()); + cling::Interpreter::PushTransactionRAII RAII(&getInterp(CXXRD)); #endif return S.IsDerivedFrom(fakeLoc, derivedType, baseType); } std::string GetFunctionArgDefault(TCppFunction_t func, TCppIndex_t param_index) { - LOCK(getInterpInfo()); auto* D = (clang::Decl*)func; clang::ParmVarDecl* PI = nullptr; + LOCK(getInterpInfo(D)); if (auto* FD = llvm::dyn_cast_or_null(D)) PI = FD->getParamDecl(param_index); @@ -3978,8 +4050,8 @@ OperatorArity GetOperatorArity(TCppFunction_t op) { void GetOperator(TCppScope_t scope, Operator op, std::vector& operators, OperatorArity kind) { - LOCK(getInterpInfo()); Decl* D = static_cast(scope); + LOCK(getInterpInfo(D)); if (auto* CXXRD = llvm::dyn_cast_or_null(D)) { auto fn = [&operators, kind, op](const RecordDecl* RD) { ASTContext& C = RD->getASTContext(); @@ -3995,7 +4067,7 @@ void GetOperator(TCppScope_t scope, Operator op, fn(CXXRD); CXXRD->forallBases(fn); } else if (auto* DC = llvm::dyn_cast_or_null(D)) { - ASTContext& C = getSema().getASTContext(); + ASTContext& C = getSema(D).getASTContext(); DeclContextLookupResult Result = DC->lookup(C.DeclarationNames.getCXXOperatorName( (clang::OverloadedOperatorKind)op)); @@ -4044,7 +4116,8 @@ TCppObject_t Construct(compat::Interpreter& interp, TCppScope_t scope, TCppObject_t Construct(TCppScope_t scope, void* arena /*=nullptr*/, TCppIndex_t count /*=1UL*/) { - return Construct(getInterp(), scope, arena, count); + auto* D = (clang::Decl*)scope; + return Construct(getInterp(D), scope, arena, count); } bool Destruct(compat::Interpreter& interp, TCppObject_t This, const Decl* Class, @@ -4060,7 +4133,7 @@ bool Destruct(compat::Interpreter& interp, TCppObject_t This, const Decl* Class, bool Destruct(TCppObject_t This, TCppConstScope_t scope, bool withFree /*=true*/, TCppIndex_t count /*=0UL*/) { const auto* Class = static_cast(scope); - return Destruct(getInterp(), This, Class, withFree, count); + return Destruct(getInterp(Class), This, Class, withFree, count); } class StreamCaptureInfo { @@ -4164,7 +4237,8 @@ std::string EndStdStreamCapture() { void CodeComplete(std::vector& Results, const char* code, unsigned complete_line /* = 1U */, unsigned complete_column /* = 1U */) { - LOCK(getInterpInfo()); + LOCK(getInterpInfo()); // FIXME: Not enough info to lock the corrent + // interpreter compat::codeComplete(Results, getInterp(), code, complete_line, complete_column); } diff --git a/unittests/CppInterOp/InterpreterTest.cpp b/unittests/CppInterOp/InterpreterTest.cpp index e9b82ea1d..7af49e6e9 100644 --- a/unittests/CppInterOp/InterpreterTest.cpp +++ b/unittests/CppInterOp/InterpreterTest.cpp @@ -361,3 +361,22 @@ if (llvm::sys::RunningOnValgrind()) delete ExtInterp; #endif } + +TEST(InterpreterTest, MultipleInterpreter) { + EXPECT_TRUE(Cpp::CreateInterpreter()); + Cpp::Declare(R"( + void f() {} + )"); + auto f = Cpp::GetNamed("f"); + + EXPECT_TRUE(Cpp::CreateInterpreter()); + Cpp::Declare(R"( + void ff() {} + )"); + auto ff = Cpp::GetNamed("ff"); + + auto f_callable = Cpp::MakeFunctionCallable(f); + EXPECT_EQ(f_callable.getKind(), Cpp::JitCall::Kind::kGenericCall); + auto ff_callable = Cpp::MakeFunctionCallable(ff); + EXPECT_EQ(ff_callable.getKind(), Cpp::JitCall::Kind::kGenericCall); +}