Skip to content
Snippets Groups Projects
Commit 1565ca2d authored by Peter Berta's avatar Peter Berta Committed by Atlas Nightlybuild
Browse files

Merge branch 'ATLASRECTS-6692_22.0-mc20_fix_usage_of_invalid_hash_and_range_error' into '22.0-mc20'

Fix usage of invalid hash and prevent range errors in SCT_RodDecoder.

See merge request !48437

(cherry picked from commit b72773b8)

57ca4379 Check SCT hashes before using them.
d9133f43 Enssure that strip and the derived index are valid before using them.
352d050b Remove error check which will always pass and perform more arithmetics at compile time.
aa9f6e50 Use idnetifier hash as hash rather than computing a new hash from the IdentifierHash.
parent 24f2f375
5 merge requests!69091Fix correlated smearing bug in JER in JetUncertainties in 22.0,!58791DataQualityConfigurations: Modify L1Calo config for web display,!51674Fixing hotSpotInHIST for Run3 HIST,!50012RecExConfig: Adjust log message levels from GetRunNumber and GetLBNumber,!48504Sweeping !48437 from 22.0-mc20 to master. Fix usage of invalid hash and prevent range errors in SCT_RodDecoder.
Pipeline #3279852 failed
...@@ -661,6 +661,7 @@ StatusCode SCT_RodDecoder::processHeader(const uint16_t inData, ...@@ -661,6 +661,7 @@ StatusCode SCT_RodDecoder::processHeader(const uint16_t inData,
// This is the real calculation for the offline // This is the real calculation for the offline
data.linkNumber = (((rodlinkNumber >>4)&0x7)*12+(rodlinkNumber &0xF)); data.linkNumber = (((rodlinkNumber >>4)&0x7)*12+(rodlinkNumber &0xF));
const uint32_t onlineID{(robID & 0xFFFFFF) | (data.linkNumber << 24)}; const uint32_t onlineID{(robID & 0xFFFFFF) | (data.linkNumber << 24)};
IdentifierHash hash;
if ((onlineID ==0) or (data.linkNumber > 95)) { if ((onlineID ==0) or (data.linkNumber > 95)) {
ATH_CHECK(addSingleError(data.linkIDHash, SCT_ByteStreamErrors::ByteStreamParseError, errs)); ATH_CHECK(addSingleError(data.linkIDHash, SCT_ByteStreamErrors::ByteStreamParseError, errs));
hasError = true; hasError = true;
...@@ -670,7 +671,15 @@ StatusCode SCT_RodDecoder::processHeader(const uint16_t inData, ...@@ -670,7 +671,15 @@ StatusCode SCT_RodDecoder::processHeader(const uint16_t inData,
return sc; return sc;
} }
else { else {
data.setCollection(m_sctID, m_cabling->getHashFromOnlineId(onlineID, ctx), rdoIDCont, errs); hash = m_cabling->getHashFromOnlineId(onlineID, ctx);
if (hash.is_valid()) {
data.setCollection(m_sctID, hash, rdoIDCont, errs);
}
else {
std::stringstream msg;
msg <<std::hex << onlineID;
ATH_MSG_ERROR("Rob fragment (rob=" << robID << ") with invalid onlineID " << msg.str() << " -> " << hash << ".");
}
} }
// Look for masked off links - bit 7 // Look for masked off links - bit 7
if ((inData >> 7) & 0x1) { if ((inData >> 7) & 0x1) {
...@@ -712,6 +721,12 @@ StatusCode SCT_RodDecoder::processHeader(const uint16_t inData, ...@@ -712,6 +721,12 @@ StatusCode SCT_RodDecoder::processHeader(const uint16_t inData,
ATH_CHECK(addSingleError(data.linkIDHash, SCT_ByteStreamErrors::FormatterError, errs)); ATH_CHECK(addSingleError(data.linkIDHash, SCT_ByteStreamErrors::FormatterError, errs));
hasError = true; hasError = true;
} }
if (!hasError and not hash.is_valid()) {
std::stringstream msg;
msg <<std::hex << onlineID;
ATH_MSG_ERROR("Rob fragment (rob=" << robID << ") with invalid onlineID " << msg.str() << " -> " << hash << ".");
hasError = true;
}
data.condensedMode = static_cast<bool>(inData & 0x100); data.condensedMode = static_cast<bool>(inData & 0x100);
...@@ -991,19 +1006,13 @@ StatusCode SCT_RodDecoder::processExpandedHit(const uint16_t inData, ...@@ -991,19 +1006,13 @@ StatusCode SCT_RodDecoder::processExpandedHit(const uint16_t inData,
} }
else { // Next hits cluster expanded else { // Next hits cluster expanded
if (inData & 0x80) { // Paired hits if (inData & 0x80) { // Paired hits
if (data.strip >= N_STRIPS_PER_SIDE) { if (data.strip >= N_STRIPS_PER_SIDE-2) {
ATH_CHECK(addSingleError(data.linkIDHash, SCT_ByteStreamErrors::ByteStreamParseError, errs)); ATH_CHECK(addSingleError(data.linkIDHash, SCT_ByteStreamErrors::ByteStreamParseError, errs));
hasError = true; hasError = true;
ATH_MSG_DEBUG("Expanded mode - strip number out of range"); ATH_MSG_DEBUG("Expanded mode - strip number out of range");
return sc; return sc;
} }
m_evenExpHitNumber++; m_evenExpHitNumber++;
if (chip>=N_CHIPS_PER_SIDE) {
ATH_MSG_DEBUG("Expanded Hit: paired hits xxx ERROR chip Nb = " << chip << " >= " << N_CHIPS_PER_SIDE);
m_chipNumberError++;
ATH_CHECK(addSingleError(data.linkIDHash, SCT_ByteStreamErrors::ByteStreamParseError, errs));
return sc;
}
// First hit from the pair // First hit from the pair
data.strip++; data.strip++;
data.timeBin = (inData & 0x7); data.timeBin = (inData & 0x7);
...@@ -1031,10 +1040,10 @@ StatusCode SCT_RodDecoder::processExpandedHit(const uint16_t inData, ...@@ -1031,10 +1040,10 @@ StatusCode SCT_RodDecoder::processExpandedHit(const uint16_t inData,
} }
else { // Last hit of the cluster else { // Last hit of the cluster
m_lastExpHitNumber++; m_lastExpHitNumber++;
if (chip>=N_CHIPS_PER_SIDE) { if (data.strip >= N_STRIPS_PER_SIDE-1) {
ATH_MSG_DEBUG("Expanded Hit: last hit xxx ERROR chip Nb = " << chip << " >= " << N_CHIPS_PER_SIDE);
m_chipNumberError++;
ATH_CHECK(addSingleError(data.linkIDHash, SCT_ByteStreamErrors::ByteStreamParseError, errs)); ATH_CHECK(addSingleError(data.linkIDHash, SCT_ByteStreamErrors::ByteStreamParseError, errs));
hasError = true;
ATH_MSG_DEBUG("Expanded mode - strip number out of range");
return sc; return sc;
} }
data.strip++; data.strip++;
......
// -*- C++ -*- // -*- C++ -*-
/* /*
Copyright (C) 2002-2020 CERN for the benefit of the ATLAS collaboration Copyright (C) 2002-2021 CERN for the benefit of the ATLAS collaboration
*/ */
#ifndef INDETRAWDATABYTESTREAM_SCT_RODDECODER_H #ifndef INDETRAWDATABYTESTREAM_SCT_RODDECODER_H
...@@ -147,12 +147,19 @@ class SCT_RodDecoder : public extends<AthAlgTool, ISCT_RodDecoder> ...@@ -147,12 +147,19 @@ class SCT_RodDecoder : public extends<AthAlgTool, ISCT_RodDecoder>
// For MissingLinkHeaderError // For MissingLinkHeaderError
bool foundMissingLinkHeaderError{false}; bool foundMissingLinkHeaderError{false};
std::unordered_set<IdentifierHash> foundHashes; std::unordered_set<IdentifierHash> foundHashes;
struct Hasher {
std::unordered_map<IdentifierHash, std::unique_ptr<SCT_RDO_Collection>> rdoCollMap; // If SCT_RDO_Collection* is nullptr, it means the collection is already present in the container. std::size_t operator()(const IdentifierHash &hash) const { return hash.value();}
std::unordered_map<IdentifierHash, SCT_RDO_Container::IDC_WriteHandle> writeHandleMap; };
std::unordered_map<IdentifierHash, std::unique_ptr<SCT_RDO_Collection>, Hasher> rdoCollMap; // If SCT_RDO_Collection* is nullptr, it means the collection is already present in the container.
std::unordered_map<IdentifierHash, SCT_RDO_Container::IDC_WriteHandle, Hasher> writeHandleMap;
bool foundHeader{false}; bool foundHeader{false};
SharedData() {
writeHandleMap.reserve( 72);
rdoCollMap.reserve( 72 );
}
void reset() { void reset() {
strip = 0; strip = 0;
oldStrip = -1; oldStrip = -1;
...@@ -169,18 +176,20 @@ class SCT_RodDecoder : public extends<AthAlgTool, ISCT_RodDecoder> ...@@ -169,18 +176,20 @@ class SCT_RodDecoder : public extends<AthAlgTool, ISCT_RodDecoder>
} }
void setSaved(const bool isOld, const int code) { void setSaved(const bool isOld, const int code) {
if (isOld) { if (isOld) {
saved[oldSide*N_STRIPS_PER_SIDE + oldStrip] = code; saved.at(oldSide*N_STRIPS_PER_SIDE + oldStrip) = code;
} }
else { else {
saved[ side*N_STRIPS_PER_SIDE + strip] = code; saved.at( side*N_STRIPS_PER_SIDE + strip) = code;
} }
} }
bool isSaved(const bool isOld) { bool isSaved(const bool isOld) {
if (isOld) { if (isOld) {
return saved[oldSide*N_STRIPS_PER_SIDE + oldStrip]; unsigned int idx = static_cast<std::size_t>(oldSide*N_STRIPS_PER_SIDE + oldStrip);
return idx < saved.size() ? saved[idx] : false;
} }
else { else {
return saved[ side*N_STRIPS_PER_SIDE + strip]; const unsigned int idx = static_cast<unsigned int>(side*N_STRIPS_PER_SIDE + strip);
return idx < saved.size() ? saved[idx] : false;
} }
} }
void setCollection(const SCT_ID* sctID, void setCollection(const SCT_ID* sctID,
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment