From 438955546e7cb66d33b55f360b3ca187eaa50a97 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Thu, 8 Oct 2026 15:20:51 -0500 Subject: [PATCH 1/8] [ntuple] Update test for v1 streamer field to realistically handle alignment. Move the (so far ununused) VersionedStreamerField in the right header and dictionary. Attach that type as the data type of the test RVersionStreamerField which has been updated to record the class and use its alignment and sizeof. Without this fix, the test asserting inside RNTuple due to the zero value alignment. --- tree/ntuple/test/CustomStruct.hxx | 4 --- tree/ntuple/test/CustomStructLinkDef.h | 1 - tree/ntuple/test/StreamerField.hxx | 4 +++ tree/ntuple/test/StreamerFieldLinkDef.h | 1 + tree/ntuple/test/rfield_streamer.cxx | 36 +++++++++++++++++++------ 5 files changed, 33 insertions(+), 13 deletions(-) diff --git a/tree/ntuple/test/CustomStruct.hxx b/tree/ntuple/test/CustomStruct.hxx index 0cde0ccb3e091..350305a28fc6c 100644 --- a/tree/ntuple/test/CustomStruct.hxx +++ b/tree/ntuple/test/CustomStruct.hxx @@ -497,8 +497,4 @@ struct AlignmentEnvelope { std::array fArr; }; -struct VersionedStreamerField { - ClassDefNV(VersionedStreamerField, 2); -}; - #endif diff --git a/tree/ntuple/test/CustomStructLinkDef.h b/tree/ntuple/test/CustomStructLinkDef.h index c8e6ec796cad9..4786ce21e2860 100644 --- a/tree/ntuple/test/CustomStructLinkDef.h +++ b/tree/ntuple/test/CustomStructLinkDef.h @@ -185,6 +185,5 @@ #pragma link C++ class AlignedAs+; #pragma link C++ class OverAligned+; #pragma link C++ class AlignmentEnvelope+; -#pragma link C++ class VersionedStreamerField+; #endif diff --git a/tree/ntuple/test/StreamerField.hxx b/tree/ntuple/test/StreamerField.hxx index cf5d29d4d26bc..dbf957d96cce4 100644 --- a/tree/ntuple/test/StreamerField.hxx +++ b/tree/ntuple/test/StreamerField.hxx @@ -101,4 +101,8 @@ struct TemperatureKelvin { float fValue; }; +struct VersionedStreamerField { + ClassDefNV(VersionedStreamerField, 2); +}; + #endif // ROOT_RNTuple_Test_StreamerField diff --git a/tree/ntuple/test/StreamerFieldLinkDef.h b/tree/ntuple/test/StreamerFieldLinkDef.h index 59010c53264e4..4d5b33de2e90e 100644 --- a/tree/ntuple/test/StreamerFieldLinkDef.h +++ b/tree/ntuple/test/StreamerFieldLinkDef.h @@ -26,5 +26,6 @@ #pragma link C++ options = rntupleStreamerMode(true) class TemperatureCelsius + ; #pragma link C++ options = rntupleStreamerMode(true) class TemperatureKelvin + ; +#pragma link C++ options = rntupleStreamerMode(true) class VersionedStreamerField + ; #endif diff --git a/tree/ntuple/test/rfield_streamer.cxx b/tree/ntuple/test/rfield_streamer.cxx index 1a331b5b195fb..41137cedad18e 100644 --- a/tree/ntuple/test/rfield_streamer.cxx +++ b/tree/ntuple/test/rfield_streamer.cxx @@ -412,10 +412,26 @@ namespace { /// Used to create on-disk streamer fields with different field versions class RVersionedStreamerField : public RFieldBase { +private: + class RVersionedStreamerFieldDeleter : public RDeleter { + private: + TClass *fClass = nullptr; + + public: + explicit RVersionedStreamerFieldDeleter(TClass *cl) : RDeleter(cl->GetClassAlignment()), fClass(cl) {} + void operator()(void *objPtr, bool dtorOnly) final + { + fClass->Destructor(objPtr, true /* dtorOnly */); + RDeleter::operator()(objPtr, dtorOnly); + } + }; + + TClass *fClass = nullptr; + protected: std::unique_ptr CloneImpl(std::string_view newName) const final { - return std::make_unique(newName, fCustomVersion); + return std::make_unique(newName, fClass->GetName(), fCustomVersion); } const RColumnRepresentations &GetColumnRepresentations() const final @@ -428,23 +444,27 @@ class RVersionedStreamerField : public RFieldBase { void GenerateColumns() final { GenerateColumnsImpl(); } void GenerateColumns(const ROOT::RNTupleDescriptor &) final {} - void ConstructValue(void *) const final {} + void ConstructValue(void *where) const final { fClass->New(where); } + std::unique_ptr GetDeleter() const final + { + return std::make_unique(fClass); + } std::size_t AppendImpl(const void *) final { return 0; } public: std::uint32_t fCustomVersion = 0; - RVersionedStreamerField(std::string_view name, std::uint32_t version) - : RFieldBase(name, "VersionedStreamerField", ROOT::ENTupleStructure::kStreamer, /*isSimple=*/false), - fCustomVersion(version) + RVersionedStreamerField(std::string_view name, std::string_view classname, std::uint32_t version) + : RFieldBase(name, classname, ROOT::ENTupleStructure::kStreamer, /*isSimple=*/false), + fClass(TClass::GetClass(std::string(classname).c_str())), fCustomVersion(version) { } std::uint32_t GetFieldVersion() const final { return fCustomVersion; } std::uint32_t GetTypeVersion() const final { return 137; } - std::size_t GetValueSize() const final { return 0; } - std::size_t GetAlignment() const final { return 0; } + std::size_t GetValueSize() const final { return fClass->GetClassSize(); } + std::size_t GetAlignment() const final { return fClass->GetClassAlignment(); } }; } // anonymous namespace @@ -455,7 +475,7 @@ TEST(RField, StreamerFieldVersion) FileRaii fileGuard("test_ntuple_rfield_streamer_version.root"); { auto model = RNTupleModel::Create(); - model->AddField(std::make_unique("f", version)); + model->AddField(std::make_unique("f", "VersionedStreamerField", version)); auto writer = RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath()); } auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath()); From 46df729cfa1c852404d3f0caddf743f2bf39fd6c Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Thu, 5 Feb 2026 19:28:27 +0100 Subject: [PATCH 2/8] io: Clarify byte count related limits --- io/io/src/TBufferFile.cxx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/io/io/src/TBufferFile.cxx b/io/io/src/TBufferFile.cxx index dd71715942c1d..d7987ee00e144 100644 --- a/io/io/src/TBufferFile.cxx +++ b/io/io/src/TBufferFile.cxx @@ -3159,7 +3159,8 @@ Version_t TBufferFile::ReadVersion(UInt_t *startpos, UInt_t *bcnt, const TClass // before reading object save start position auto full_startpos = fBufCur - fBuffer; *startpos = full_startpos <= kMaxCountPosition ? UInt_t(full_startpos) : kOverflowPosition; - fByteCountStack.push_back({(size_t)full_startpos, cl, nullptr}); + if (bcnt) + fByteCountStack.push_back({(size_t)full_startpos, cl, nullptr}); } // read byte count (older files don't have byte count) @@ -3196,7 +3197,8 @@ Version_t TBufferFile::ReadVersion(UInt_t *startpos, UInt_t *bcnt, const TClass // did not fit and thus we stored it in 'fByteCounts' instead. // Mark this case by setting startpos to kOverflowCount. *bcnt = kOverflowCount; - *startpos = kOverflowPosition; + if (startpos) + *startpos = kOverflowPosition; } } } @@ -3302,7 +3304,8 @@ Version_t TBufferFile::ReadVersionNoCheckSum(UInt_t *startpos, UInt_t *bcnt) auto full_startpos = fBufCur - fBuffer; *startpos = full_startpos < kMaxCountPosition ? UInt_t(full_startpos) : kOverflowPosition; // TODO: Extend ReadVersionNoCheckSum to take the class pointer. - fByteCountStack.push_back({(size_t)full_startpos, nullptr, nullptr}); + if (bcnt) + fByteCountStack.push_back({(size_t)full_startpos, nullptr, nullptr}); } // read byte count (older files don't have byte count) @@ -3338,7 +3341,8 @@ Version_t TBufferFile::ReadVersionNoCheckSum(UInt_t *startpos, UInt_t *bcnt) // did not fit and thus we stored it in 'fByteCounts' instead. // Mark this case by setting startpos to kOverflowCount. *bcnt = kOverflowCount; - *startpos = kOverflowPosition; + if (startpos) + *startpos = kOverflowPosition; } } } From baec0777454e99bda20b1dc38e7764bdeacc9162 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Tue, 7 Apr 2026 13:26:29 -0500 Subject: [PATCH 3/8] io: Improve ByteCountStack assert Support the case where the class information is passed by name rather than by TClass pointer (eg. the action ReadCollectionDouble32) --- io/io/src/TBufferFile.cxx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/io/io/src/TBufferFile.cxx b/io/io/src/TBufferFile.cxx index d7987ee00e144..d0b8d9bd0d002 100644 --- a/io/io/src/TBufferFile.cxx +++ b/io/io/src/TBufferFile.cxx @@ -440,7 +440,10 @@ Long64_t TBufferFile::CheckByteCount(ULong64_t startpos, ULong64_t bcnt, const T // The position is above 4GB but was cached using a 32 bit variable. startpos = fByteCountStack.back().locator; // See below - R__ASSERT((fByteCountStack.back().cl == nullptr || clss == fByteCountStack.back().cl) + R__ASSERT((fByteCountStack.back().cl == nullptr || + (clss == nullptr && (classname == nullptr || classname[0] == '\0')) || + (clss && clss == fByteCountStack.back().cl) || + (classname && strcmp(classname, fByteCountStack.back().cl->GetName()) == 0)) && "Class on the byte count position stack does not match the passed class"); } else { // This assert allows to reject cases that used to be valid (missing or From 7156bdb53f884dd54d65dcd1c44ec2cd93d442e2 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Tue, 7 Apr 2026 17:07:37 -0500 Subject: [PATCH 4/8] io: TBufferClass fix handling of large bytecount. In TBufferFile::ReadClass properly set the byte count returned to kOverflowCount when the byte count in the stream was not updated (was too large). --- io/io/src/TBufferFile.cxx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/io/io/src/TBufferFile.cxx b/io/io/src/TBufferFile.cxx index d0b8d9bd0d002..4fe776f2905cd 100644 --- a/io/io/src/TBufferFile.cxx +++ b/io/io/src/TBufferFile.cxx @@ -3018,6 +3018,14 @@ TClass *TBufferFile::ReadClass(const TClass *clReq, ULong64_t *objTag) // return bytecount in objTag if (objTag) { *objTag = (bcnt & ~kByteCountMask); + if (*objTag == 0) { + // The byte count was stored but is zero, this means the data + // did not fit and thus we stored it in 'fByteCounts' instead. + // Mark this case by setting startpos to kOverflowCount. + *objTag = kOverflowCount; + // We do not have access to the caller's "cntpos" and can not + // update to kOverflowPosition (unlike the same code in ReadVersion). + } if (cl) fByteCountStack.back().alt = cl; } From 0f7caefe348bb8b27664edec32ef37d8db51cea8 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Mon, 16 Feb 2026 12:58:57 -0600 Subject: [PATCH 5/8] io: Add more debugging for ByteCountStack --- io/io/src/TBufferFile.cxx | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/io/io/src/TBufferFile.cxx b/io/io/src/TBufferFile.cxx index 4fe776f2905cd..01b74a6993477 100644 --- a/io/io/src/TBufferFile.cxx +++ b/io/io/src/TBufferFile.cxx @@ -2661,6 +2661,8 @@ void TBufferFile::SkipObjectAny(Long64_t start, UInt_t count) /// real beginning of the object in memory. You will need to use a /// dynamic_cast later if you need to retrieve it. + + void *TBufferFile::ReadObjectAny(const TClass *clCast) { R__ASSERT(IsReading()); @@ -2668,6 +2670,20 @@ void *TBufferFile::ReadObjectAny(const TClass *clCast) // make sure fMap is initialized InitMap(); + struct CaptureAndCheck { + using ByteCountStack_t = TBufferFile::ByteCountStack_t; + size_t fCurrent; + ByteCountStack_t *fByteCountStack; + + CaptureAndCheck(ByteCountStack_t *stack) : fCurrent(stack->size()), fByteCountStack(stack) {} + ~CaptureAndCheck() { + if (fCurrent != fByteCountStack->size()) { + ::Fatal("CaptureAndCheck", "Byte count stack was not properly cleaned up. Current size is %zu but should be %zu", fByteCountStack->size(), fCurrent); + } + } + }; + CaptureAndCheck checker(&(this->fByteCountStack)); + // before reading object save start position ULong64_t startpos = static_cast(fBufCur-fBuffer); ULong64_t cntpos = startpos <= kMaxCountPosition ? startpos : kOverflowPosition; @@ -3862,6 +3878,7 @@ Int_t TBufferFile::ReadClassBuffer(const TClass *cl, void *pointer, const TClass else version = ReadVersion(&R__s, &R__c, cl); + size_t current = fByteCountStack.size(); Bool_t v2file = kFALSE; TFile *file = (TFile*)GetParent(); if (file && file->GetVersion() < 30000) { @@ -3880,6 +3897,7 @@ Int_t TBufferFile::ReadClassBuffer(const TClass *cl, void *pointer, const TClass Error("ReadClassBuffer", "Could not find the right streamer info to convert %s version %d into a %s, object skipped at offset %d", onFileClass->GetName(), version, cl->GetName(), Length() ); + --current; CheckByteCount(R__s, R__c, onFileClass); return 0; } @@ -3904,6 +3922,7 @@ Int_t TBufferFile::ReadClassBuffer(const TClass *cl, void *pointer, const TClass if (version < -1 || version >= infocapacity) { Error("ReadClassBuffer","class: %s, attempting to access a wrong version: %d, object skipped at offset %d", cl->GetName(), version, Length()); + --current; CheckByteCount(R__s, R__c, cl); return 0; } @@ -3962,11 +3981,13 @@ Int_t TBufferFile::ReadClassBuffer(const TClass *cl, void *pointer, const TClass // When the object was written the class was version zero, so // there is no StreamerInfo to be found. // Check that the buffer position corresponds to the byte count. + --current; CheckByteCount(R__s, R__c, cl); return 0; } else { Error( "ReadClassBuffer", "Could not find the StreamerInfo for version %d of the class %s, object skipped at offset %d", version, cl->GetName(), Length() ); + --current; CheckByteCount(R__s, R__c, cl); return 0; } @@ -3978,6 +3999,9 @@ Int_t TBufferFile::ReadClassBuffer(const TClass *cl, void *pointer, const TClass ApplySequence(*(sinfo->GetReadObjectWiseActions()), (char*)pointer ); if (sinfo->TStreamerInfo::IsRecovered()) R__c=0; // 'TStreamerInfo::' avoids going via a virtual function. + if (current != fByteCountStack.size()) + Fatal("ReadClassBuffer", "We are out of sync at level %lld vs %zu for %s", current, fByteCountStack.size(), cl->GetName()); + --current; // Check that the buffer position corresponds to the byte count. CheckByteCount(R__s, R__c, cl); @@ -4046,8 +4070,11 @@ Int_t TBufferFile::ApplySequence(const TStreamerInfoActions::TActionSequence &se for(TStreamerInfoActions::ActionContainer_t::const_iterator iter = sequence.fActions.begin(); iter != end; ++iter) { + auto current = fByteCountStack.size(); (*iter).PrintDebug(*this,obj); (*iter)(*this,obj); + if (current != fByteCountStack.size()) + Fatal("ApplySequence", "We are out of sync starting at level %lld vs %zu", current, fByteCountStack.size()); } } else { @@ -4056,7 +4083,10 @@ Int_t TBufferFile::ApplySequence(const TStreamerInfoActions::TActionSequence &se for(TStreamerInfoActions::ActionContainer_t::const_iterator iter = sequence.fActions.begin(); iter != end; ++iter) { + auto current = fByteCountStack.size(); (*iter)(*this,obj); + if (current != fByteCountStack.size()) + Fatal("ApplySequence", "We are out of sync starting at level %lld vs %zu", current, fByteCountStack.size()); } } From 02f35fe0186f74c25a5f88e12992cc992a195f72 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Thu, 9 Apr 2026 17:11:37 -0500 Subject: [PATCH 6/8] io: Improve ByteCountStack error message --- io/io/src/TBufferFile.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/io/io/src/TBufferFile.cxx b/io/io/src/TBufferFile.cxx index 01b74a6993477..d0e0047612ab5 100644 --- a/io/io/src/TBufferFile.cxx +++ b/io/io/src/TBufferFile.cxx @@ -4086,7 +4086,7 @@ Int_t TBufferFile::ApplySequence(const TStreamerInfoActions::TActionSequence &se auto current = fByteCountStack.size(); (*iter)(*this,obj); if (current != fByteCountStack.size()) - Fatal("ApplySequence", "We are out of sync starting at level %lld vs %zu", current, fByteCountStack.size()); + Fatal("ApplySequence", "ByteCountStack is out of sync starting at level %zu vs %zu", current, fByteCountStack.size()); } } From fa2f46ddd78f944dfce526d4f3b2e0ae5cc7acb5 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Thu, 23 Apr 2026 13:53:39 -0500 Subject: [PATCH 7/8] io: Add missing CheckByteCount in TConvertClonesArrayToProxy --- io/io/src/TContainerConverters.cxx | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/io/io/src/TContainerConverters.cxx b/io/io/src/TContainerConverters.cxx index 6958ad082e2fc..b74fe8b30f4b6 100644 --- a/io/io/src/TContainerConverters.cxx +++ b/io/io/src/TContainerConverters.cxx @@ -72,9 +72,8 @@ void TConvertClonesArrayToProxy::operator()(TBuffer &b, void *pmember, Int_t siz Int_t nobjects, dummy; char nch; TString s; - char classv[256]; - void *env; - UInt_t start, bytecount; + char classv[256]; + void *env; R__ASSERT(b.IsReading()); @@ -95,6 +94,9 @@ void TConvertClonesArrayToProxy::operator()(TBuffer &b, void *pmember, Int_t siz if (size==0) size=1; for(Int_t k=0; kCommit(env); b.CheckByteCount(start, bytecount,TClonesArray::Class()); + if (needAlloc) { + // Balance/check for the ReadClass byte count. + b.CheckByteCount(cntpos, tag, TClonesArray::Class()); + } } } From 7b9378f173d391b0111e5f7b8d7b530307c58d5b Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Thu, 9 Apr 2026 17:03:22 -0500 Subject: [PATCH 8/8] net: Keep TMessage to 32 bits (no real enforcement yet) --- net/net/src/TMessage.cxx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/net/net/src/TMessage.cxx b/net/net/src/TMessage.cxx index f36e8b296804c..4105aadcf9f41 100644 --- a/net/net/src/TMessage.cxx +++ b/net/net/src/TMessage.cxx @@ -392,10 +392,16 @@ Int_t TMessage::Compress() fCompPos = fBufCur; bufcur = fBufComp; + if (static_cast(CompLength()) > static_cast(std::numeric_limits::max())) { + Warning("Compress", "Compressed buffer length too large (%d)", CompLength()); + } tobuf(bufcur, (UInt_t)(CompLength() - sizeof(UInt_t))); Int_t what = fWhat | kMESS_ZIP; tobuf(bufcur, what); - tobuf(bufcur, Length()); // original uncompressed buffer length + if (static_cast(Length()) > static_cast(std::numeric_limits::max())) { + Warning("Compress", "Original buffer length too large (%zu)", Length()); + } + tobuf(bufcur, (UInt_t)Length()); // original uncompressed buffer length return 0; }