Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions core/metacling/src/TCling.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -3191,6 +3191,30 @@ void TCling::ClearStack()
// No-op for cling due to cling::Value.
}

namespace {
// Re-enable JIT symbol library autoloading (independently of class/dictionary
// autoloading) for the lifetime of the object. See its use in TCling::Declare
// and #16601.
class EnableAutoLoadingForJITSymbolsRAII {
TClingCallbacks *fCallbacks;
bool fOldValue = false;

public:
EnableAutoLoadingForJITSymbolsRAII(TClingCallbacks *callbacks) : fCallbacks(callbacks)
{
if (fCallbacks) {
fOldValue = fCallbacks->IsAutoLoadingForJITSymbols();
fCallbacks->SetAutoLoadingForJITSymbols(true);
}
}
~EnableAutoLoadingForJITSymbolsRAII()
{
if (fCallbacks)
fCallbacks->SetAutoLoadingForJITSymbols(fOldValue);
}
};
}

////////////////////////////////////////////////////////////////////////////////
/// Declare code to the interpreter, without any of the interpreter actions
/// that could trigger a re-interpretation of the code. I.e. make cling
Expand All @@ -3206,6 +3230,12 @@ bool TCling::Declare(const char* code)
SuspendAutoLoadingRAII autoLoadOff(this);
Comment thread
devajithvs marked this conversation as resolved.
SuspendAutoParsing autoParseRaii(this);

// The suspensions above make parsing behave like a plain compiler, but they also
// gate the JIT's library autoloading - and a declared global's static initializer
// may still need symbols from a not-yet-loaded library (e.g. TVectorT<float> from
// libMatrix). Re-allow autoloading for such genuine JIT symbol resolution. #16601
EnableAutoLoadingForJITSymbolsRAII autoLoadJITOn(fClingCallbacks);

bool oldDynLookup = fInterpreter->isDynamicLookupEnabled();
fInterpreter->enableDynamicLookup(false);
bool oldRawInput = fInterpreter->isRawInputEnabled();
Expand Down
9 changes: 8 additions & 1 deletion core/metacling/src/TClingCallbacks.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,14 @@ class AutoloadLibraryGenerator : public llvm::orc::DefinitionGenerator {
llvm::orc::JITDylibLookupFlags JDLookupFlags,
const llvm::orc::SymbolLookupSet &Symbols) override
{
if (!fCallbacks.IsAutoLoadingEnabled())
// The IsAutoLoadingEnabled() gate keeps speculative lookups (e.g. querying
// the address of a global that is not loaded) from triggering a costly scan
// of all libraries. But that gate is also flipped off by the class-autoloading
// suspension in TCling::Declare, which merely wants parsing to behave like a
// plain compiler - it must not stop us from autoloading a library whose
// symbols the emitted static-initializer code genuinely needs to run. Declare
// signals that case via IsAutoLoadingForJITSymbols(). See #16601.
if (!fCallbacks.IsAutoLoadingEnabled() && !fCallbacks.IsAutoLoadingForJITSymbols())
return llvm::Error::success();

// If we get here, the symbols have not been found in the current process,
Expand Down
7 changes: 7 additions & 0 deletions core/metacling/src/TClingCallbacks.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class TClingCallbacks : public cling::InterpreterCallbacks {
bool fIsAutoLoading = false;
bool fIsAutoLoadingRecursively = false;
bool fIsAutoParsingSuspended = false;
bool fIsAutoLoadingForJITSymbols = false;
bool fIsCodeGening = false;
bool fIsLoadingModule = false;
llvm::DenseMap<llvm::StringRef, clang::DeclarationName> m_LoadedModuleFiles;
Expand All @@ -63,6 +64,12 @@ class TClingCallbacks : public cling::InterpreterCallbacks {
void SetAutoParsingSuspended(bool val = true) { fIsAutoParsingSuspended = val; }
bool IsAutoParsingSuspended() { return fIsAutoParsingSuspended; }

// Whether the JIT is allowed to autoload a library to resolve a symbol it
// genuinely needs to link and run emitted code, even while class/dictionary
// autoloading is suspended (as TCling::Declare does while parsing). See #16601.
void SetAutoLoadingForJITSymbols(bool val = true) { fIsAutoLoadingForJITSymbols = val; }
bool IsAutoLoadingForJITSymbols() const { return fIsAutoLoadingForJITSymbols; }

bool LibraryLoadingFailed(const std::string &, const std::string &, bool, bool) override;

void InclusionDirective(clang::SourceLocation /*HashLoc*/, const clang::Token & /*IncludeTok*/,
Expand Down
15 changes: 15 additions & 0 deletions core/metacling/test/TClingTests.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,21 @@ TEST_F(TClingTests, UndeclaredIdentifierCrash)
gInterpreter->ProcessLine("for(i=0; i < 0;); // the second usage of `i` was enough to get a segfault");
}

// https://github.com/root-project/root/issues/16601
// Declaring a global whose static initializer needs symbols from a library that
// is not loaded yet (here libMatrix, via TVectorT) must still autoload that
// library: TInterpreter::Declare suspends *class* autoloading while parsing, but
// this must not prevent the JIT from materializing symbols that the emitted
// static-initializer code genuinely requires to run.
TEST_F(TClingTests, DeclareAutoloadsSymbolsForStaticInit)
{
ASSERT_FALSE(gInterpreter->IsLoaded("libMatrix"));
EXPECT_TRUE(gInterpreter->Declare("#include <TVectorT.h>\n"
"const auto gROOT16601vec = TVectorT<float>(3);"));
// The variable must exist and be usable, i.e. its constructor ran.
EXPECT_EQ(3L, gInterpreter->ProcessLine("gROOT16601vec.GetNrows();"));
}

// https://github.com/root-project/root/issues/15818
#if !defined(_MSC_VER) || defined(R__ENABLE_BROKEN_WIN_TESTS)
TEST_F(TClingTests, VeryLongExpression)
Expand Down
Loading