From 0dafa24d2a72305e837e8f5d8dc9b0ee0063ab08 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Wed, 10 Oct 2018 13:14:34 -0500 Subject: [PATCH] Fix cling::Interpreter::compileFunction getting confused by trailing transactions. This fixes https://sft.its.cern.ch/jira/browse/ROOT-9672 by having cling::Interpreter::DeclareCFunction return the transaction containing the compiled code. With the previous code, cling::Interpreter::compileFunction will get confused by transaction created during the callbacks executed during the cling::IncrementalParser::commitTransaction of the main transaction. Reproducer: With a main composed of 'only': int main(int argc, char ** argv) { char const * class_string = (argc == 2) ? argv[1] : "std::vector"; auto const result [[gnu::unused]] = TClass::GetClass(class_string); return 0; } which is a representation of real use case (in a more complex setup) in ART. We were getting: Error in : Failed to compile ==== SOURCE BEGIN ==== #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wformat-security" __attribute__((used)) extern "C" void __cf_0(void* obj, int nargs, void** args, void* ret) { if (ret) { (*(TStreamerInfo**)ret) = new TStreamerInfo(); return; } else { new TStreamerInfo(); return; } } #pragma clang diagnostic pop ==== SOURCE END ==== Error in : Called with no wrapper, not implemented! Error in : The plugin handler for TVirtualStreamerInfo was found but failed to create the factory object! The reason is that during TClingCallFunc::make_wrapper, the call to cling::Interpreter::compileFunction ends with: if (const llvm::GlobalValue* GV = getLastTransaction()->getModule()->getNamedValue(name)) However in the 'broken' case, the getLastTransaction does not return the transaction for the code being compiled by DeclareCFunction but instead the one used/created at: #0 cling::IncrementalParser::endTransaction (this=0x4a2980, T=0x8c0fb0) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/interpreter/cling/lib/Interpreter/IncrementalParser.cpp:345 #1 0x00007fffeebc7899 in cling::Interpreter::PushTransactionRAII::pop (this=0x7fffffffcb00) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/interpreter/cling/lib/Interpreter/Interpreter.cpp:111 #2 0x00007fffeebc785e in cling::Interpreter::PushTransactionRAII::~PushTransactionRAII (this=0x7fffffffcb00, __in_chrg=) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/interpreter/cling/lib/Interpreter/Interpreter.cpp:106 #3 0x00007fffeebeb659 in cling::LookupHelper::findScope (this=0x4a9dd0, className=..., diagOnOff=cling::LookupHelper::NoDiagnostics, resultType=0x7fffffffcd08, instantiateTemplate=false) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/interpreter/cling/lib/Interpreter/LookupHelper.cpp:466 #4 0x00007fffeeabe0df in TCling::CheckClassInfo (this=0x4a0550, name=, autoload=, isClassOrNamespaceOnly=) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/metacling/src/TCling.cxx:3630 #5 0x00007ffff7c3040d in TClass::Init (this=this@entry=0xdafd20, name=name@entry=0x7ffff7cb7638 "TGlobal", cversion=cversion@entry=2, typeinfo=typeinfo@entry=0x7ffff7d8b6d8 , isa=isa@entry=0x477430, dfil=dfil@entry=0x7ffff7cb8cab "TGlobal.h", ifil=, dl=, il=, givenInfo=, silent=) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/meta/src/TClass.cxx:1431 #6 0x00007ffff7c3a1b8 in TClass::TClass (this=0xdafd20, name=0x7ffff7cb7638 "TGlobal", cversion=, info=..., isa=0x477430, dfil=0x7ffff7cb8cab "TGlobal.h", ifil=0x7ffff7cccf88 "/local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/meta/src/TGlobal.cxx", dl=27, il=25, silent=false) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/meta/src/TClass.cxx:1273 #7 0x00007ffff7c3a72a in ROOT::CreateClass (cname=0x7ffff7cb7638 "TGlobal", id=id@entry=2, info=..., isa=isa@entry=0x477430, dfil=dfil@entry=0x7ffff7cb8cab "TGlobal.h", ifil=ifil@entry=0x7ffff7cccf88 "/local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/meta/src/TGlobal.cxx", dl=27, il=25) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/meta/src/TClass.cxx:5607 #8 0x00007ffff7c4b552 in ROOT::Internal::TDefaultInitBehavior::CreateClass (il=25, dl=27, ifil=0x7ffff7cccf88 "/local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/meta/src/TGlobal.cxx", dfil=0x7ffff7cb8cab "TGlobal.h", isa=0x477430, info=..., id=2, cname=, this=0x7ffff7da7508 ) at /home/pcanal/root_builds/v6-14-00-patches/opt/include/Rtypes.h:176 #9 ROOT::TGenericClassInfo::GetClass (this=0x7ffff7dab660 ) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/meta/src/TGenericClassInfo.cxx:250 #10 0x00007ffff7b1a2d8 in TGlobal::Class () at /home/pcanal/root_builds/v6-14-00-patches/opt/core/base/G__Core.cxx:17156 #11 0x00007ffff7ac01de in TGlobal::IsA (this=0xee3bc0) at /home/pcanal/root_builds/v6-14-00-patches/opt/include/TGlobal.h:48 #12 TGlobal::CheckTObjectHashConsistency (this=0xee3bc0) at /home/pcanal/root_builds/v6-14-00-patches/opt/include/TGlobal.h:48 #13 0x00007ffff7be9dcd in TObject::CheckedHash (this=0xee3bc0) at /home/pcanal/root_builds/v6-14-00-patches/opt/include/TObject.h:314 #14 THashTable::GetCheckedHashValue (this=0xe65a20, obj=0xee3bc0) at /home/pcanal/root_builds/v6-14-00-patches/opt/include/THashTable.h:94 #15 THashTable::Add (this=0xe65a20, obj=0xee3bc0) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/cont/src/THashTable.cxx:96 #16 0x00007ffff7be6bf1 in THashList::AddLast (this=this@entry=0x5be690, obj=obj@entry=0xee3bc0) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/cont/src/THashList.cxx:100 #17 0x00007ffff7c4e0d1 in TListOfDataMembers::AddLast (this=0x5be690, obj=0xee3bc0) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/meta/src/TListOfDataMembers.cxx:103 #18 0x00007ffff7ab8785 in TList::Add (obj=0xee3bc0, this=0x5be690) at /home/pcanal/root_builds/v6-14-00-patches/opt/include/TList.h:87 #19 TROOT::GetListOfGlobals (this=0x7ffff7da7a60 , load=load@entry=false) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/base/src/TROOT.cxx:1767 #20 0x00007fffeeab1058 in TCling::HandleNewDecl (this=0x4a0550, DV=0xedf238, isDeserialized=isDeserialized@entry=true, modifiedTClasses=...) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/metacling/src/TCling.cxx:555 #21 0x00007fffeeabb785 in TCling::UpdateListsOnCommitted (this=0x4a0550, T=...) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/metacling/src/TCling.cxx:6115 #22 0x00007fffeebd0103 in cling::MultiplexInterpreterCallbacks::TransactionCommitted (this=0x57fe20, T=...) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/interpreter/cling/lib/Interpreter/MultiplexInterpreterCallbacks.h:76 #23 0x00007fffeed05d71 in cling::IncrementalParser::commitTransaction (this=0x4a2980, PRT=..., ClearDiagClient=true) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/interpreter/cling/lib/Interpreter/IncrementalParser.cpp:532 #24 0x00007fffeed06399 in cling::IncrementalParser::Compile (this=0x4a2980, input=..., Opts=...) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/interpreter/cling/lib/Interpreter/IncrementalParser.cpp:663 #25 0x00007fffeebcbc4e in cling::Interpreter::DeclareInternal (this=0x4a0f30, input=..., CO=..., T=0x7fffffffd680) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/interpreter/cling/lib/Interpreter/Interpreter.cpp:1195 #26 0x00007fffeebca8e8 in cling::Interpreter::declare (this=0x4a0f30, input=..., T=0x7fffffffd680) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/interpreter/cling/lib/Interpreter/Interpreter.cpp:823 #27 0x00007fffeebcb560 in cling::Interpreter::DeclareCFunction (this=0x4a0f30, name=..., code=..., withAccessControl=true) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/interpreter/cling/lib/Interpreter/Interpreter.cpp:1096 #28 0x00007fffeebcb862 in cling::Interpreter::compileFunction (this=0x4a0f30, name=..., code=..., ifUnique=false, withAccessControl=true) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/interpreter/cling/lib/Interpreter/Interpreter.cpp:1140 #29 0x00007fffeeafb83c in TClingCallFunc::compile_wrapper (withAccessControl=true, wrapper=..., wrapper_name=..., this=0xcf3c10) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/metacling/src/TClingCallFunc.cxx:270 #30 TClingCallFunc::make_wrapper (this=this@entry=0xcf3c10) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/metacling/src/TClingCallFunc.cxx:1096 #31 0x00007fffeeafbcb8 in TClingCallFunc::IFacePtr (this=this@entry=0xcf3c10) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/metacling/src/TClingCallFunc.cxx:2233 #32 0x00007fffeeafbe83 in TClingCallFunc::ExecT (address=0x0, this=0xcf3c10) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/metacling/src/TClingCallFunc.cxx:2045 #33 TClingCallFunc::ExecInt (this=0xcf3c10, address=0x0) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/metacling/src/TClingCallFunc.cxx:2065 #34 0x00007ffff7c56e8d in TMethodCall::Execute (this=0xd97710, object=, retLong=@0x7fffffffd958: 0) at /local2/pcanal/cint_working/rootcling/v6-14-00-patches/core/meta/src/TMethodCall.cxx:457 #35 0x0000000000401009 in TMethodCall::Execute(long&) () #36 0x00000000004010ea in long TPluginHandler::ExecPluginImpl<>() () #37 0x000000000040106d in long TPluginHandler::ExecPlugin<>(int) () #38 0x0000000000400e21 in mytest() () #39 0x0000000000400e92 in main () --- .../cling/include/cling/Interpreter/Interpreter.h | 4 +++- interpreter/cling/lib/Interpreter/Interpreter.cpp | 12 +++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/interpreter/cling/include/cling/Interpreter/Interpreter.h b/interpreter/cling/include/cling/Interpreter/Interpreter.h index f8a5d14ef75c7..668389928c519 100644 --- a/interpreter/cling/include/cling/Interpreter/Interpreter.h +++ b/interpreter/cling/include/cling/Interpreter/Interpreter.h @@ -282,9 +282,11 @@ namespace cling { ///\param[in] name - name of the function, used to find its Decl. ///\param[in] code - function definition, starting with 'extern "C"'. ///\param[in] withAccessControl - whether to enforce access restrictions. + ///\param[out] T - The cling::Transaction of the input const clang::FunctionDecl* DeclareCFunction(llvm::StringRef name, llvm::StringRef code, - bool withAccessControl); + bool withAccessControl, + Transaction*& T); ///\brief Initialize runtime and C/C++ level overrides /// diff --git a/interpreter/cling/lib/Interpreter/Interpreter.cpp b/interpreter/cling/lib/Interpreter/Interpreter.cpp index 5ebe4cbc5462a..4a3e2914a8758 100644 --- a/interpreter/cling/lib/Interpreter/Interpreter.cpp +++ b/interpreter/cling/lib/Interpreter/Interpreter.cpp @@ -1020,7 +1020,8 @@ namespace cling { const FunctionDecl* Interpreter::DeclareCFunction(StringRef name, StringRef code, - bool withAccessControl) { + bool withAccessControl, + Transaction*& T) { /* In CallFunc we currently always (intentionally and somewhat necessarily) always fully specify member function template, however this can lead to @@ -1109,7 +1110,7 @@ namespace cling { LangOptions& LO = const_cast(getCI()->getLangOpts()); bool savedAccessControl = LO.AccessControl; LO.AccessControl = withAccessControl; - cling::Transaction* T = 0; + T = nullptr; cling::Interpreter::CompilationResult CR = declare(code, &T); LO.AccessControl = savedAccessControl; @@ -1154,15 +1155,16 @@ namespace cling { } } - const FunctionDecl* FD = DeclareCFunction(name, code, withAccessControl); - if (!FD) + Transaction* T = nullptr; + const FunctionDecl* FD = DeclareCFunction(name, code, withAccessControl, T); + if (!FD || !T) return 0; // // Get the wrapper function pointer // from the ExecutionEngine (the JIT). // if (const llvm::GlobalValue* GV - = getLastTransaction()->getModule()->getNamedValue(name)) + = T->getModule()->getNamedValue(name)) return m_Executor->getPointerToGlobalFromJIT(*GV); return 0;