Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
VinInn opened this issue May 17, 2024 · 14 comments
Closed

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

VinInn opened this issue May 17, 2024 · 14 comments

Comments

@VinInn
Copy link
Contributor

VinInn commented May 17, 2024

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
Copy link
Contributor

cmsbuild commented May 17, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

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
Copy link
Contributor Author

VinInn commented May 17, 2024

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.

@VinInn VinInn changed the title access to vector of size 0 in VirtualJetProducer reference to first element of a vector of size 0 in VirtualJetProducer May 17, 2024
@makortel
Copy link
Contributor

assign RecoJets/JetProducers

@cmsbuild
Copy link
Contributor

New categories assigned: reconstruction

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

@jfernan2
Copy link
Contributor

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

@jfernan2
Copy link
Contributor

type jetmet

@jfernan2
Copy link
Contributor

ping @laurenhay @michaelwassmer

@smuzaffar
Copy link
Contributor

smuzaffar commented Sep 13, 2024

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
Copy link
Contributor

@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
Copy link
Contributor

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

@smuzaffar
Copy link
Contributor

I have opened #46581 which should create a vector of size 1 for fjJets.size()==1

@jfernan2
Copy link
Contributor

jfernan2 commented Nov 3, 2024

+1
Fixed by #46581

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 3, 2024

This issue is fully signed and ready to be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants