JeffersonLab / hcana

Hall C++ Analyzer
7 stars 118 forks source link

Out of bounds array index in THcScalerEvtHandler #427

Closed hansenjo closed 5 years ago

hansenjo commented 5 years ago

https://github.com/JeffersonLab/hcana/blob/b20e8b9b73ac698c053aa60e8702a1311f81fd83/src/THcScalerEvtHandler.cxx#L538

Valgrind reports and "invalid read of size 4" at this location (see below). Debugging reveals that the array index nscal is zero, causing the code to read scal_prev_read[-1]. In fact, in the test replay I am doing, run 4914 with the UTIL_KAONLT/scripts_Replay/replay_production_coin.C script, nscal is always zero at this point in the code.

Looks like a logic error.

As a temporary fix, I have changed that line to

if(nscal > 0 && scaldata < scal_prev_read[nscal-1]) {

but I doubt that that does what is intended here.

Partial output of

valgrind --suppressions=/apps/root/6.08.00/root/etc/valgrind-root.supp --leak-check=yes --track-origins=yes ./hcana -q -b .x UTIL_KAONLT/scripts_Replay/replay_production_coin.C(4914,10000)

544 errors in context 3 of 44:
==38116== Invalid read of size 4
==38116==    at 0x4FA1C8D: THcScalerEvtHandler::AnalyzeBuffer(unsigned int*, bool) (THcScalerEvtHandler.cxx:538)
==38116==    by 0x4F9F168: THcScalerEvtHandler::Analyze(THaEvData*) (THcScalerEvtHandler.cxx:234)
==38116==    by 0x53AE125: THaAnalyzer::MainAnalysis() (THaAnalyzer.cxx:1274)
==38116==    by 0x53AE984: THaAnalyzer::Process(THaRunBase*) (THaAnalyzer.cxx:1425)
==38116==  Address 0x2966cd9c is 4 bytes before a block of size 2,048 alloc'd
==38116==    at 0x4C2A1E3: operator new(unsigned long) (vg_replace_malloc.c:334)
==38116==    by 0x4FA84B1: __gnu_cxx::new_allocator<unsigned int>::allocate(unsigned long, void const*) (new_allocator.h:104)
==38116==    by 0x4FA7BCB: std::allocator_traits<std::allocator<unsigned int> >::allocate(std::allocator<unsigned int>&, unsigned long) (alloc_traits.h:360)
==38116==    by 0x4FA6A8F: std::_Vector_base<unsigned int, std::allocator<unsigned int> >::_M_allocate(unsigned long) (stl_vector.h:170)
==38116==    by 0x4FA6B8B: void std::vector<unsigned int, std::allocator<unsigned int> >::_M_emplace_back_aux<unsigned int>(unsigned int&&) (vector.tcc:412)
==38116==    by 0x4FA5C82: void std::vector<unsigned int, std::allocator<unsigned int> >::emplace_back<unsigned int>(unsigned int&&) (vector.tcc:101)
==38116==    by 0x4FA4EC3: std::vector<unsigned int, std::allocator<unsigned int> >::push_back(unsigned int&&) (stl_vector.h:932)
==38116==    by 0x4F9FF23: THcScalerEvtHandler::AnalyzeBuffer(unsigned int*, bool) (THcScalerEvtHandler.cxx:370)
==38116==    by 0x4F9F168: THcScalerEvtHandler::Analyze(THaEvData*) (THcScalerEvtHandler.cxx:234)
==38116==    by 0x53AE125: THaAnalyzer::MainAnalysis() (THaAnalyzer.cxx:1274)
==38116==    by 0x53AE984: THaAnalyzer::Process(THaRunBase*) (THaAnalyzer.cxx:1425)
hansenjo commented 5 years ago

Closed with commit de986a692df193ba4dbb7caca03ef82deb8894d3