davidlt
commented
8 years ago @ggovi
Closed davidlt closed 5 years ago
davidlt
commented
8 years ago @ggovi
davidlt
commented
8 years ago Just code review, but this looks a bit suspicious: https://github.com/cms-sw/cmssw/blob/9087f9b34c86c57c3b3bef7e263468cb7e8fc1f4/CondFormats/SiStripObjects/interface/SiStripApvGain.h#L77 https://github.com/cms-sw/cmssw/blob/9087f9b34c86c57c3b3bef7e263468cb7e8fc1f4/CondFormats/SiStripObjects/src/SiStripApvGain.cc#L29
Especially return SiStripApvGain::Range(v_gains.end(),v_gains.end());
end() already point to the element after the last element in vector. Thus doing anything like *(range.first+apv) is strictly no-go.
One would need to go into debugger to verify if this is happening.
davidlt
commented
8 years ago Few snippets from debug output.
Good one:
DEBUG_ACCESS: 0x7fb36fd468b8
DEBUG_APV: 1
DEBUG_ATTEMPT: 0x7fb36fd468bc
GET_GAIN: 1.12618Bad one:
RANGE_EMPTY: 0x7fb36fd86470 // v_gains.end()
DEBUG_ACCESS: 0x7fb36fd86470 // range.first
DEBUG_APV: 5 // apv
DEBUG_ATTEMPT: 0x7fb36fd86484 // range.first+apv
GET_GAIN: 0Basically SiStripApvGain::Range(v_gains.end(),v_gains.end()) was returned, which is an empty range. apv is 5, so we go off bounds by 4 x 5 = 20 bytes. Then we tends to get gain as 0 or 1.
gain was 0 was 32,500 times, while 1 1964 times out of 177,248 returns.
Patch I used:
diff --git a/CalibFormats/SiStripObjects/src/SiStripGain.cc b/CalibFormats/SiStripObjects/src/SiStripGain.cc
index 9115499..b067e63 100644
--- a/CalibFormats/SiStripObjects/src/SiStripGain.cc
+++ b/CalibFormats/SiStripObjects/src/SiStripGain.cc
@@ -99,6 +99,7 @@ float SiStripGain::getStripGain(const uint16_t& strip, const SiStripApvGain::Ran
float SiStripGain::getApvGain(const uint16_t& apv, const SiStripApvGain::Range& range, const uint32_t index) const
{
if( !(apvgainVector_.empty()) ) {
+ std::cout << "GET_GAIN: " << apvgainVector_[index]->getApvGain(apv, range) << std::endl;
return (apvgainVector_[index]->getApvGain(apv, range))/(normVector_[index]);
}
edm::LogError("SiStripGain::getApvGain") << "ERROR: no gain available. Returning gain = 1." << std::endl;
diff --git a/CondFormats/SiStripObjects/interface/SiStripApvGain.h b/CondFormats/SiStripObjects/interface/SiStripApvGain.h
index c3abeb6..10703a4 100644
--- a/CondFormats/SiStripObjects/interface/SiStripApvGain.h
+++ b/CondFormats/SiStripObjects/interface/SiStripApvGain.h
@@ -74,7 +74,13 @@ class SiStripApvGain {
static float getApvGain (const uint16_t& apv, const Range& range);
#else
static float getStripGain (uint16_t strip, const Range& range) {uint16_t apv = strip/128; return *(range.first+apv);}
- static float getApvGain (uint16_t apv, const Range& range) {return *(range.first+apv);}
+ static float getApvGain (uint16_t apv, const Range& range) {
+ //std::terminate();
+ std::cout << "DEBUG_ACCESS: " << std::hex << std::addressof(*range.first) << std::dec << std::endl;
+ std::cout << "DEBUG_APV: " << std::dec << apv << std::endl;
+ std::cout << "DEBUG_ATTEMPT: " << std::hex << std::addressof(*(range.first+apv)) << std::dec << std::endl;
+ return *(range.first+apv);
+ }
#endif
diff --git a/CondFormats/SiStripObjects/src/SiStripApvGain.cc b/CondFormats/SiStripObjects/src/SiStripApvGain.cc
index 446211f..07acda3 100644
--- a/CondFormats/SiStripObjects/src/SiStripApvGain.cc
+++ b/CondFormats/SiStripObjects/src/SiStripApvGain.cc
@@ -29,8 +29,10 @@ bool SiStripApvGain::put(const uint32_t& DetId, Range input) {
const SiStripApvGain::Range SiStripApvGain::getRange(const uint32_t DetId) const {
// get SiStripApvGain Range of DetId
RegistryConstIterator p = std::lower_bound(v_detids.begin(),v_detids.end(),DetId);
- if (p==v_detids.end() || *p!=DetId)
- return SiStripApvGain::Range(v_gains.end(),v_gains.end());
+ if (p==v_detids.end() || *p!=DetId) {
+ std::cout << "RANGE_EMPTY: " << std::hex << std::addressof(*v_gains.end()) << std::dec << std::endl;
+ return SiStripApvGain::Range(v_gains.end(),v_gains.end());
+ }
else{
unsigned int pd= p-v_detids.begin();
unsigned int ibegin = *(v_ibegin.begin()+pd);@VinInn I guess, this could be interesting to you. I see you have modified relevant parts of these files.
VinInn
commented
8 years ago so, somewhere there is a need to verify that the range is not empty Actually WHY an empty range is returned. Notice this is "ConditionDBWriter" not reconstruction so if the problem exists, is in the client code.... I deal only with hlt and reconstruction use cases that need to be highly optimized the "ConditionDBWriter" can put checks at second instruction... the problem is most probably in SiStripGainFromCalibTree, maybe it has to use a lower level interface w/o the range optimization (that is critical for HLT and reco as described in the pull request or jira somewhere)
davidlt
commented
8 years ago @quertenmont SiStripGainFromCalibTree::algoBeginRun was added by your dcd706f7474a70abc6a83cb81f60b66fe20e256c no so long ago (July).
PR #10680
@mmusich
mmusich
commented
8 years ago @diguida might want to watch as well
smuzaffar
commented
5 years ago closing it as we do not see this error any more in ASAN IBs.
cmsbuild
slc6_amd64_gcc493 and CMSSW_7_6_X_2015-10-19-1100.
Noticed in 1001.0 step3 (ALCA).
I guess, the issue in this line: https://github.com/cms-sw/cmssw/blob/9087f9b34c86c57c3b3bef7e263468cb7e8fc1f4/CondFormats/SiStripObjects/interface/SiStripApvGain.h#L77