Uploaded image for project: 'ROOT'
  1. ROOT
  2. ROOT-7245

when reading objects from a TTree via GetEntry(), some member pointers are not properly initialized/rewritten

    Details

    • Type: Bug
    • Status: Closed (View Workflow)
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 6.02/02, 6.02/04
    • Fix Version/s: None
    • Component/s: I/O
    • Labels:
      None
    • Environment:

      SLC6 gcc4.9.1

    • Development:

      Description

      Reported by Dmitrijus Bugelskis <Dmitrijus.Bugelskis@cern.ch>
      Hi all,

      when reading objects from a TTree via GetEntry(), some member pointers are not properly
      initialized/rewritten (CMSSW_7_5_X_2015-04-08-2300 ...).

      I've attached an example script to reproduce it. Run with root -q -b test.C
      The output should be:

      N: 5 address: 0x3d39770
      N: 0 address: (null)

      however, current behavior is:

      N: 5 address: 0x3d39770
      N: 0 address: 0x3d39770

      (the old pointer is not rewritten and/or set to null).

      I imagine the deserializer just ignores an empty array and does nothing.
      However, it should either treat nullptr as a special case and/or set the member to nullptr on empty arrays.
      Or, alternatively, we can label this "bug" as an undefined behavior and fix the TH1 instead.

      This affects TH1* classes because they have a protected member named fBuffer.
      In many instances, TH1 checks whether fBuffer != 0 and tries to read/write it,
      even if fBufferSize is set to zero. fBuffer was added in root6.

      Furthermore, if you Clone() the corrupted TH1 object, the fBuffer gets re-allocated with
      fBufferSize=0. Behavior of such zero-size allocation is undefined.
      Unfortunately, TH1 code assumes that fBuffer (iff fBuffer != nullptr) contains at least one element (first member stores the actual size).
      and reading/writing it causes an invalid read/write past the zero-size allocation.

      This causes many memory corruptions later on and recent segfaults in cmssw (harvesting step):
      (for example, https://github.com/cms-sw/cmssw/pull/8673#issuecomment-91045875)

      To fix, either or both:

      • fix deserialization code (I've tried to understand the root code, but ...)
      • make sure TH1 never checks for fBuffer != 0, but rather checks for fBufferSize != 0 instead.

        Attachments

          Activity

            People

            • Assignee:
              pcanal Philippe Canal
              Reporter:
              wmtan William Tanenbaum
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: