[Arlesienne] Clean up for large bytecount implementation.

io/io/src/TBufferFile.cxx

@@ -440,7 +440,10 @@
       // 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
@@ -2658,13 +2661,29 @@
 /// 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());
 
    // 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<ULong64_t>(fBufCur-fBuffer);
    ULong64_t cntpos = startpos <= kMaxCountPosition ? startpos : kOverflowPosition;
@@ -3015,6 +3034,14 @@
    // 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;
    }
@@ -3159,7 +3186,8 @@
       // 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 +3224,8 @@
             // 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 +3331,8 @@
       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 +3368,8 @@
             // 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;
          }
       }
    }
@@ -3847,6 +3878,7 @@
    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) {
@@ -3865,6 +3897,7 @@
          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;
       }
@@ -3889,6 +3922,7 @@
                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;
                }
@@ -3947,11 +3981,13 @@
                // 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;
             }
@@ -3963,6 +3999,9 @@
    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);
 
@@ -4031,8 +4070,11 @@
       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 {
@@ -4041,7 +4083,10 @@
       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", "ByteCountStack is out of sync starting at level %zu vs %zu", current, fByteCountStack.size());
       }
    }
 

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; k<size; ++k, addr += fOffset ) {
 
+      ULong64_t tag    = 0; // either tag or byte count, set when 'needAlloc' is true
+      UInt_t    cntpos = 0; // position where to store byte count, set when 'needAlloc' is true
+
       if (needAlloc) {
          // Read the class name.
 
@@ -104,8 +106,9 @@ void TConvertClonesArrayToProxy::operator()(TBuffer &b, void *pmember, Int_t siz
          // before reading object save start position
          ULong64_t startpos = b.Length();
 
+         cntpos = startpos; // <= kMaxCountPosition ? startpos : kOverflowPosition;
+
          // attempt to load next object as TClass clCast
-         ULong64_t tag;       // either tag or byte count
          TClass *clRef = b.ReadClass(TClonesArray::Class(), &tag);
 
          if (clRef==0) {
@@ -175,7 +178,8 @@ void TConvertClonesArrayToProxy::operator()(TBuffer &b, void *pmember, Int_t siz
       if (fIsPointer) obj = *(void**)addr;
       else obj = addr;
 
-      TObject objdummy;
+      TObject   objdummy;
+      UInt_t    start, bytecount;
       Version_t v = b.ReadVersion(&start, &bytecount);
 
       //if (v == 3) {
@@ -197,6 +201,10 @@ void TConvertClonesArrayToProxy::operator()(TBuffer &b, void *pmember, Int_t siz
       if (!cl) {
          printf("TClonesArray::Streamer expecting class %s\n", classv);
          b.CheckByteCount(start, bytecount, TClonesArray::Class());
+         if (needAlloc) {
+            // Balance/check for the ReadClass byte count.
+            b.CheckByteCount(cntpos, tag, TClonesArray::Class());
+         }
          return;
       }
 
@@ -224,6 +232,10 @@ void TConvertClonesArrayToProxy::operator()(TBuffer &b, void *pmember, Int_t siz
       }
       proxy->Commit(env);
       b.CheckByteCount(start, bytecount,TClonesArray::Class());
+      if (needAlloc) {
+         // Balance/check for the ReadClass byte count.
+         b.CheckByteCount(cntpos, tag, TClonesArray::Class());
+      }
    }
 }
 

net/net/src/TMessage.cxx

@@ -392,10 +392,16 @@
    fCompPos    = fBufCur;
 
    bufcur = fBufComp;
+   if (static_cast<Long64_t>(CompLength()) > static_cast<Long64_t>(std::numeric_limits<UInt_t>::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<Long64_t>(Length()) >  static_cast<Long64_t>(std::numeric_limits<UInt_t>::max())) {
+      Warning("Compress", "Original buffer length too large (%zu)", Length());
+   }
+   tobuf(bufcur, (UInt_t)Length());    // original uncompressed buffer length
 
    return 0;
 }

tree/ntuple/test/CustomStruct.hxx

@@ -497,8 +497,4 @@ struct AlignmentEnvelope {
    std::array<OverAligned, 2> fArr;
 };
 
-struct VersionedStreamerField {
-   ClassDefNV(VersionedStreamerField, 2);
-};
-
 #endif

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

tree/ntuple/test/StreamerField.hxx

@@ -101,4 +101,8 @@ struct TemperatureKelvin {
    float fValue;
 };
 
+struct VersionedStreamerField {
+   ClassDefNV(VersionedStreamerField, 2);
+};
+
 #endif // ROOT_RNTuple_Test_StreamerField

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

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<RFieldBase> CloneImpl(std::string_view newName) const final
    {
-      return std::make_unique<RVersionedStreamerField>(newName, fCustomVersion);
+      return std::make_unique<RVersionedStreamerField>(newName, fClass->GetName(), fCustomVersion);
    }
 
    const RColumnRepresentations &GetColumnRepresentations() const final
@@ -428,23 +444,27 @@ class RVersionedStreamerField : public RFieldBase {
    void GenerateColumns() final { GenerateColumnsImpl<ROOT::Internal::RColumnIndex, std::byte>(); }
    void GenerateColumns(const ROOT::RNTupleDescriptor &) final {}
 
-   void ConstructValue(void *) const final {}
+   void ConstructValue(void *where) const final { fClass->New(where); }
+   std::unique_ptr<RDeleter> GetDeleter() const final
+   {
+      return std::make_unique<RVersionedStreamerFieldDeleter>(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<RVersionedStreamerField>("f", version));
+         model->AddField(std::make_unique<RVersionedStreamerField>("f", "VersionedStreamerField", version));
          auto writer = RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath());
       }
       auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath());