reference to first element of a vector of size 0 in VirtualJetProducer

[VinInn]

running UBSAN on HLT I observed this message

/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/stl_vector.h:1124:34: runtime error: reference binding to null pointer of type 'struct value_type'
    #0 0x7f6dadd1d408 in std::vector<std::pair<double, double>, std::allocator<std::pair<double, double> > >::operator[](unsigned long) /data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/stl_vector.h:1124
    #1 0x7f6dadd1d408 in void VirtualJetProducer::writeJets<reco::PFJet>(edm::Event&, edm::EventSetup const&) src/RecoJets/JetProducers/plugins/VirtualJetProducer.cc:692

looking to the code

685    if (!fjJets_.empty()) {
   686      // Distance between jet centers and overlap area -- for disk-based area calculation
   687      using RIJ = std::pair<double, double>;
   688      std::vector<RIJ> rijStorage(fjJets_.size() * (fjJets_.size() / 2));
   689      RIJ* rij[fjJets_.size()];
   690      unsigned int k = 0;
   691      for (unsigned int ijet = 0; ijet < fjJets_.size(); ++ijet) {
   692        rij[ijet] = &rijStorage[k];
   693        k += ijet;
   694      }
   695
   696      float etaJ[fjJets_.size()], phiJ[fjJets_.size()];

it seems to me that if fjJets_.size() == 1 the vector at line 688 will be empty .... btw why a vector and not just a trivial VLA as in line 696?

[cmsbuild]

cms-bot internal usage

[cmsbuild]

A new Issue was created by @VinInn.

@Dr15Jones, @smuzaffar, @sextonkennedy, @antoniovilela, @makortel, @rappoccio can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[VinInn]

most probably rij[ijet] = rijStorage.data()+k; will suffice and nullptr+0 is still nullptr;

Anyhow low priority, will never cause a crash nor even a valgrind error.

[makortel]

assign RecoJets/JetProducers

[cmsbuild]

New categories assigned: reconstruction

@jfernan2,@mandrenguyen you have been requested to review this Pull request/Issue and eventually sign? Thanks

[jfernan2]

Please @laurenhay @michaelwassmer as JetMET RECO contacts, try to apply a fix. Thanks

[jfernan2]

type jetmet

[jfernan2]

ping @laurenhay @michaelwassmer

[smuzaffar]

Latest UBSAN IBs shows [a]. As @VinInn mentioned above for fjJets_.size() ==1 the https://github.com/cms-sw/cmssw/blob/master/RecoJets/JetProducers/plugins/VirtualJetProducer.cc#L692 is trying to access wrong memory

gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/stl_vector.h:1124:34: runtime error: reference binding to null pointer of type 'struct value_type'

[a] https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/raw/el8_amd64_gcc12/CMSSW_14_2_UBSAN_X_2024-09-11-2300/pyRelValMatrixLogs/run/136.759_RunDoubleEG2016E/step3_RunDoubleEG2016E.log

    #0 0x14e30703dce6 in std::vector<std::pair<double, double>, std::allocator<std::pair<double, double> > >::operator[](unsigned long) /data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/stl_vector.h:1124
    #1 0x14e30703dce6 in void VirtualJetProducer::writeJets<reco::PFJet>(edm::Event&, edm::EventSetup const&) src/RecoJets/JetProducers/plugins/VirtualJetProducer.cc:692
    #2 0x14e3070571bd in VirtualJetProducer::produce(edm::Event&, edm::EventSetup const&) src/RecoJets/JetProducers/plugins/VirtualJetProducer.cc:435
    #3 0x14e306aa1a97 in FastjetJetProducer::produce(edm::Event&, edm::EventSetup const&) src/RecoJets/JetProducers/plugins/FastjetJetProducer.cc:179
    #4 0x14e44dcd7013 in edm::stream::EDProducerAdaptorBase::doEvent(edm::EventTransitionInfo const&, edm::ActivityRegistry*, edm::ModuleCallingContext const*) src/FWCore/Framework/src/stream/EDProducerAdaptorBase.cc:83

[smuzaffar]

@cms-sw/reconstruction-l2 , does this https://github.com/cms-sw/cmssw/blob/master/RecoJets/JetProducers/plugins/VirtualJetProducer.cc#L685-L784 blockof code makes any sense for fjJets==1 ? If not then may be we can change if (!fjJets_.empty()) { to if (fjJets_.size() > 1) { otherwise we can update https://github.com/cms-sw/cmssw/blob/master/RecoJets/JetProducers/plugins/VirtualJetProducer.cc#L688C5-L688C72 to have

std::vector<RIJ> rijStorage(fjJets_.size() * (fjJets_.size()+1 / 2));

OR to avoid divide by 2

std::vector<RIJ> rijStorage(fjJets_.size() * ((fjJets_.size()+1)>>1));

this will make sure that rijStorage is not of zero size

[jfernan2]

@laurenhay as Jet RECO contact, can you please comment?

[smuzaffar]

I have opened https://github.com/cms-sw/cmssw/pull/46581 which should create a vector of size 1 for fjJets.size()==1

[jfernan2]

+1 Fixed by https://github.com/cms-sw/cmssw/pull/46581

[cmsbuild]

This issue is fully signed and ready to be closed.