KFbase is reading beyond array bounds

[Dr15Jones]

The issue from UBSAN is

23234.0/step2:L1Trigger/TrackFindingTMTT/src/KFbase.cc:837:49: runtime error: index 7 out of bounds for type 'bool [7]'

[Dr15Jones]

assign l1

[cmsbuild]

New categories assigned: l1

@epalencia,@rekovic,@cecilecaillol you have been requested to review this Pull request/Issue and eventually sign? Thanks

[cmsbuild]

A new Issue was created by @Dr15Jones Chris Jones.

@Dr15Jones, @perrotta, @dpiparo, @makortel, @smuzaffar, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[Dr15Jones]

I checked out the package and built it under debug and added asserts to test the following read

cmssw/L1Trigger/TrackFindingTMTT/src/KFbase.cc

Line 837 in 5920cbe

ambiguous = ambiguityMap[kfEtaReg][kfLayer];

I ran in the debugger and saw the failure. The traceback was

#0  0x00007ffff51d3387 in raise () from /lib64/libc.so.6
#1  0x00007ffff51d4a78 in abort () from /lib64/libc.so.6
#2  0x00007ffff51cc1a6 in __assert_fail_base () from /lib64/libc.so.6
#3  0x00007ffff51cc252 in __assert_fail () from /lib64/libc.so.6
#4  0x00007fffa58d6733 in tmtt::KFbase::kalmanAmbiguousLayer (this=0x7fff5ced2f00, iEtaReg=1, kfLayer=7)
    at /uscms_data/d2/cdj/build/temp/ubsan/kf/CMSSW_12_4_X_2022-04-25-1100/src/L1Trigger/TrackFindingTMTT/src/KFbase.cc:838
#5  0x00007fffa58d29d4 in tmtt::KFbase::doKF (this=0x7fff5ced2f00, l1track3D=..., stubs=..., tpa=0x0)
    at /uscms_data/d2/cdj/build/temp/ubsan/kf/CMSSW_12_4_X_2022-04-25-1100/src/L1Trigger/TrackFindingTMTT/src/KFbase.cc:245
#6  0x00007fffa58d1536 in tmtt::KFbase::fit (this=0x7fff5ced2f00, l1track3D=...) at /uscms_data/d2/cdj/build/temp/ubsan/kf/CMSSW_12_4_X_2022-04-25-1100/src/L1Trigger/TrackFindingTMTT/src/KFbase.cc:75
#7  0x00007fffa5a0ec6a in trklet::HybridFit::Fit(trklet::Tracklet*, std::vector<trklet::Stub const*, std::allocator<trklet::Stub const*> >&) ()
   from /cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_4_X_2022-04-25-1100/lib/slc7_amd64_gcc10/libL1TriggerTrackFindingTracklet.so
#8  0x00007fffa5a2888c in trklet::PurgeDuplicate::execute(std::vector<trklet::Track, std::allocator<trklet::Track> >&, unsigned int) ()
   from /cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_4_X_2022-04-25-1100/lib/slc7_amd64_gcc10/libL1TriggerTrackFindingTracklet.so
#9  0x00007fffa5a30c7a in trklet::Sector::executePD(std::vector<trklet::Track, std::allocator<trklet::Track> >&) ()
   from /cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_4_X_2022-04-25-1100/lib/slc7_amd64_gcc10/libL1TriggerTrackFindingTracklet.so
#10 0x00007fffa5a9ed8d in trklet::TrackletEventProcessor::event(trklet::SLHCEvent&) ()
   from /cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_4_X_2022-04-25-1100/lib/slc7_amd64_gcc10/libL1TriggerTrackFindingTracklet.so
#11 0x00007fffaf4a944f in L1FPGATrackProducer::produce(edm::Event&, edm::EventSetup const&) ()
   from /cvmfs/cms-ib.cern.ch/week0/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_4_X_2022-04-25-1100/lib/slc7_amd64_gcc10/pluginTrackFindingTrackletPlugins.so

In the call to tmtt::KFbase::doKF, the present information was gleaned for the line with the problem (line 245)

cmssw/L1Trigger/TrackFindingTMTT/src/KFbase.cc

Lines 239 to 248 in 5920cbe

	unsigned nSkippedDeadLayers = 0;
	unsigned nSkippedAmbiguousLayers = 0;
	while (kfDeadLayers.find(layer) != kfDeadLayers.end() && layerStubs[layer].empty()) {
	layer += 1;
	++nSkippedDeadLayers;
	}
	while (this->kalmanAmbiguousLayer(etaReg, layer) && layerStubs[layer].empty()) {
	layer += 1;
	++nSkippedAmbiguousLayers;
	}

(gdb) print nSkippedDeadLayers
$1 = 0
(gdb) print nSkippedAmbiguousLayers
$10 = 0

The other important state information about where it was in the loops are

(gdb) print nSkipped
$3 = 2
(gdb) print maxIterations
$7 = 6
(gdb) print iteration
$8 = 5

So it is on its last iteration check and has already skipped over two layers. It also means the call to

cmssw/L1Trigger/TrackFindingTMTT/src/KFbase.cc

Line 232 in c5448e1

unsigned int layer = the_state->nextLayer(); // Get KF layer where stubs to be searched for next

must have returned 7.

So the number of layers being allowed by the loop (which must be 8 since we start counting at 0?) is greater than what is expected in kalmanAmbiguousLayer.

[cecilecaillol]

Fixed in #37700

[cecilecaillol]

+l1

[cmsbuild]

This issue is fully signed and ready to be closed.