search

JeffersonLab / hcana

Hall C++ Analyzer
7 stars 118 forks source link

Uninitialized data in matrix multiplication in THcDC #428

Closed hansenjo closed 3 years ago

hansenjo commented 5 years ago

https://github.com/JeffersonLab/hcana/blob/b20e8b9b73ac698c053aa60e8702a1311f81fd83/src/THcDC.cxx#L1011

Valgrind reports that some operations in the above line refer to uninitialized memory and hence will likely produce garbage results. It might the matrix AA or the vector TT, which may not even be uninitialized per se, but be initialized with values which come from function calls that retrieve uninitialized data. It's not obvious, and I haven't been able to track this down yet. It is unlikely to be false positive though, based on my experience with valgrind.

Partial output of replaying 10000 events (note: 2.8 such errors per event):

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)

28240 errors in context 4 of 44:
==38116== Conditional jump or move depends on uninitialised value(s)
==38116==    at 0x6186055: TObject::operator=(TObject const&) (TObject.cxx:107)
==38116==    by 0x83E38BD: TVectorT<double>::operator=(TVectorT<double> const&) (TVectorT.cxx:686)
==38116==    by 0x83E398B: TVectorT<double>::TVectorT(TVectorT<double> const&) (TVectorT.cxx:217)
==38116==    by 0x83E3B2F: TVectorT<double> operator*<double>(TMatrixT<double> const&, TVectorT<double> const&) (TVectorT.cxx:1436)
==38116==    by 0x4F4E082: THcDC::TrackFit() (THcDC.cxx:1011)
==38116==    by 0x4F4BBDC: THcDC::CoarseTrack(TClonesArray&) (THcDC.cxx:604)
==38116==    by 0x540281D: THaSpectrometer::CoarseTrack() (THaSpectrometer.cxx:290)
==38116==    by 0x53AD225: THaAnalyzer::PhysicsAnalysis(int) (THaAnalyzer.cxx:1098)
==38116==    by 0x53AE190: THaAnalyzer::MainAnalysis() (THaAnalyzer.cxx:1281)
==38116==    by 0x53AE984: THaAnalyzer::Process(THaRunBase*) (THaAnalyzer.cxx:1425)
==38116==  Uninitialised value was created by a stack allocation
==38116==    at 0x4F4D853: THcDC::TrackFit() (THcDC.cxx:926)
hansenjo commented 5 years ago

This is probably a false positive. See comments at TObject.cxx:84 where it is observed that Valgrind reports uninitialized memory for objects created on the stack. Indeed, the matrix AA and the vector TT that are being multiplied at THcDC.cxx:1011 are both stack objects. While the ROOT suppressions file squelches the report for the TObject copy constructor, it does not apparently do so for the TObject::operator=.

sawjlab commented 3 years ago

Seems like this issue should be closed.