Skip to content

v6-28 Backport cling fixes for ATLAS #14423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dpiparo merged 5 commits into from
Jan 24, 2024
Merged

v6-28 Backport cling fixes for ATLAS #14423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8764574
[core] Reduce symbol search only to when autoloading is enabled.
vgvassilev Jan 8, 2024
856131c
[core] Evaluate initializer of constant variables
vgvassilev Jan 8, 2024
ad093d2
[core] Prefer variable initializer over symbol search
vgvassilev Jan 8, 2024
ab29906
[core] Make some constants constexpr
vepadulano Jan 8, 2024
50e1e29
[core] Fix statements at global scope in TCling test
vepadulano Jan 16, 2024
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
2 changes: 1 addition & 1 deletion core/base/inc/TString.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ friend std::strong_ordering operator<=>(const TString &s1, const TString &s2) {
public:
enum EStripType { kLeading = 0x1, kTrailing = 0x2, kBoth = 0x3 };
enum ECaseCompare { kExact, kIgnoreCase };
static const Ssiz_t kNPOS = ::kNPOS;
static constexpr Ssiz_t kNPOS = ::kNPOS;

TString(); // Null string
explicit TString(Ssiz_t ic); // Suggested capacity
Expand Down
6 changes: 0 additions & 6 deletions core/base/src/TString.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,6 @@ as a TString, construct a TString from it, eg:
#include "TVirtualMutex.h"
#include "ThreadLocalStorage.h"

// Definition of the TString static data member. Declaration (even with
// initialization) in the class body *is not* definition according to C++
// standard. The definition must be explicitly done in one TU for ODR use. See
// https://en.cppreference.com/w/cpp/language/definition
const Ssiz_t TString::kNPOS;

#if defined(R__WIN32)
#define strtoull _strtoui64
#endif
Expand Down
38 changes: 19 additions & 19 deletions core/foundation/inc/RtypesCore.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,31 +97,31 @@ typedef float Size_t; //Attribute size (float)

//---- constants ---------------------------------------------------------------

const Bool_t kTRUE = true;
const Bool_t kFALSE = false;
constexpr Bool_t kTRUE = true;
constexpr Bool_t kFALSE = false;

const Int_t kMaxUChar = 256;
const Int_t kMaxChar = kMaxUChar >> 1;
const Int_t kMinChar = -kMaxChar - 1;
constexpr Int_t kMaxUChar = 256;
constexpr Int_t kMaxChar = kMaxUChar >> 1;
constexpr Int_t kMinChar = -kMaxChar - 1;

const Int_t kMaxUShort = 65534;
const Int_t kMaxShort = kMaxUShort >> 1;
const Int_t kMinShort = -kMaxShort - 1;
constexpr Int_t kMaxUShort = 65534;
constexpr Int_t kMaxShort = kMaxUShort >> 1;
constexpr Int_t kMinShort = -kMaxShort - 1;

const UInt_t kMaxUInt = UInt_t(~0);
const Int_t kMaxInt = Int_t(kMaxUInt >> 1);
const Int_t kMinInt = -kMaxInt - 1;
constexpr UInt_t kMaxUInt = UInt_t(~0);
constexpr Int_t kMaxInt = Int_t(kMaxUInt >> 1);
constexpr Int_t kMinInt = -kMaxInt - 1;

const ULong_t kMaxULong = ULong_t(~0);
const Long_t kMaxLong = Long_t(kMaxULong >> 1);
const Long_t kMinLong = -kMaxLong - 1;
constexpr ULong_t kMaxULong = ULong_t(~0);
constexpr Long_t kMaxLong = Long_t(kMaxULong >> 1);
constexpr Long_t kMinLong = -kMaxLong - 1;

const ULong64_t kMaxULong64 = ULong64_t(~0LL);
const Long64_t kMaxLong64 = Long64_t(kMaxULong64 >> 1);
const Long64_t kMinLong64 = -kMaxLong64 - 1;
constexpr ULong64_t kMaxULong64 = ULong64_t(~0LL);
constexpr Long64_t kMaxLong64 = Long64_t(kMaxULong64 >> 1);
constexpr Long64_t kMinLong64 = -kMaxLong64 - 1;

const ULong_t kBitsPerByte = 8;
const Ssiz_t kNPOS = ~(Ssiz_t)0;
constexpr ULong_t kBitsPerByte = 8;
constexpr Ssiz_t kNPOS = ~(Ssiz_t)0;

//---- debug global ------------------------------------------------------------

Expand Down
5 changes: 3 additions & 2 deletions core/metacling/src/TCling.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4737,6 +4737,9 @@ TInterpreter::DeclId_t TCling::GetDataMember(ClassInfo_t *opaque_cl, const char
DeclId_t d;
TClingClassInfo *cl = (TClingClassInfo*)opaque_cl;

// Could trigger deserialization of decls.
cling::Interpreter::PushTransactionRAII RAII(GetInterpreterImpl());

if (cl) {
d = cl->GetDataMember(name);
// We check if the decl of the data member has an annotation which indicates
Expand Down Expand Up @@ -4767,8 +4770,6 @@ TInterpreter::DeclId_t TCling::GetDataMember(ClassInfo_t *opaque_cl, const char
LookupResult R(SemaR, DName, SourceLocation(), Sema::LookupOrdinaryName,
Sema::ForExternalRedeclaration);

// Could trigger deserialization of decls.
cling::Interpreter::PushTransactionRAII RAII(GetInterpreterImpl());
cling::utils::Lookup::Named(&SemaR, R);

LookupResult::Filter F = R.makeFilter();
Expand Down
30 changes: 19 additions & 11 deletions core/metacling/src/TClingCallbacks.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,24 @@ extern "C" {
}

class AutoloadLibraryMU : public llvm::orc::MaterializationUnit {
const TClingCallbacks &fCallbacks;
std::string fLibrary;
llvm::orc::SymbolNameVector fSymbols;
public:
AutoloadLibraryMU(const std::string &Library, const llvm::orc::SymbolNameVector &Symbols)
: MaterializationUnit(getSymbolFlagsMap(Symbols), nullptr), fLibrary(Library), fSymbols(Symbols)
AutoloadLibraryMU(const TClingCallbacks &cb, const std::string &Library, const llvm::orc::SymbolNameVector &Symbols)
: MaterializationUnit(getSymbolFlagsMap(Symbols), nullptr), fCallbacks(cb), fLibrary(Library), fSymbols(Symbols)
{
}

StringRef getName() const override { return "<Symbols from Autoloaded Library>"; }

void materialize(std::unique_ptr<llvm::orc::MaterializationResponsibility> R) override
{
if (!fCallbacks.IsAutoLoadingEnabled()) {
R->failMaterialization();
return;
}

llvm::orc::SymbolMap loadedSymbols;
llvm::orc::SymbolNameSet failedSymbols;
bool loadedLibrary = false;
Expand Down Expand Up @@ -150,19 +158,22 @@ class AutoloadLibraryMU : public llvm::orc::MaterializationUnit {
map[symbolName] = llvm::JITSymbolFlags::Exported;
return map;
}

std::string fLibrary;
llvm::orc::SymbolNameVector fSymbols;
};

class AutoloadLibraryGenerator : public llvm::orc::DefinitionGenerator {
const TClingCallbacks &fCallbacks;
cling::Interpreter *fInterpreter;
public:
AutoloadLibraryGenerator(cling::Interpreter *interp) : fInterpreter(interp) {}
AutoloadLibraryGenerator(cling::Interpreter *interp, const TClingCallbacks& cb)
: fCallbacks(cb), fInterpreter(interp) {}

llvm::Error tryToGenerate(llvm::orc::LookupState &LS, llvm::orc::LookupKind K, llvm::orc::JITDylib &JD,
llvm::orc::JITDylibLookupFlags JDLookupFlags,
const llvm::orc::SymbolLookupSet &Symbols) override
{
if (!fCallbacks.IsAutoLoadingEnabled())
llvm::Error::success();

// If we get here, the symbols have not been found in the current process,
// so no need to check that again. Instead search for the library that
// provides the symbol and create one MaterializationUnit per library to
Expand Down Expand Up @@ -197,7 +208,7 @@ class AutoloadLibraryGenerator : public llvm::orc::DefinitionGenerator {
}

for (auto &&KV : found) {
auto MU = std::make_unique<AutoloadLibraryMU>(KV.first, std::move(KV.second));
auto MU = std::make_unique<AutoloadLibraryMU>(fCallbacks, KV.first, std::move(KV.second));
if (auto Err = JD.define(MU))
return Err;
}
Expand All @@ -207,9 +218,6 @@ class AutoloadLibraryGenerator : public llvm::orc::DefinitionGenerator {

return llvm::Error::success();
}

private:
cling::Interpreter *fInterpreter;
};

TClingCallbacks::TClingCallbacks(cling::Interpreter *interp, bool hasCodeGen) : InterpreterCallbacks(interp)
Expand All @@ -219,7 +227,7 @@ TClingCallbacks::TClingCallbacks(cling::Interpreter *interp, bool hasCodeGen) :
m_Interpreter->declare("namespace __ROOT_SpecialObjects{}", &T);
fROOTSpecialNamespace = dyn_cast<NamespaceDecl>(T->getFirstDecl().getSingleDecl());

interp->addGenerator(std::make_unique<AutoloadLibraryGenerator>(interp));
interp->addGenerator(std::make_unique<AutoloadLibraryGenerator>(interp, *this));
}
}

Expand Down
2 changes: 1 addition & 1 deletion core/metacling/src/TClingCallbacks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class TClingCallbacks : public cling::InterpreterCallbacks {
void Initialize();

void SetAutoLoadingEnabled(bool val = true) { fIsAutoLoading = val; }
bool IsAutoLoadingEnabled() { return fIsAutoLoading; }
bool IsAutoLoadingEnabled() const { return fIsAutoLoading; }

void SetAutoParsingSuspended(bool val = true) { fIsAutoParsingSuspended = val; }
bool IsAutoParsingSuspended() { return fIsAutoParsingSuspended; }
Expand Down
11 changes: 7 additions & 4 deletions core/metacling/src/TClingDataMemberInfo.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,10 +352,9 @@ Longptr_t TClingDataMemberInfo::Offset()
// static constexpr Long64_t something = std::numeric_limits<Long64_t>::max();
cling::Interpreter::PushTransactionRAII RAII(fInterp);

if (Longptr_t addr = reinterpret_cast<Longptr_t>(fInterp->getAddressOfGlobal(GlobalDecl(VD))))
return addr;
auto evalStmt = VD->ensureEvaluatedStmt();
if (evalStmt && evalStmt->Value) {
// We can't reassign constexpr or const variables. We can compute the
// initializer.
if (VD->hasInit() && (VD->isConstexpr() || VD->getType().isConstQualified())) {
if (const APValue* val = VD->evaluateValue()) {
if (VD->getType()->isIntegralType(C)) {
return reinterpret_cast<Longptr_t>(val->getInt().getRawData());
Expand Down Expand Up @@ -388,6 +387,10 @@ Longptr_t TClingDataMemberInfo::Offset()
} // not integral type
} // have an APValue
} // have an initializing value

// Try the slow operation.
if (Longptr_t addr = reinterpret_cast<Longptr_t>(fInterp->getAddressOfGlobal(GlobalDecl(VD))))
return addr;
}
// FIXME: We have to explicitly check for not enum constant because the
// implementation of getAddressOfGlobal relies on mangling the name and in
Expand Down
32 changes: 32 additions & 0 deletions core/metacling/test/TClingDataMemberInfoTests.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,3 +220,35 @@ class Outer {
auto *dmInnerProtected = (TDataMember*)TClass::GetClass("DMLookup::Outer")->GetListOfDataMembers()->FindObject("InnerProtected");
EXPECT_EQ(dmInnerProtected->Property(), kIsProtected | kIsClass);
}

TEST(TClingDataMemberInfo, Offset)
{
gInterpreter->ProcessLine("int ROOT_7459 = 42; ROOT_7459++;");
EXPECT_TRUE(43 == *(int*)gROOT->GetGlobal("ROOT_7459")->GetAddress());

gInterpreter->ProcessLine("constexpr int var1 = 1;");
EXPECT_TRUE(1 == *(int*)gROOT->GetGlobal("var1")->GetAddress());

gInterpreter->ProcessLine("static constexpr int var2 = -2;");
EXPECT_TRUE(-2 == *(int*)gROOT->GetGlobal("var2")->GetAddress());

gInterpreter->ProcessLine("const float var3 = 3.1;");
EXPECT_FLOAT_EQ(3.1, *(float*)gROOT->GetGlobal("var3")->GetAddress());

gInterpreter->ProcessLine("static const double var4 = 4.2;");
EXPECT_DOUBLE_EQ(4.2, *(double*)gROOT->GetGlobal("var4")->GetAddress());

// Make sure ROOT's Core constexpr constants work
EXPECT_EQ(3000, *(int*)gROOT->GetGlobal("kError")->GetAddress());

#ifdef R__USE_CXXMODULES
// gGeoManager is defined in the Geom libraries and we want to make sure we
// do not load it when autoloading is off. We can only test this in modules
// mode because gGeoManager is not part of the PCH and non-modular ROOT has
// header parsing and autoloading coupled leading to redundant load of
// libGeom at gROOT->GetGlobal time.
TGlobal *GeoManagerInfo = gROOT->GetGlobal("gGeoManager");
TInterpreter::SuspendAutoLoadingRAII autoloadOff(gInterpreter);
EXPECT_TRUE(-1L == (ptrdiff_t)GeoManagerInfo->GetAddress());
#endif // R__USE_CXXMODULES
}