-
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:
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.