[ROOT-6706] Array overflow in TH1::Rebin Created: 17/Sep/14  Updated: 30/Sep/14  Resolved: 17/Sep/14

Status: Closed
Project: ROOT
Component/s: Core Libraries
Affects Version/s: 5.34/21
Fix Version/s: None

Type: Bug Priority: High
Reporter: Salvatore Aiola Assignee: Lorenzo Moneta
Resolution: Fixed Votes: 0
Labels: None
Environment:

Any OS


Development:

 Description   

On line 5985 of TH1.cxx the index oldbin+i can overflow the array oldBins. This happens consistently when all the 3 following conditions are verified at the same time:

1) the newname argument is provided (namely a new histogram is created without overwriting the current one);

2) the xbins argument is provided and is != 0 (namely a new variable bin size histogram is created);

3) the new bins go beyond the upper edge of the last bin of the old histogram (e.g. the old histogram range is [0,100], the new histogram range is [0,150]).

In such cases the conditional 5981-5982 is false even when oldbin+i > nbins (nbins is the size of the oldBins array).
My proposed solution is to change line 5981 to if( (oldbin+i > nbins) ||.
In this way the overflow of the array is always avoided.



 Comments   
Comment by Lorenzo Moneta [ 17/Sep/14 ]

Hi,

I agree with your findings and your proposed solution. Thank you for reporting it. The only doubt I have is why do you want to rebin an histogram in a new extended range (e.g. from 0,100 to 0,150 ).
A potential problem can occur when you have overflows entries which are stored in the oldnbins+1. With your proposed changes the overflows will not be included in the rebind histogram, which I think is correct, but probably a warning message should also be given when overflows are present.

Best Regards

Lorenzo

Comment by Salvatore Aiola [ 17/Sep/14 ]

Hi Lorenzo,

thanks for looking into this issue. The need to expand the range of a histogram is certainly not common. However there are a few cases were this feature might come at hand, e.g. to do operations between histograms (add, divide) that require consistent binning.
I agree that a warning should be issued in case the overflow bin of the old histogram is not empty.

Best,
Salvatore

Comment by Lorenzo Moneta [ 17/Sep/14 ]

This is now fixed in both master and 5.34 patched versions of ROOT

Comment by Salvatore Aiola [ 17/Sep/14 ]

Ok, great. Note that root 6 is affected by the same bug.

Best,
Salvatore

Comment by Lorenzo Moneta [ 18/Sep/14 ]

Yes, I know, for this reason I have fixed it also in the master version for 6.02. We do not apply patches in the 6.00 version

Lorenzo

Generated at Sun Sep 22 01:44:21 CEST 2019 using Jira 7.13.1#713001-sha1:5e06076c2d215a6f699b7e5c90ab2fae7ba5a1ce.