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

[UBSAN]runtime error applying non-zero offset 18446744073709551568 to null pointer #46908

Open
smuzaffar opened this issue Dec 10, 2024 · 15 comments

Comments

@smuzaffar
Copy link
Contributor

smuzaffar commented Dec 10, 2024

We have few runtime errors like [a] in UBSAN. This is triggered from https://github.com/cms-sw/cmssw/blob/master/DQM/SiStripMonitorCluster/src/SiStripMonitorCluster.cc#L847 when cluster_detset.end() is called. Could it be that there is no data https://github.com/cms-sw/cmssw/blob/master/DataFormats/Common/interface/DetSetNew.h#L57 and we return nullptr + m_size?

@dan131riley , does this method https://github.com/cms-sw/cmssw/blob/master/DataFormats/Common/interface/DetSetNew.h#L84-L88

    data_type const *data() const {
      if (isValid() || !empty())
        assert(m_data);
      return m_data ? (&((*m_data)[m_offset])) : nullptr;
    }

make any sense? shouldn't it be if (!isValid() || empty()) ?

[a] https://cmssdt.cern.ch/SDT/cgi-bin/logreader/el8_amd64_gcc12/CMSSW_15_0_UBSAN_X_2024-12-09-2300/pyRelValMatrixLogs/run/2024.003001_RunDisplacedJet2024B_10k/step3_RunDisplacedJet2024B_10k.log#/

/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:1143:34: runtime error: applying non-zero offset 18446744073709551568 to null pointer
    #0 0x14ca8a88a662 in std::vector<SiStripCluster, std::allocator<SiStripCluster> >::operator[](unsigned long) const /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:1143
    #1 0x14ca8a88a662 in edmNew::DetSet<SiStripCluster>::data() src/DataFormats/Common/interface/DetSetNew.h:92
    #2 0x14ca8a88a662 in edmNew::DetSet<SiStripCluster>::end() src/DataFormats/Common/interface/DetSetNew.h:53
    #3 0x14ca8a88a662 in SiStripMonitorCluster::analyze(edm::Event const&, edm::EventSetup const&) src/DQM/SiStripMonitorCluster/src/SiStripMonitorCluster.cc:847
    #4 0x14cbe84baa9f in edm::stream::EDProducerAdaptorBase::doEvent(edm::EventTransitionInfo const&, edm::ActivityRegistry*, edm::ModuleCallingContext const*) src/FWCore/Framework/src/stream/EDProducerAdaptorBase.cc:83

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 10, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

A new Issue was created by @smuzaffar.

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

cms-bot commands are listed here

@smuzaffar
Copy link
Contributor Author

ah my bad if (isValid() || !empty()) seems correct

@dan131riley
Copy link

ah my bad if (isValid() || !empty()) seems correct

correct, it just says that if we have an offset or a size, then m_data must not be nullptr.

@makortel
Copy link
Contributor

assign core

@cmsbuild
Copy link
Contributor

New categories assigned: core

@Dr15Jones,@makortel,@smuzaffar you have been requested to review this Pull request/Issue and eventually sign? Thanks

@makortel
Copy link
Contributor

This part of warning applying non-zero offset 18446744073709551568 to null pointer sounds like the default-constructed

inline DetSet() : m_id(0), m_data(nullptr), m_offset(-1), m_size(0) {}

edmNew::DetSet would have been used. Although the offset is 47 less than 2**64-1 (to which the -1 would convert to in std::vector::operator[]()).

@makortel
Copy link
Contributor

I'm not sure if this is really relevant for this issue, but I realized a const-correctness issue in with the edmNew::DetSetVector. I opened a new issue for that #46917

@makortel
Copy link
Contributor

By the way, from the trace

    #1 0x14ca8a88a662 in edmNew::DetSet<SiStripCluster>::data() src/DataFormats/Common/interface/DetSetNew.h:92

corresponds to the non-const overload of data()

data_type *data() {
assert(m_data);
return const_cast<data_type *>(&((*m_data)[m_offset]));
}

The assert() should have fired if the m_data would be nullptr. So it looks like the m_data would be valid, but the pointed-to std::vector<SiStripCluster> would be empty (assuming the internal pointers of empty std::vector would be nullptr).

I noticed sizeof(SiStripCluster) == 48, which could be related to the offset being 47 less than 2**64-1.

