From 3d607bc72430e7279b3eea5898ff6ad44e26323f Mon Sep 17 00:00:00 2001 From: Axel Naumann Date: Tue, 30 Mar 2021 16:58:29 +0200 Subject: [PATCH 1/8] [cling] Fix alignment of Transaction allocation: new of a char array might not have the correct alignment to hold a Transaction. Allocate a Transaction itself directly, instead of in-place constructing it in the character array. Each Transaction in the pool is thus constructed through `new Transaction(S)` and destructed through `delete T`, which is nicely symmetrical. The use of `::operator new` and `::operator delete` isn't actually necessary. While I'm at it, improve the assert message's wording. --- .../cling/lib/Interpreter/TransactionPool.h | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/interpreter/cling/lib/Interpreter/TransactionPool.h b/interpreter/cling/lib/Interpreter/TransactionPool.h index 106fda1c36424..1242bac1eaacc 100644 --- a/interpreter/cling/lib/Interpreter/TransactionPool.h +++ b/interpreter/cling/lib/Interpreter/TransactionPool.h @@ -48,10 +48,9 @@ namespace cling { Transaction* takeTransaction(clang::Sema& S) { Transaction *T; - if (kDebugMode || m_Transactions.empty()) { - T = (Transaction*) ::operator new(sizeof(Transaction)); - new(T) Transaction(S); - } else + if (kDebugMode || m_Transactions.empty()) + T = new Transaction(S); + else T = new (m_Transactions.pop_back_val()) Transaction(S); return T; @@ -63,7 +62,7 @@ namespace cling { if (reuse) { assert((T->getState() == Transaction::kCompleted || T->getState() == Transaction::kRolledBack) - && "Transaction must completed!"); + && "Transaction must be completed!"); // Force reuse to off when not in Debug mode if (kDebugMode) reuse = false; @@ -73,15 +72,14 @@ namespace cling { if (T->getParent()) T->getParent()->removeNestedTransaction(T); - T->~Transaction(); - // don't overflow the pool if (reuse && (m_Transactions.size() < kPoolSize)) { T->m_State = Transaction::kNumStates; + T->~Transaction(); m_Transactions.push_back(T); } else - ::operator delete(T); + delete T; } }; From 8349d74a6b3e618a900392f4cc194b32072d1913 Mon Sep 17 00:00:00 2001 From: Axel Naumann Date: Wed, 31 Mar 2021 19:22:54 +0200 Subject: [PATCH 2/8] [cling] Move PushTransactionRAII out of Interpreter: This will allow Transaction remembering its RAII to not depend on Interpreter. --- core/clingutils/src/TClingUtils.cxx | 21 ++++----- core/dictgen/src/DictSelectionReader.cxx | 3 +- core/dictgen/src/rootcling_impl.cxx | 7 +-- core/metacling/src/TCling.cxx | 22 ++++----- core/metacling/src/TClingBaseClassInfo.cxx | 10 ++--- core/metacling/src/TClingCallFunc.cxx | 3 +- core/metacling/src/TClingClassInfo.cxx | 11 ++--- core/metacling/src/TClingDataMemberInfo.cxx | 5 ++- core/metacling/src/TClingMemberIter.cxx | 9 ++-- core/metacling/src/TClingMethodArgInfo.cxx | 3 +- core/metacling/src/TClingMethodInfo.cxx | 11 ++--- core/metacling/src/TClingTypeInfo.cxx | 3 +- core/metacling/src/TClingTypedefInfo.cxx | 4 +- core/metacling/src/TClingTypedefInfo.h | 3 +- .../include/cling/Interpreter/Interpreter.h | 15 +------ .../cling/Interpreter/PushTransactionRAII.h | 35 +++++++++++++++ .../cling/lib/Interpreter/CMakeLists.txt | 1 + .../cling/lib/Interpreter/ClingPragmas.cpp | 3 +- .../cling/lib/Interpreter/Interpreter.cpp | 25 +---------- .../cling/lib/Interpreter/LookupHelper.cpp | 13 +++--- .../lib/Interpreter/PushTransactionRAII.cpp | 45 +++++++++++++++++++ .../cling/lib/Interpreter/ValuePrinter.cpp | 3 +- .../cling/lib/MetaProcessor/Display.cpp | 41 ++++++++--------- .../cling/test/Interfaces/transactionReuse.C | 3 +- 24 files changed, 181 insertions(+), 118 deletions(-) create mode 100644 interpreter/cling/include/cling/Interpreter/PushTransactionRAII.h create mode 100644 interpreter/cling/lib/Interpreter/PushTransactionRAII.cpp diff --git a/core/clingutils/src/TClingUtils.cxx b/core/clingutils/src/TClingUtils.cxx index b82eb7d23a72c..9a807aa8b6ba4 100644 --- a/core/clingutils/src/TClingUtils.cxx +++ b/core/clingutils/src/TClingUtils.cxx @@ -48,9 +48,10 @@ #include "clang/Sema/Sema.h" #include "clang/Sema/SemaDiagnostic.h" +#include "cling/Interpreter/Interpreter.h" #include "cling/Interpreter/LookupHelper.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "cling/Interpreter/Transaction.h" -#include "cling/Interpreter/Interpreter.h" #include "cling/Utils/AST.h" #include "llvm/Support/Path.h" @@ -818,7 +819,7 @@ bool ROOT::TMetaUtils::RequireCompleteType(const cling::Interpreter &interp, cla clang::Sema& S = interp.getCI()->getSema(); // Here we might not have an active transaction to handle // the caused instantiation decl. - cling::Interpreter::PushTransactionRAII RAII(const_cast(&interp)); + cling::PushTransactionRAII RAII(const_cast(&interp)); return S.RequireCompleteType(Loc, Type, clang::diag::err_incomplete_type); } @@ -1051,7 +1052,7 @@ bool ROOT::TMetaUtils::CheckDefaultConstructor(const clang::CXXRecordDecl* cl, c clang::CXXRecordDecl* ncCl = const_cast(cl); // We may induce template instantiation - cling::Interpreter::PushTransactionRAII clingRAII(const_cast(&interpreter)); + cling::PushTransactionRAII clingRAII(const_cast(&interpreter)); if (auto* Ctor = interpreter.getCI()->getSema().LookupDefaultConstructor(ncCl)) { if (Ctor->getAccess() == clang::AS_public && !Ctor->isDeleted()) { @@ -1084,7 +1085,7 @@ ROOT::TMetaUtils::EIOCtorCategory ROOT::TMetaUtils::CheckIOConstructor(const cla return EIOCtorCategory::kAbsent; // FIXME: We should not iterate here. That costs memory! - cling::Interpreter::PushTransactionRAII clingRAII(const_cast(&interpreter)); + cling::PushTransactionRAII clingRAII(const_cast(&interpreter)); for (auto iter = cl->ctor_begin(), end = cl->ctor_end(); iter != end; ++iter) { if ((iter->getAccess() != clang::AS_public) || (iter->getNumParams() != 1)) @@ -1235,7 +1236,7 @@ bool ROOT::TMetaUtils::NeedDestructor(const clang::CXXRecordDecl *cl, if (cl->hasUserDeclaredDestructor()) { - cling::Interpreter::PushTransactionRAII clingRAII(const_cast(&interp)); + cling::PushTransactionRAII clingRAII(const_cast(&interp)); clang::CXXDestructorDecl *dest = cl->getDestructor(); if (dest) { return (dest->getAccess() == clang::AS_public); @@ -3067,7 +3068,7 @@ clang::QualType ROOT::TMetaUtils::AddDefaultParameters(clang::QualType instanceT clang::TemplateTypeParmDecl *TTP = llvm::dyn_cast(*Param); { // We may induce template instantiation - cling::Interpreter::PushTransactionRAII clingRAII(const_cast(&interpreter)); + cling::PushTransactionRAII clingRAII(const_cast(&interpreter)); clang::sema::HackForDefaultTemplateArg raii; bool HasDefaultArgs; clang::TemplateArgumentLoc ArgType = S.SubstDefaultTemplateArgumentIfAvailable( @@ -3508,7 +3509,7 @@ void ROOT::TMetaUtils::GetFullyQualifiedTypeName(std::string &typenamestr, // We need this because GetFullyQualifiedTypeName is triggering deserialization // This calling the same name function GetFullyQualifiedTypeName, but this should stay here because // callee doesn't have an interpreter pointer - cling::Interpreter::PushTransactionRAII RAII(const_cast(&interpreter)); + cling::PushTransactionRAII RAII(const_cast(&interpreter)); GetFullyQualifiedTypeName(typenamestr, qtype, @@ -3665,7 +3666,7 @@ static bool areEqualTypes(const clang::TemplateArgument& tArg, TemplateArgument newArg = tArg; { clang::Sema& S = interp.getCI()->getSema(); - cling::Interpreter::PushTransactionRAII clingRAII(const_cast(&interp)); + cling::PushTransactionRAII clingRAII(const_cast(&interp)); clang::sema::HackForDefaultTemplateArg raii; // Hic sunt leones bool HasDefaultArgs; TemplateArgumentLoc defTArgLoc = S.SubstDefaultTemplateArgumentIfAvailable(Template, @@ -4024,7 +4025,7 @@ clang::QualType ROOT::TMetaUtils::GetNormalizedType(const clang::QualType &type, clang::ASTContext &ctxt = interpreter.getCI()->getASTContext(); // Modules can trigger deserialization. - cling::Interpreter::PushTransactionRAII RAII(const_cast(&interpreter)); + cling::PushTransactionRAII RAII(const_cast(&interpreter)); clang::QualType normalizedType = cling::utils::Transform::GetPartiallyDesugaredType(ctxt, type, normCtxt.GetConfig(), true /* fully qualify */); // Readd missing default template parameters @@ -4066,7 +4067,7 @@ void ROOT::TMetaUtils::GetNormalizedName(std::string &norm_name, const clang::Qu std::string normalizedNameStep1; // getAsStringInternal can trigger deserialization - cling::Interpreter::PushTransactionRAII clingRAII(const_cast(&interpreter)); + cling::PushTransactionRAII clingRAII(const_cast(&interpreter)); normalizedType.getAsStringInternal(normalizedNameStep1,policy); // Still remove the std:: and default template argument for STL container and diff --git a/core/dictgen/src/DictSelectionReader.cxx b/core/dictgen/src/DictSelectionReader.cxx index 38dd4c8b4c6e0..941ef0359e5ec 100644 --- a/core/dictgen/src/DictSelectionReader.cxx +++ b/core/dictgen/src/DictSelectionReader.cxx @@ -3,6 +3,7 @@ #include "clang/AST/AST.h" #include "cling/Interpreter/Interpreter.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "ClassSelectionRule.h" #include "SelectionRules.h" @@ -27,7 +28,7 @@ DictSelectionReader::DictSelectionReader(cling::Interpreter &interp, SelectionRu { // We push a new transaction because we could deserialize decls here - cling::Interpreter::PushTransactionRAII RAII(&interp); + cling::PushTransactionRAII RAII(&interp); // Inspect the AST TraverseDecl(translUnitDecl); } diff --git a/core/dictgen/src/rootcling_impl.cxx b/core/dictgen/src/rootcling_impl.cxx index e847b1ec213be..d42e55850803c 100644 --- a/core/dictgen/src/rootcling_impl.cxx +++ b/core/dictgen/src/rootcling_impl.cxx @@ -63,6 +63,7 @@ #include "cling/Interpreter/Interpreter.h" #include "cling/Interpreter/InterpreterCallbacks.h" #include "cling/Interpreter/LookupHelper.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "cling/Interpreter/Value.h" #include "clang/AST/CXXInheritance.h" #include "clang/Basic/Diagnostic.h" @@ -3066,7 +3067,7 @@ std::list RecordDecl2Headers(const clang::CXXRecordDecl &rcd, std::list headers; // We push a new transaction because we could deserialize decls here - cling::Interpreter::PushTransactionRAII RAII(&interp); + cling::PushTransactionRAII RAII(&interp); // Avoid infinite recursion if (!visitedDecls.insert(rcd.getCanonicalDecl()).second) @@ -4352,7 +4353,7 @@ int RootClingMain(int argc, } DepMod = GetModuleNameFromRdictName(DepMod); // We might deserialize. - cling::Interpreter::PushTransactionRAII RAII(&interp); + cling::PushTransactionRAII RAII(&interp); if (!interp.loadModule(DepMod, /*complain*/false)) { ROOT::TMetaUtils::Error(0, "Module '%s' failed to load.\n", DepMod.data()); @@ -5042,7 +5043,7 @@ int RootClingMain(int argc, // appropriate deconstructors in the interpreter. This writes out the C++ // module file that we currently generate. { - cling::Interpreter::PushTransactionRAII RAII(&interp); + cling::PushTransactionRAII RAII(&interp); CI->getSema().getASTConsumer().HandleTranslationUnit(CI->getSema().getASTContext()); } diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index 27834209f552c..715d9916cb6b2 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -1049,7 +1049,7 @@ static bool LoadModule(const std::string &ModuleName, cling::Interpreter &interp ::Info("TCling::__LoadModule", "Preloading module %s. \n", ModuleName.c_str()); - cling::Interpreter::PushTransactionRAII deserRAII(&interp); + cling::PushTransactionRAII deserRAII(&interp); return interp.loadModule(ModuleName, /*Complain=*/true); } @@ -1115,7 +1115,7 @@ static GlobalModuleIndex *loadGlobalModuleIndex(cling::Interpreter &interp) RecreateIndex = true; } if (RecreateIndex) { - cling::Interpreter::PushTransactionRAII deserRAII(&interp); + cling::PushTransactionRAII deserRAII(&interp); clang::GlobalModuleIndex::UserDefinedInterestingIDs IDs; struct DefinitionFinder : public RecursiveASTVisitor { @@ -1844,7 +1844,7 @@ void TCling::LoadPCM(std::string pcmFileNameFullPath) std::string RDictFileOpts = pcmFileNameFullPath + "?filetype=pcm"; TMemFile pcmMemFile(RDictFileOpts.c_str(), range); - cling::Interpreter::PushTransactionRAII deserRAII(GetInterpreterImpl()); + cling::PushTransactionRAII deserRAII(GetInterpreterImpl()); LoadPCMImpl(pcmMemFile); fPendingRdicts.erase(pendingRdict); @@ -2748,7 +2748,7 @@ void TCling::InspectMembers(TMemberInspector& insp, const void* obj, { // Force possible deserializations first. We need to have no pending // Transaction when passing control flow to the inspector below (ROOT-7779). - cling::Interpreter::PushTransactionRAII deserRAII(GetInterpreterImpl()); + cling::PushTransactionRAII deserRAII(GetInterpreterImpl()); astContext.getASTRecordLayout(recordDecl); @@ -3746,7 +3746,7 @@ Int_t TCling::DeleteVariable(const char* name) unscopedName += posScope + 2; } // Could trigger deserialization of decls. - cling::Interpreter::PushTransactionRAII RAII(GetInterpreterImpl()); + cling::PushTransactionRAII RAII(GetInterpreterImpl()); clang::NamedDecl* nVarDecl = cling::utils::Lookup::Named(&fInterpreter->getSema(), unscopedName, declCtx); if (!nVarDecl) { @@ -4292,7 +4292,7 @@ void TCling::LoadEnums(TListOfEnums& enumList) const } // Iterate on the decl of the class and get the enums. if (const clang::DeclContext* DC = dyn_cast(D)) { - cling::Interpreter::PushTransactionRAII deserRAII(GetInterpreterImpl()); + cling::PushTransactionRAII deserRAII(GetInterpreterImpl()); // Collect all contexts of the namespace. llvm::SmallVector< DeclContext *, 4> allDeclContexts; const_cast< clang::DeclContext *>(DC)->collectAllContexts(allDeclContexts); @@ -4341,7 +4341,7 @@ void TCling::LoadFunctionTemplates(TClass* cl) const } // Iterate on the decl of the class and get the enums. if (const clang::DeclContext* DC = dyn_cast(D)) { - cling::Interpreter::PushTransactionRAII deserRAII(GetInterpreterImpl()); + cling::PushTransactionRAII deserRAII(GetInterpreterImpl()); // Collect all contexts of the namespace. llvm::SmallVector< DeclContext *, 4> allDeclContexts; const_cast< clang::DeclContext *>(DC)->collectAllContexts(allDeclContexts); @@ -4686,7 +4686,7 @@ TInterpreter::DeclId_t TCling::GetDataMember(ClassInfo_t *opaque_cl, const char Sema::ForExternalRedeclaration); // Could trigger deserialization of decls. - cling::Interpreter::PushTransactionRAII RAII(GetInterpreterImpl()); + cling::PushTransactionRAII RAII(GetInterpreterImpl()); cling::utils::Lookup::Named(&SemaR, R); LookupResult::Filter F = R.makeFilter(); @@ -4727,7 +4727,7 @@ TInterpreter::DeclId_t TCling::GetEnum(TClass *cl, const char *name) const if (dc) { // If it is a data member enum. // Could trigger deserialization of decls. - cling::Interpreter::PushTransactionRAII RAII(GetInterpreterImpl()); + cling::PushTransactionRAII RAII(GetInterpreterImpl()); possibleEnum = cling::utils::Lookup::Tag(&fInterpreter->getSema(), name, dc); } else { Error("TCling::GetEnum", "DeclContext not found for %s .\n", name); @@ -4736,7 +4736,7 @@ TInterpreter::DeclId_t TCling::GetEnum(TClass *cl, const char *name) const } else { // If it is a global enum. // Could trigger deserialization of decls. - cling::Interpreter::PushTransactionRAII RAII(GetInterpreterImpl()); + cling::PushTransactionRAII RAII(GetInterpreterImpl()); possibleEnum = cling::utils::Lookup::Tag(&fInterpreter->getSema(), name); } if (possibleEnum && (possibleEnum != (clang::Decl*)-1) @@ -6807,7 +6807,7 @@ void TCling::UpdateListsOnCommitted(const cling::Transaction &T) { continue; } // Could trigger deserialization of decls. - cling::Interpreter::PushTransactionRAII RAII(GetInterpreterImpl()); + cling::PushTransactionRAII RAII(GetInterpreterImpl()); // Unlock the TClass for updates ((TCling*)gCling)->GetModTClasses().erase(*I); diff --git a/core/metacling/src/TClingBaseClassInfo.cxx b/core/metacling/src/TClingBaseClassInfo.cxx index 0f2c869ca6432..610f2307f08ea 100644 --- a/core/metacling/src/TClingBaseClassInfo.cxx +++ b/core/metacling/src/TClingBaseClassInfo.cxx @@ -30,7 +30,7 @@ the Clang C++ compiler, not CINT. #include "cling/Interpreter/Interpreter.h" #include "cling/Interpreter/Transaction.h" - +#include "cling/Interpreter/PushTransactionRAII.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" @@ -78,7 +78,7 @@ TClingBaseClassInfo::TClingBaseClassInfo(cling::Interpreter* interp, fDecl = CRD; { // In particular if the base are templated, this might deserialize. - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); fIter = CRD->bases_begin(); } } @@ -111,7 +111,7 @@ TClingBaseClassInfo::TClingBaseClassInfo(cling::Interpreter* interp, clang::CXXBasePaths Paths; // CXXRecordDecl::isDerivedFrom can trigger deserialization. - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); if (!CRD->isDerivedFrom(BaseCRD, Paths)) { //Not valid fBaseInfo = 0. @@ -274,7 +274,7 @@ int TClingBaseClassInfo::InternalNext(int onlyDirect) // now we process the bases of that base class. // At least getASTRecordLayout() might deserialize. - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); fDescend = false; const clang::RecordType *Ty = fIter->getType()-> getAs(); @@ -389,7 +389,7 @@ static clang::CharUnits computeOffsetHint(clang::ASTContext &Context, continue; // Accumulate the base class offsets. - cling::Interpreter::PushTransactionRAII RAII(interp); + cling::PushTransactionRAII RAII(interp); const clang::ASTRecordLayout &L = Context.getASTRecordLayout(J->Class); Offset += L.getBaseClassOffset(J->Base->getType()->getAsCXXRecordDecl()); } diff --git a/core/metacling/src/TClingCallFunc.cxx b/core/metacling/src/TClingCallFunc.cxx index 6545035d73f94..4646fdb7059b2 100644 --- a/core/metacling/src/TClingCallFunc.cxx +++ b/core/metacling/src/TClingCallFunc.cxx @@ -36,6 +36,7 @@ C++ interpreter and the Clang C++ compiler, not CINT. #include "cling/Interpreter/Interpreter.h" #include "cling/Interpreter/LookupHelper.h" #include "cling/Interpreter/Transaction.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "cling/Interpreter/Value.h" #include "cling/Utils/AST.h" @@ -809,7 +810,7 @@ int TClingCallFunc::get_wrapper_code(std::string &wrapper_name, std::string &wra clang::FunctionDecl *FDmod = const_cast(FD); clang::Sema &S = fInterp->getSema(); // Could trigger deserialization of decls. - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); S.InstantiateFunctionDefinition(SourceLocation(), FDmod, /*Recursive=*/true, /*DefinitionRequired=*/true); diff --git a/core/metacling/src/TClingClassInfo.cxx b/core/metacling/src/TClingClassInfo.cxx index 042bfa133bf36..fad281786c7a2 100644 --- a/core/metacling/src/TClingClassInfo.cxx +++ b/core/metacling/src/TClingClassInfo.cxx @@ -33,6 +33,7 @@ but the class metadata comes from the Clang C++ compiler, not CINT. #include "cling/Interpreter/Interpreter.h" #include "cling/Interpreter/LookupHelper.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "cling/Utils/AST.h" #include "clang/AST/ASTContext.h" @@ -148,7 +149,7 @@ long TClingClassInfo::ClassProperty() const const RecordDecl *RD = llvm::dyn_cast(GetDecl()); // isAbstract and other calls can trigger deserialization - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); if (!RD) { // We are an enum or namespace. @@ -648,7 +649,7 @@ std::vector TClingClassInfo::GetUsingNamespaces() R__LOCKGUARD(gInterpreterMutex); - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); const auto DC = dyn_cast(fDecl); if (!DC) return res; @@ -931,7 +932,7 @@ int TClingClassInfo::InternalNext() fDeclFileName.clear(); // invalidate decl file name. fNameCache.clear(); // invalidate the cache. - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); if (fFirstTime) { // GetDecl() must be a DeclContext in order to iterate. const clang::DeclContext *DC = cast(GetDecl()); @@ -1258,7 +1259,7 @@ long TClingClassInfo::Property() const property |= kIsCPPCompiled; // Modules can deserialize while querying the various decls for information. - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); const clang::DeclContext *ctxt = GetDecl()->getDeclContext(); clang::NamespaceDecl *std_ns =fInterp->getSema().getStdNamespace(); @@ -1342,7 +1343,7 @@ int TClingClassInfo::Size() const return 0; } ASTContext &Context = GetDecl()->getASTContext(); - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD); int64_t size = Layout.getSize().getQuantity(); int clang_size = static_cast(size); diff --git a/core/metacling/src/TClingDataMemberInfo.cxx b/core/metacling/src/TClingDataMemberInfo.cxx index 9d9fa3a466ef8..ed7ca8e5b32c4 100644 --- a/core/metacling/src/TClingDataMemberInfo.cxx +++ b/core/metacling/src/TClingDataMemberInfo.cxx @@ -32,6 +32,7 @@ from the Clang C++ compiler, not CINT. #include "TVirtualMutex.h" #include "cling/Interpreter/Interpreter.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "clang/AST/Attr.h" #include "clang/AST/ASTContext.h" @@ -335,7 +336,7 @@ long TClingDataMemberInfo::Offset() // Could trigger deserialization of decls, in particular in case // of constexpr, like: // static constexpr Long64_t something = std::numeric_limits::max(); - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); if (long addr = reinterpret_cast(fInterp->getAddressOfGlobal(GlobalDecl(VD)))) return addr; @@ -481,7 +482,7 @@ long TClingDataMemberInfo::Property() const const clang::TagType *tt = qt->getAs(); if (tt) { // tt->getDecl() might deserialize. - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); const clang::TagDecl *td = tt->getDecl(); if (td->isClass()) { property |= kIsClass; diff --git a/core/metacling/src/TClingMemberIter.cxx b/core/metacling/src/TClingMemberIter.cxx index ba19b05cd4674..c08eef7064e86 100644 --- a/core/metacling/src/TClingMemberIter.cxx +++ b/core/metacling/src/TClingMemberIter.cxx @@ -13,12 +13,13 @@ #include "cling/Interpreter/Interpreter.h" #include "cling/Interpreter/LookupHelper.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "clang/AST/DeclTemplate.h" ClingMemberIterInternal::DCIter::DCIter(clang::DeclContext *DC, cling::Interpreter *interp) : fInterp(interp) { - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); DC->collectAllContexts(fContexts); fDeclIter = fContexts[0]->decls_begin(); // Skip initial empty decl contexts. @@ -76,7 +77,7 @@ bool ClingMemberIterInternal::DCIter::IterNext() ++fDCIdx; if (fDCIdx == fContexts.size()) return false; - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); fDeclIter = fContexts[fDCIdx]->decls_begin(); } return true; @@ -91,7 +92,7 @@ bool ClingMemberIterInternal::DCIter::Next() ClingMemberIterInternal::UsingDeclIter::UsingDeclIter(const clang::UsingDecl *UD, cling::Interpreter *interp) : fInterp(interp) { - cling::Interpreter::PushTransactionRAII RAII(interp); + cling::PushTransactionRAII RAII(interp); fUsingIterStack.push({UD}); } @@ -109,7 +110,7 @@ bool ClingMemberIterInternal::UsingDeclIter::Next() } if (auto *UD = llvm::dyn_cast(Iter()->getTargetDecl())) { if (UD->shadow_size()) { - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); fUsingIterStack.push({UD}); // Continue with child. } diff --git a/core/metacling/src/TClingMethodArgInfo.cxx b/core/metacling/src/TClingMethodArgInfo.cxx index 1e33c62641bfa..1fbd36283c64f 100644 --- a/core/metacling/src/TClingMethodArgInfo.cxx +++ b/core/metacling/src/TClingMethodArgInfo.cxx @@ -27,6 +27,7 @@ the Clang C++ compiler, not CINT. #include "ThreadLocalStorage.h" #include "cling/Interpreter/Interpreter.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" #include "clang/AST/Expr.h" @@ -86,7 +87,7 @@ const char *TClingMethodArgInfo::DefaultValue() const // Instantiate default arg if needed if (pvd->hasUninstantiatedDefaultArg()) { // Could deserialize / create instantiated decls. - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); auto fd = llvm::cast_or_null(TClingDeclInfo::GetDecl()); fInterp->getSema().BuildCXXDefaultArgExpr(clang::SourceLocation(), const_cast(fd), diff --git a/core/metacling/src/TClingMethodInfo.cxx b/core/metacling/src/TClingMethodInfo.cxx index 8e28ed54ae77c..ac8bef4956491 100644 --- a/core/metacling/src/TClingMethodInfo.cxx +++ b/core/metacling/src/TClingMethodInfo.cxx @@ -34,6 +34,7 @@ compiler, not CINT. #include "cling/Interpreter/Interpreter.h" #include "cling/Interpreter/LookupHelper.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "cling/Utils/AST.h" #include "clang/AST/ASTContext.h" @@ -69,7 +70,7 @@ TClingCXXRecMethIter::SpecFuncIter::SpecFuncIter(cling::Interpreter *interp, cla return; // Could trigger deserialization of decls. - cling::Interpreter::PushTransactionRAII RAII(interp); + cling::PushTransactionRAII RAII(interp); auto emplaceSpecFunIfNeeded = [&](clang::CXXMethodDecl *D) { if (!D) @@ -201,7 +202,7 @@ TClingCXXRecMethIter::InstantiateTemplateWithDefaults(const clang::RedeclarableT } } - cling::Interpreter::PushTransactionRAII RAII(interp); + cling::PushTransactionRAII RAII(interp); // Now substitute the dependent function parameter types given defaultTemplateArgs. llvm::SmallVector paramTypes; @@ -264,7 +265,7 @@ TClingMethodInfo::TClingMethodInfo(cling::Interpreter *interp, // DeclContext content! // Could trigger deserialization of decls. - cling::Interpreter::PushTransactionRAII RAII(interp); + cling::PushTransactionRAII RAII(interp); auto &SemaRef = interp->getSema(); SemaRef.ForceDeclarationOfImplicitMembers(CXXRD); @@ -594,7 +595,7 @@ std::string TClingMethodInfo::GetMangledName() const const FunctionDecl* D = GetTargetFunctionDecl(); R__LOCKGUARD(gInterpreterMutex); - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); GlobalDecl GD; if (const CXXConstructorDecl* Ctor = dyn_cast(D)) GD = GlobalDecl(Ctor, Ctor_Complete); @@ -690,7 +691,7 @@ const char *TClingMethodInfo::Title() R__LOCKGUARD(gInterpreterMutex); // Could trigger deserialization of decls. - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); if (const FunctionDecl *AnnotFD = ROOT::TMetaUtils::GetAnnotatedRedeclarable(FD)) { if (AnnotateAttr *A = AnnotFD->getAttr()) { diff --git a/core/metacling/src/TClingTypeInfo.cxx b/core/metacling/src/TClingTypeInfo.cxx index 3a3c124f330ab..5164b4ff78f15 100644 --- a/core/metacling/src/TClingTypeInfo.cxx +++ b/core/metacling/src/TClingTypeInfo.cxx @@ -29,6 +29,7 @@ but the type metadata comes from the Clang C++ compiler, not CINT. #include "cling/Interpreter/Interpreter.h" #include "cling/Interpreter/LookupHelper.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "cling/Utils/AST.h" #include "clang/AST/ASTContext.h" @@ -150,7 +151,7 @@ long TClingTypeInfo::Property() const property |= kIsUnion; } // isAbstract can trigger deserialization - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); if (CRD->isThisDeclarationADefinition() && CRD->isAbstract()) { property |= kIsAbstract; } diff --git a/core/metacling/src/TClingTypedefInfo.cxx b/core/metacling/src/TClingTypedefInfo.cxx index e708200069b7c..bdee87d58efb6 100644 --- a/core/metacling/src/TClingTypedefInfo.cxx +++ b/core/metacling/src/TClingTypedefInfo.cxx @@ -117,7 +117,7 @@ int TClingTypedefInfo::InternalNext() return 0; } // Deserialization might happen during the iteration. - cling::Interpreter::PushTransactionRAII pushedT(fInterp); + cling::PushTransactionRAII pushedT(fInterp); while (true) { // Advance to next usable decl, or return if // there is no next usable decl. @@ -216,7 +216,7 @@ int TClingTypedefInfo::Size() const } // Deserialization might happen during the size calculation. - cling::Interpreter::PushTransactionRAII pushedT(fInterp); + cling::PushTransactionRAII pushedT(fInterp); // Note: This is an int64_t. clang::CharUnits::QuantityType quantity = diff --git a/core/metacling/src/TClingTypedefInfo.h b/core/metacling/src/TClingTypedefInfo.h index 6b6fceb4d3f7a..7b96fec3be5d0 100644 --- a/core/metacling/src/TClingTypedefInfo.h +++ b/core/metacling/src/TClingTypedefInfo.h @@ -29,6 +29,7 @@ #include "TClingDeclInfo.h" #include "cling/Interpreter/Interpreter.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" #include "clang/Frontend/CompilerInstance.h" @@ -60,7 +61,7 @@ class TClingTypedefInfo final : public TClingDeclInfo { { const clang::TranslationUnitDecl *TU = fInterp->getCI()->getASTContext().getTranslationUnitDecl(); const clang::DeclContext *DC = llvm::cast(TU); - cling::Interpreter::PushTransactionRAII RAII(fInterp); + cling::PushTransactionRAII RAII(fInterp); fIter = DC->decls_begin(); } diff --git a/interpreter/cling/include/cling/Interpreter/Interpreter.h b/interpreter/cling/include/cling/Interpreter/Interpreter.h index 5419db86e3dec..0795a50f4f258 100644 --- a/interpreter/cling/include/cling/Interpreter/Interpreter.h +++ b/interpreter/cling/include/cling/Interpreter/Interpreter.h @@ -72,6 +72,7 @@ namespace cling { class InterpreterCallbacks; class LookupHelper; class Value; + class PushTransactionRAII; class Transaction; class IncrementalCUDADeviceCompiler; @@ -84,19 +85,6 @@ namespace cling { // include the actual definition of PresumedLoc. using IgnoreFilesFunc_t = bool (*)(const clang::PresumedLoc&); - ///\brief Pushes a new transaction, which will collect the decls that came - /// within the scope of the RAII object. Calls commit transaction at - /// destruction. - class PushTransactionRAII { - private: - Transaction* m_Transaction; - const Interpreter* m_Interpreter; - public: - PushTransactionRAII(const Interpreter* i); - ~PushTransactionRAII(); - void pop() const; - }; - class StateDebuggerRAII { private: const Interpreter* m_Interpreter; @@ -827,6 +815,7 @@ namespace cling { [](const clang::PresumedLoc&) { return false;}) const; friend class runtime::internal::LifetimeHandler; + friend class PushTransactionRAII; }; } // namespace cling diff --git a/interpreter/cling/include/cling/Interpreter/PushTransactionRAII.h b/interpreter/cling/include/cling/Interpreter/PushTransactionRAII.h new file mode 100644 index 0000000000000..0e0cc33ef1ede --- /dev/null +++ b/interpreter/cling/include/cling/Interpreter/PushTransactionRAII.h @@ -0,0 +1,35 @@ +//--------------------------------------------------------------------*- C++ -*- +// CLING - the C++ LLVM-based InterpreterG :) +// author: Vassil Vassilev +// +// 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_PUSHTRANSACTIONRAII_H +#define CLING_PUSHTRANSACTIONRAII_H + +namespace clang { + class PresumedLoc; +} +namespace cling { + class Interpreter; + class Transaction; + + ///\brief Pushes a new transaction, which will collect the decls that came + /// within the scope of the RAII object. Calls commit transaction at + /// destruction. + class PushTransactionRAII { + private: + Transaction* m_Transaction; + const Interpreter* m_Interpreter; + public: + PushTransactionRAII(const Interpreter* i); + ~PushTransactionRAII(); + void pop() const; + }; + +} // namespace cling + +#endif // CLING_PUSHTRANSACTIONRAII_H diff --git a/interpreter/cling/lib/Interpreter/CMakeLists.txt b/interpreter/cling/lib/Interpreter/CMakeLists.txt index 75396717355f5..0fda3fd1a8046 100644 --- a/interpreter/cling/lib/Interpreter/CMakeLists.txt +++ b/interpreter/cling/lib/Interpreter/CMakeLists.txt @@ -87,6 +87,7 @@ add_cling_library(clingInterpreter OBJECT InvocationOptions.cpp LookupHelper.cpp NullDerefProtectionTransformer.cpp + PushTransactionRAII.cpp RequiredSymbols.cpp Transaction.cpp TransactionUnloader.cpp diff --git a/interpreter/cling/lib/Interpreter/ClingPragmas.cpp b/interpreter/cling/lib/Interpreter/ClingPragmas.cpp index 5dfd7db63ac83..ad7b854c090b6 100644 --- a/interpreter/cling/lib/Interpreter/ClingPragmas.cpp +++ b/interpreter/cling/lib/Interpreter/ClingPragmas.cpp @@ -10,6 +10,7 @@ #include "ClingPragmas.h" #include "cling/Interpreter/Interpreter.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "cling/Interpreter/Transaction.h" #include "cling/Utils/Output.h" #include "cling/Utils/Paths.h" @@ -154,7 +155,7 @@ namespace { m_Interp.getCI()->getASTContext().getTranslationUnitDecl(); Sema::ContextAndScopeRAII pushedDCAndS(m_Interp.getSema(), TU, m_Interp.getSema().TUScope); - Interpreter::PushTransactionRAII pushedT(&m_Interp); + PushTransactionRAII pushedT(&m_Interp); for (const LibraryFileInfo& FI : FileInfos) { // FIXME: Consider the case where the library static init section has diff --git a/interpreter/cling/lib/Interpreter/Interpreter.cpp b/interpreter/cling/lib/Interpreter/Interpreter.cpp index 9686a1dcff42d..3f41ab17cfe20 100644 --- a/interpreter/cling/lib/Interpreter/Interpreter.cpp +++ b/interpreter/cling/lib/Interpreter/Interpreter.cpp @@ -33,6 +33,7 @@ #include "cling/Interpreter/Exception.h" #include "cling/Interpreter/IncrementalCUDADeviceCompiler.h" #include "cling/Interpreter/LookupHelper.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "cling/Interpreter/Transaction.h" #include "cling/Interpreter/Value.h" #include "cling/Utils/AST.h" @@ -92,30 +93,6 @@ namespace { namespace cling { - Interpreter::PushTransactionRAII::PushTransactionRAII(const Interpreter* i) - : m_Interpreter(i) { - CompilationOptions CO = m_Interpreter->makeDefaultCompilationOpts(); - CO.ResultEvaluation = 0; - CO.DynamicScoping = 0; - - m_Transaction = m_Interpreter->m_IncrParser->beginTransaction(CO); - } - - Interpreter::PushTransactionRAII::~PushTransactionRAII() { - pop(); - } - - void Interpreter::PushTransactionRAII::pop() const { - if (m_Transaction->getState() == Transaction::kRolledBack) - return; - IncrementalParser::ParseResultTransaction PRT - = m_Interpreter->m_IncrParser->endTransaction(m_Transaction); - if (PRT.getPointer()) { - assert(PRT.getPointer()==m_Transaction && "Ended different transaction?"); - m_Interpreter->m_IncrParser->commitTransaction(PRT); - } - } - Interpreter::StateDebuggerRAII::StateDebuggerRAII(const Interpreter* i) : m_Interpreter(i) { if (m_Interpreter->isPrintingDebug()) { diff --git a/interpreter/cling/lib/Interpreter/LookupHelper.cpp b/interpreter/cling/lib/Interpreter/LookupHelper.cpp index 49bb135160f4e..c0b32021daf8e 100644 --- a/interpreter/cling/lib/Interpreter/LookupHelper.cpp +++ b/interpreter/cling/lib/Interpreter/LookupHelper.cpp @@ -12,6 +12,7 @@ #include "DeclUnloader.h" #include "cling/Interpreter/Interpreter.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "cling/Utils/AST.h" #include "cling/Utils/ParserStateRAII.h" @@ -477,7 +478,7 @@ namespace cling { if (typeName.empty()) return TheQT; // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(m_Interpreter); + PushTransactionRAII RAII(m_Interpreter); // Deal with the most common case. // Going through this custom finder is both much faster @@ -538,7 +539,7 @@ namespace cling { // Here we might not have an active transaction to handle // the caused instantiation decl. // Also quickFindDecl could trigger deserialization of decls. - Interpreter::PushTransactionRAII pushedT(m_Interpreter); + PushTransactionRAII pushedT(m_Interpreter); // See if we can find it without a buffer and any clang parsing, // We need to go scope by scope. @@ -890,7 +891,7 @@ namespace cling { } if (where) { // Great we now have a scope and something to search for,let's go ahead. - Interpreter::PushTransactionRAII pushedT(m_Interpreter); + PushTransactionRAII pushedT(m_Interpreter); DeclContext::lookup_result R = where->lookup(P.getCurToken().getIdentifierInfo()); for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); @@ -917,7 +918,7 @@ namespace cling { const clang::DeclContext *dc = llvm::cast(scopeDecl); - Interpreter::PushTransactionRAII pushedT(m_Interpreter); + PushTransactionRAII pushedT(m_Interpreter); DeclContext::lookup_result lookup = const_cast(dc)->lookup(decl_name); for (DeclContext::lookup_iterator I = lookup.begin(), E = lookup.end(); I != E; ++I) { @@ -1570,7 +1571,7 @@ namespace cling { llvm::SmallVector GivenArgs; if (!inputEval(GivenArgs,funcArgs,diagOnOff,P,Interp,LH)) return 0; - Interpreter::PushTransactionRAII pushedT(Interp); + PushTransactionRAII pushedT(Interp); return findFunction(foundDC, funcName, GivenArgs, objectIsConst, Context, Interp, functionSelector, @@ -1924,7 +1925,7 @@ namespace cling { // Parse the arguments now. // - Interpreter::PushTransactionRAII TforDeser(Interp); + PushTransactionRAII TforDeser(Interp); StartParsingRAII ParseStarted(LH, funcArgs, llvm::StringRef("func.args.file"), diagOnOff); diff --git a/interpreter/cling/lib/Interpreter/PushTransactionRAII.cpp b/interpreter/cling/lib/Interpreter/PushTransactionRAII.cpp new file mode 100644 index 0000000000000..709161e596fbf --- /dev/null +++ b/interpreter/cling/lib/Interpreter/PushTransactionRAII.cpp @@ -0,0 +1,45 @@ +//------------------------------------------------------------------------------ +// CLING - the C++ LLVM-based InterpreterG :) +// author: Vassil Vassilev +// +// 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. +//------------------------------------------------------------------------------ + +#include "cling/Interpreter/PushTransactionRAII.h" + +#include "cling/Interpreter/Interpreter.h" +#include "cling/Interpreter/Transaction.h" + +#include "IncrementalParser.h" + +namespace cling { + + PushTransactionRAII::PushTransactionRAII(const Interpreter* i) + : m_Interpreter(i) { + CompilationOptions CO = m_Interpreter->makeDefaultCompilationOpts(); + CO.ResultEvaluation = 0; + CO.DynamicScoping = 0; + + m_Transaction = m_Interpreter->m_IncrParser->beginTransaction(CO); + m_Transaction->setScope(*this); + } + + PushTransactionRAII::~PushTransactionRAII() { + pop(); + } + + void PushTransactionRAII::pop() const { + assert(m_Transaction->getScope() == this && "transaction not owned by *this"); + if (m_Transaction->getState() == Transaction::kRolledBack) + return; + IncrementalParser::ParseResultTransaction PRT + = m_Interpreter->m_IncrParser->endTransaction(m_Transaction); + if (PRT.getPointer()) { + assert(PRT.getPointer()==m_Transaction && "Ended different transaction?"); + m_Interpreter->m_IncrParser->commitTransaction(PRT); + } + } + +} //end namespace cling diff --git a/interpreter/cling/lib/Interpreter/ValuePrinter.cpp b/interpreter/cling/lib/Interpreter/ValuePrinter.cpp index 1d8ecd96c7f16..b322d84062698 100644 --- a/interpreter/cling/lib/Interpreter/ValuePrinter.cpp +++ b/interpreter/cling/lib/Interpreter/ValuePrinter.cpp @@ -15,6 +15,7 @@ #include "cling/Interpreter/Interpreter.h" #include "cling/Interpreter/LookupHelper.h" #include "cling/Interpreter/Transaction.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "cling/Interpreter/Value.h" #include "cling/Utils/AST.h" #include "cling/Utils/Casting.h" @@ -577,7 +578,7 @@ static const char* BuildAndEmitVPWrapperBody(cling::Interpreter &Interp, clang::LookupResult R(S, PVDN, noSrcLoc, clang::Sema::LookupOrdinaryName); // The subsequent lookup might deserialize or instantiate. - Interpreter::PushTransactionRAII ScopedT(&Interp); + PushTransactionRAII ScopedT(&Interp); S.LookupQualifiedName(R, clingNS); clang::Expr *OverldExpr = clang::UnresolvedLookupExpr::Create(Ctx, nullptr /*namingClass*/, diff --git a/interpreter/cling/lib/MetaProcessor/Display.cpp b/interpreter/cling/lib/MetaProcessor/Display.cpp index bce7c87e3fbdf..25ec02213d9c4 100644 --- a/interpreter/cling/lib/MetaProcessor/Display.cpp +++ b/interpreter/cling/lib/MetaProcessor/Display.cpp @@ -11,6 +11,7 @@ #include "cling/Interpreter/Interpreter.h" #include "cling/Interpreter/LookupHelper.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclBase.h" @@ -525,7 +526,7 @@ void ClassPrinter::DisplayAllClasses()const fOut.Print("List of classes"); // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); for (decl_iterator decl = tuDecl->decls_begin(); decl != tuDecl->decls_end(); ++decl) ProcessDecl(decl); } @@ -612,7 +613,7 @@ void ClassPrinter::ProcessBlockDecl(decl_iterator decl)const assert(blockDecl != 0 && "ProcessBlockDecl, internal error - decl is not a BlockDecl"); // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); for (decl_iterator it = blockDecl->decls_begin(); it != blockDecl->decls_end(); ++it) ProcessDecl(it); } @@ -629,7 +630,7 @@ void ClassPrinter::ProcessFunctionDecl(decl_iterator decl)const assert(functionDecl != 0 && "ProcessFunctionDecl, internal error - decl is not a FunctionDecl"); // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); for (decl_iterator it = functionDecl->decls_begin(); it != functionDecl->decls_end(); ++it) ProcessDecl(it); } @@ -647,7 +648,7 @@ void ClassPrinter::ProcessNamespaceDecl(decl_iterator decl)const assert(namespaceDecl != 0 && "ProcessNamespaceDecl, 'decl' parameter is not a NamespaceDecl"); // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); for (decl_iterator it = namespaceDecl->decls_begin(); it != namespaceDecl->decls_end(); ++it) ProcessDecl(it); } @@ -663,7 +664,7 @@ void ClassPrinter::ProcessLinkageSpecDecl(decl_iterator decl)const assert(linkageSpec != 0 && "ProcessLinkageSpecDecl, decl is not a LinkageSpecDecl"); // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); for (decl_iterator it = linkageSpec->decls_begin(); it != linkageSpec->decls_end(); ++it) ProcessDecl(it); } @@ -685,7 +686,7 @@ void ClassPrinter::ProcessClassDecl(decl_iterator decl) const DisplayClassDecl(classDecl); // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); // Now we have to check nested scopes for class declarations. for (decl_iterator nestedDecl = classDecl->decls_begin(); nestedDecl != classDecl->decls_end(); ++nestedDecl) @@ -707,7 +708,7 @@ void ClassPrinter::ProcessClassTemplateDecl(decl_iterator decl)const return; // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); //Now we have to display all the specialization (/instantiations) for (ClassTemplateDecl::spec_iterator spec = templateDecl->spec_begin(); spec != templateDecl->spec_end(); ++spec) @@ -721,7 +722,7 @@ void ClassPrinter::DisplayClassDecl(const CXXRecordDecl* classDecl)const assert(fInterpreter != 0 && "DisplayClassDecl, fInterpreter is null"); // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); classDecl = classDecl->getDefinition(); assert(classDecl != 0 && "DisplayClassDecl, invalid decl - no definition"); @@ -835,7 +836,7 @@ void ClassPrinter::DisplayBasesAsList(const CXXRecordDecl* classDecl)const //we print a list of base classes as one line, with access specifiers and 'virtual' if needed. std::string bases(": "); // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); for (base_decl_iterator baseIt = classDecl->bases_begin(); baseIt != classDecl->bases_end(); ++baseIt) { if (baseIt != classDecl->bases_begin()) bases += ", "; @@ -868,7 +869,7 @@ void ClassPrinter::DisplayBasesAsTree(const CXXRecordDecl* classDecl, unsigned n std::string textLine; // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); for (base_decl_iterator baseIt = classDecl->bases_begin(); baseIt != classDecl->bases_end(); ++baseIt) { textLine.assign(nSpaces, ' '); const RecordType* const type = baseIt->getType()->getAs(); @@ -906,7 +907,7 @@ void ClassPrinter::DisplayMemberFunctions(const CXXRecordDecl* classDecl)const std::string textLine; // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); for (ctor_iterator ctor = classDecl->ctor_begin(); ctor != classDecl->ctor_end(); ++ctor) { if (ctor->isImplicit())//Compiler-generated. continue; @@ -988,7 +989,7 @@ void ClassPrinter::DisplayDataMembers(const CXXRecordDecl* classDecl, unsigned n const std::string gap(std::max(nSpaces, 1u), ' '); // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); for (field_iterator field = classDecl->field_begin(); field != classDecl->field_end(); ++field) { textLine.clear(); @@ -1144,7 +1145,7 @@ void GlobalsPrinter::DisplayGlobals()const assert(tuDecl != 0 && "DisplayGlobals, translation unit is empty"); // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); //Try to print global macro definitions (object-like only). const Preprocessor& pp = compiler->getPreprocessor(); @@ -1182,7 +1183,7 @@ void GlobalsPrinter::DisplayGlobal(const std::string& name)const unsigned count = 0; // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); const Preprocessor& pp = compiler->getPreprocessor(); for (macro_iterator macro = pp.macro_begin(); macro != pp.macro_end(); ++macro) { auto* MD = macro->second.getLatest(); @@ -1449,7 +1450,7 @@ void TypedefPrinter::ProcessNestedDeclarations(const DeclContext* decl)const { assert(decl != 0 && "ProcessNestedDeclarations, parameter 'decl' is null"); // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(fInterpreter)); + PushTransactionRAII RAII(const_cast(fInterpreter)); for (decl_iterator it = decl->decls_begin(), eIt = decl->decls_end(); it != eIt; ++it) ProcessDecl(it); } @@ -1552,10 +1553,10 @@ void DisplayClass(llvm::raw_ostream& stream, const Interpreter* interpreter, void DisplayNamespaces(llvm::raw_ostream &stream, const Interpreter *interpreter) { assert(interpreter != 0 && "DisplayNamespaces, parameter 'interpreter' is null"); - Interpreter::PushTransactionRAII RAII(const_cast(interpreter)); + PushTransactionRAII RAII(const_cast(interpreter)); NamespacePrinter printer(stream, interpreter); - Interpreter::PushTransactionRAII guard(const_cast(interpreter)); + PushTransactionRAII guard(const_cast(interpreter)); printer.Print(); } @@ -1566,7 +1567,7 @@ void DisplayGlobals(llvm::raw_ostream& stream, const Interpreter* interpreter) GlobalsPrinter printer(stream, interpreter); // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(interpreter)); + PushTransactionRAII RAII(const_cast(interpreter)); printer.DisplayGlobals(); } @@ -1578,7 +1579,7 @@ void DisplayGlobal(llvm::raw_ostream& stream, const Interpreter* interpreter, GlobalsPrinter printer(stream, interpreter); // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(interpreter)); + PushTransactionRAII RAII(const_cast(interpreter)); printer.DisplayGlobal(name); } @@ -1589,7 +1590,7 @@ void DisplayTypedefs(llvm::raw_ostream &stream, const Interpreter *interpreter) TypedefPrinter printer(stream, interpreter); // Could trigger deserialization of decls. - Interpreter::PushTransactionRAII RAII(const_cast(interpreter)); + PushTransactionRAII RAII(const_cast(interpreter)); printer.DisplayTypedefs(); } diff --git a/interpreter/cling/test/Interfaces/transactionReuse.C b/interpreter/cling/test/Interfaces/transactionReuse.C index a05ac3d937e4a..1355550831eba 100644 --- a/interpreter/cling/test/Interfaces/transactionReuse.C +++ b/interpreter/cling/test/Interfaces/transactionReuse.C @@ -15,6 +15,7 @@ #include "cling/Interpreter/Interpreter.h" #include "cling/Interpreter/Transaction.h" +#include "cling/Interpreter/PushTransactionRAII.h" #include "clang/AST/Decl.h" @@ -25,7 +26,7 @@ using namespace cling; void generateNestedTransaction(int depth) { if (!depth) return; - cling::Interpreter::PushTransactionRAII RAIIT(gCling); + cling::PushTransactionRAII RAIIT(gCling); if (depth | 0x1) { // if odd char buff[100]; sprintf(buff, "int i%d;", depth); From caca1c44108fdaf7e330d67aab1df8fd1ebd6350 Mon Sep 17 00:00:00 2001 From: Axel Naumann Date: Wed, 31 Mar 2021 19:28:35 +0200 Subject: [PATCH 3/8] [cling] Order includes alphabetically (NFC). --- interpreter/cling/include/cling/Interpreter/Interpreter.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/interpreter/cling/include/cling/Interpreter/Interpreter.h b/interpreter/cling/include/cling/Interpreter/Interpreter.h index 0795a50f4f258..f931744e240db 100644 --- a/interpreter/cling/include/cling/Interpreter/Interpreter.h +++ b/interpreter/cling/include/cling/Interpreter/Interpreter.h @@ -46,13 +46,13 @@ namespace clang { class NamedDecl; class Parser; class Preprocessor; + class PresumedLoc; class QualType; class RecordDecl; class Sema; class SourceLocation; class SourceManager; class Type; - class PresumedLoc; } namespace cling { @@ -67,14 +67,14 @@ namespace cling { class ClangInternalState; class CompilationOptions; class DynamicLibraryManager; + class IncrementalCUDADeviceCompiler; class IncrementalExecutor; class IncrementalParser; class InterpreterCallbacks; class LookupHelper; - class Value; class PushTransactionRAII; class Transaction; - class IncrementalCUDADeviceCompiler; + class Value; ///\brief Class that implements the interpreter-like behavior. It manages the /// incremental compilation. From 8a19e559bef6738e9bbe209b92488cd2b73c4dc6 Mon Sep 17 00:00:00 2001 From: Axel Naumann Date: Wed, 31 Mar 2021 19:30:49 +0200 Subject: [PATCH 4/8] [cling] Transaction needs to know its RAII: This will allow the Intrepreter to prevent unload() on Transactions held by RAIIs. --- .../cling/include/cling/Interpreter/Transaction.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/interpreter/cling/include/cling/Interpreter/Transaction.h b/interpreter/cling/include/cling/Interpreter/Transaction.h index c5660a0afd9d3..8f3a527859083 100644 --- a/interpreter/cling/include/cling/Interpreter/Transaction.h +++ b/interpreter/cling/include/cling/Interpreter/Transaction.h @@ -40,6 +40,7 @@ namespace llvm { namespace cling { class IncrementalExecutor; + class PushTransactionRAII; class TransactionPool; ///\brief Contains information about the consumed input at once. @@ -135,6 +136,10 @@ namespace cling { /// Transaction* m_Parent; + ///\brief RAII owning this Transaction. + /// + const PushTransactionRAII* m_Scope = nullptr; + unsigned m_State : 3; unsigned m_IssuedDiags : 2; @@ -308,6 +313,15 @@ namespace cling { m_State = val; } + const PushTransactionRAII* getScope() const { + return m_Scope; + } + + void setScope(const PushTransactionRAII& scope) { + assert(!m_Scope && "Cannot change ownership of a transaction"); + m_Scope = &scope; + } + IssuedDiags getIssuedDiags() const { return static_cast(getTopmostParent()->m_IssuedDiags); } From 290a31c9d19eca35c70c63994f0c9d15a4b48445 Mon Sep 17 00:00:00 2001 From: Axel Naumann Date: Wed, 31 Mar 2021 19:31:34 +0200 Subject: [PATCH 5/8] [cling] Assert that no nested attaches to unloading Transaction. --- interpreter/cling/include/cling/Interpreter/Transaction.h | 7 +++++++ interpreter/cling/lib/Interpreter/Interpreter.cpp | 1 + interpreter/cling/lib/Interpreter/Transaction.cpp | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/interpreter/cling/include/cling/Interpreter/Transaction.h b/interpreter/cling/include/cling/Interpreter/Transaction.h index 8f3a527859083..7b9792716d4a8 100644 --- a/interpreter/cling/include/cling/Interpreter/Transaction.h +++ b/interpreter/cling/include/cling/Interpreter/Transaction.h @@ -144,6 +144,11 @@ namespace cling { unsigned m_IssuedDiags : 2; + ///\brief the Transaction is currently being unloaded. Currently, + /// used for ensuring system consistency when unloading transactions. + /// + unsigned m_Unloading : 1; + ///\brief Options controlling the transformers and code generator. /// CompilationOptions m_Opts; @@ -313,6 +318,8 @@ namespace cling { m_State = val; } + void setUnloading() { m_Unloading = 1; } + const PushTransactionRAII* getScope() const { return m_Scope; } diff --git a/interpreter/cling/lib/Interpreter/Interpreter.cpp b/interpreter/cling/lib/Interpreter/Interpreter.cpp index 3f41ab17cfe20..9285c9d062543 100644 --- a/interpreter/cling/lib/Interpreter/Interpreter.cpp +++ b/interpreter/cling/lib/Interpreter/Interpreter.cpp @@ -1449,6 +1449,7 @@ namespace cling { } void Interpreter::unload(Transaction& T) { + T.setUnloading(); // Clear any stored states that reference the llvm::Module. // Do it first in case auto Module = T.getModule(); diff --git a/interpreter/cling/lib/Interpreter/Transaction.cpp b/interpreter/cling/lib/Interpreter/Transaction.cpp index 151745e311db4..b355f420e6e32 100644 --- a/interpreter/cling/lib/Interpreter/Transaction.cpp +++ b/interpreter/cling/lib/Interpreter/Transaction.cpp @@ -39,6 +39,7 @@ namespace cling { m_NestedTransactions.reset(0); m_Parent = 0; m_State = kCollecting; + m_Unloading = 0; m_IssuedDiags = kNone; m_Opts = CompilationOptions(); m_DefinitionShadowNS = 0; @@ -94,6 +95,7 @@ namespace cling { } void Transaction::addNestedTransaction(Transaction* nested) { + assert(!m_Unloading && "Must not nest within unloading transaction"); // Create lazily the list if (!m_NestedTransactions) m_NestedTransactions.reset(new NestedTransactions()); @@ -357,6 +359,8 @@ namespace cling { } cling::log() << indent << " state: " << stateNames[getState()] << " decl groups, "; + if (m_Unloading) + cling::log() << "currently unloading, "; if (hasNestedTransactions()) cling::log() << m_NestedTransactions->size(); else From 86b567f43a567bb635bf13540811442a63be289a Mon Sep 17 00:00:00 2001 From: Axel Naumann Date: Tue, 30 Mar 2021 22:52:11 +0200 Subject: [PATCH 6/8] [cling] Prevent double delete / double TransactionPool-ing: When a Transaction is unloaded for which *also* a ScopedTransactionRAII is waiting, the latter will potentially access a deleted Transaction. It can be deleted due to the Pool being full. Even if it is not deleted (as is the case in https://github.com/root-project/root/issues/7657 ) it will get pushed into the Pool once by unload() and a second time by the RAII! The two options to track this case were full-blown ref counting or noting that a Transaction is attached to an RAII scope. If that is the case, freeing the Transaction must be skipped, as the RAII will take care of it. --- interpreter/cling/lib/Interpreter/IncrementalParser.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/interpreter/cling/lib/Interpreter/IncrementalParser.cpp b/interpreter/cling/lib/Interpreter/IncrementalParser.cpp index 40e033320fb74..f421cdb9230bd 100644 --- a/interpreter/cling/lib/Interpreter/IncrementalParser.cpp +++ b/interpreter/cling/lib/Interpreter/IncrementalParser.cpp @@ -711,7 +711,11 @@ namespace cling { } } - m_TransactionPool->releaseTransaction(&T); + if (!T.getScope()) { + // Do not unload a transaction for which an RAII is waiting to prevent + // double deletes (ROOT issue 7657). + m_TransactionPool->releaseTransaction(&T); + } } std::vector IncrementalParser::getAllTransactions() { From 553d4f4b72fbe679bbc0e56ccd5b486fdbda4870 Mon Sep 17 00:00:00 2001 From: Axel Naumann Date: Wed, 31 Mar 2021 11:18:19 +0200 Subject: [PATCH 7/8] [cling] Remove historical leftover in TransactionPool size (NFC). --- interpreter/cling/lib/Interpreter/TransactionPool.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/interpreter/cling/lib/Interpreter/TransactionPool.h b/interpreter/cling/lib/Interpreter/TransactionPool.h index 1242bac1eaacc..d2cb4d7489d3e 100644 --- a/interpreter/cling/lib/Interpreter/TransactionPool.h +++ b/interpreter/cling/lib/Interpreter/TransactionPool.h @@ -26,8 +26,7 @@ namespace cling { #else kDebugMode = 1, // Always use a new Transaction #endif - kTransactionsInBlock = 8, - kPoolSize = 2 * kTransactionsInBlock + kPoolSize = 16 }; // It is twice the size of the block because there might be easily around 8 From b9fa1a4395b501ed9784773b702e17864b05eae8 Mon Sep 17 00:00:00 2001 From: Axel Naumann Date: Wed, 31 Mar 2021 15:10:21 +0200 Subject: [PATCH 8/8] [cling] Assert released transaction is != last in pool: a cheap way to notice what went wrong in https://github.com/root-project/root/issues/7657. --- interpreter/cling/lib/Interpreter/TransactionPool.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/interpreter/cling/lib/Interpreter/TransactionPool.h b/interpreter/cling/lib/Interpreter/TransactionPool.h index d2cb4d7489d3e..5c3f0ade2f86e 100644 --- a/interpreter/cling/lib/Interpreter/TransactionPool.h +++ b/interpreter/cling/lib/Interpreter/TransactionPool.h @@ -58,6 +58,8 @@ namespace cling { // Transaction T must be from call to TransactionPool::takeTransaction // void releaseTransaction(Transaction* T, bool reuse = true) { + assert((m_Transactions.empty() || m_Transactions.back() != T) \ + && "Transaction already in pool"); if (reuse) { assert((T->getState() == Transaction::kCompleted || T->getState() == Transaction::kRolledBack)