[ROOT-7245] when reading objects from a TTree via GetEntry(), some member pointers are not properly initialized/rewritten Created: 11/Apr/15  Updated: 21/Apr/15  Resolved: 21/Apr/15

Status: Closed
Project: ROOT
Component/s: I/O
Affects Version/s: 6.02/02, 6.02/04
Fix Version/s: None

Type: Bug Priority: Critical
Reporter: William Tanenbaum Assignee: Philippe Canal
Resolution: Fixed Votes: 0
Labels: None
Environment:

SLC6 gcc4.9.1


Attachments: File test.C    
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.


 Comments   
Comment by William Tanenbaum [ 18/Apr/15 ]

This bug has the highest priority for CMS. Please investigate soon.

Comment by Philippe Canal [ 21/Apr/15 ]

Hi Bill,

This problem had already been fixed in the master and v5.34 patch branch. I pushed the commit (292ab3f7c74a4d88d59a3f1019701e444c847246) onto the v6.02 branch also.

Cheers,
Philippe

Generated at Wed Sep 18 15:32:22 CEST 2019 using Jira 7.13.1#713001-sha1:5e06076c2d215a6f699b7e5c90ab2fae7ba5a1ce.