@makortel
Copy link
Contributor

Hmh, I tried to investigate locally in CMSSW_15_0_UBSAN_X_2024-12-09-2300, but runTheMatrix.py -l 2024.003001 --ibeos fails for me on DAS query with an error

No X509 proxy set. Exiting.

(even after voms-proxy-init)

@makortel
Copy link
Contributor

Hmh, I tried to investigate locally in CMSSW_15_0_UBSAN_X_2024-12-09-2300, but runTheMatrix.py -l 2024.003001 --ibeos fails for me on DAS query with an error

No X509 proxy set. Exiting.

(even after voms-proxy-init)

Apparently those workflows require X509_USER_PROXY environment variable to be set.

@makortel
Copy link
Contributor

I was able to reproduce the warning locally on 1 thread.

@makortel
Copy link
Contributor

The warning in SiStripMonitorCluster comes from module with a label HLTSiStripMonitorCluster, that uses the hltSiStripRawToClustersFacility data product as input, and it is read from the file. The hltSiStripRawToClustersFacility is the SiStripClusterizerFromRaw module, configured with onDemand = True (i.e. the edmNew::DetSetVector<SiStripCluster> is filled on demand). For this particular event, the edmNew::DetSetVector<SiStripCluster>::m_data is empty, which is exactly the situation I hypothesized in #46908 (comment).

@makortel
Copy link
Contributor

makortel commented Dec 13, 2024

Here is a unit-test-like reproducer of the warning (with DetSetNewTS_t.cpp nomenclature

void TestDetSet::fillParEmpty() {
  auto pg = std::make_shared<Getter>(this);
  DSTV detsets(pg, std::vector<unsigned>{21,22}, 2);
  CPPUNIT_ASSERT(detsets.size() == 2);

  auto idetset = detsets.begin();
  CPPUNIT_ASSERT(idetset != detsets.end());
  CPPUNIT_ASSERT(idetset->size() == 0);
  {
    auto idet = idetset->begin();     // <-- UBSAN warning comes from this line
    CPPUNIT_ASSERT(idet == idetset->end());
  }
}

and the corresponding warning

./cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02867/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/stl_vector.h:1143:34: runtime error: applying non-zero offset 18446744073709551600 to null pointer
    #0 0x4383af in std::vector<T, std::allocator<T> >::operator[](unsigned long) const /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02867/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/stl_vector.h:1143
    #1 0x432ac4 in edmNew::DetSet<T>::data() src/DataFormats/Common/interface/DetSetNew.h:92
    #2 0x42e18b in edmNew::DetSet<T>::begin() src/DataFormats/Common/interface/DetSetNew.h:51
    #3 0x418c19 in TestDetSet::fillParEmpty() src/DataFormats/Common/test/DetSetNewTS_t.cpp:349

@makortel
Copy link
Contributor

makortel commented Dec 13, 2024

The "serial filling" has also a similar problem, demonstrated with

void TestDetSet::fillSeqEmpty() {
  DSTV detsets(2);

  {
    FF ff(detsets, 21, true);
  }
  {
    FF ff(detsets, 22, true);
  }

  CPPUNIT_ASSERT(detsets.size() == 2);

  auto idetset = detsets.begin();
  CPPUNIT_ASSERT(idetset != detsets.end());
  CPPUNIT_ASSERT(idetset->size() == 0);
  {
    auto idet = idetset->begin();    // <-- UBSAN warning comes from this line
    CPPUNIT_ASSERT(idet == idetset->end());
  }
}

and warning

./cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02867/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/stl_vector.h:1143:34: runtime error: reference binding to null pointer of type 'const struct value_type'
    #0 0x4383de in std::vector<T, std::allocator<T> >::operator[](unsigned long) const /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02867/el8_amd64_gcc12/external/gcc/12.3.1-40d504be6370b5a30e3947a6e575ca28/include/c++/12.3.1/bits/stl_vector.h:1143
    #1 0x432ac4 in edmNew::DetSet<T>::data() src/DataFormats/Common/interface/DetSetNew.h:92
    #2 0x42e18b in edmNew::DetSet<T>::begin() src/DataFormats/Common/interface/DetSetNew.h:51
    #3 0x40f2b9 in TestDetSet::fillSeqEmpty() src/DataFormats/Common/test/DetSetNewTS_t.cpp:236

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

4 participants