Skip to content

Commit 4ad8670

Browse files
mfasDashahor02
authored andcommitted
[EMCAL-610] Fix segfault in Raw creator
- Fix buffer overrun in findBunches leading to segfault - In getting online ID (SRU ID in supermodule had to be scaled with the number of supermodules in order to get the DDL ID) - Convert trailer size as size in 32bit words to size in byte in carryOverMethod in order to calculate properly the payload size
1 parent d325902 commit 4ad8670

3 files changed

Lines changed: 22 additions & 19 deletions

File tree

‎Detectors/EMCAL/base/include/EMCALBase/RCUTrailer.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,8 @@ class RCUTrailer
161161
private:
162162
int mRCUId = -1; ///< current RCU identifier
163163
unsigned char mFirmwareVersion = 0; ///< RCU firmware version
164-
unsigned int mTrailerSize = 0; ///< Size of the trailer
165-
unsigned int mPayloadSize = 0; ///< Size of the payload
164+
unsigned int mTrailerSize = 0; ///< Size of the trailer (in number of 32 bit words)
165+
unsigned int mPayloadSize = 0; ///< Size of the payload (in nunber of 32 bit words)
166166
unsigned int mFECERRA = 0; ///< contains errors related to ALTROBUS transactions
167167
unsigned int mFECERRB = 0; ///< contains errors related to ALTROBUS transactions
168168
unsigned short mERRREG2 = 0; ///< contains errors related to ALTROBUS transactions or trailer of ALTRO channel block

‎Detectors/EMCAL/base/src/Mapper.cxx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,6 @@ Mapper& MappingHandler::getMappingForDDL(int ddl)
115115

116116
std::ostream& o2::emcal::operator<<(std::ostream& stream, const Mapper::ChannelID& channel)
117117
{
118-
stream << "Row " << channel.mRow << ", Column " << channel.mColumn << ", type " << o2::emcal::channelTypeToString(channel.mChannelType);
118+
stream << "Row " << static_cast<int>(channel.mRow) << ", Column " << static_cast<int>(channel.mColumn) << ", type " << o2::emcal::channelTypeToString(channel.mChannelType);
119119
return stream;
120120
}

‎Detectors/EMCAL/simulation/src/RawWriter.cxx‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ bool RawWriter::processNextTrigger()
152152
// register output data
153153
auto ddlid = srucont.mSRUid;
154154
auto [crorc, link] = getLinkAssignment(ddlid);
155+
LOG(DEBUG1) << "Adding payload with size " << payload.size() << " (" << payload.size() / 4 << " ALTRO words)";
155156
mRawWriter->addData(ddlid, crorc, link, 0, mCurrentTrigger->getBCData(), payload);
156157
}
157158
mCurrentTrigger++;
@@ -162,30 +163,32 @@ std::vector<AltroBunch> RawWriter::findBunches(const std::vector<o2::emcal::Digi
162163
{
163164
std::vector<AltroBunch> result;
164165
AltroBunch* currentBunch = nullptr;
165-
int starttime = 0;
166-
for (auto ien = channelDigits.size() - 1;; ien--) {
167-
auto dig = channelDigits[ien];
166+
// Digits in ALTRO bunch in time-reversed order
167+
int itime;
168+
for (itime = channelDigits.size() - 1; itime >= 0; itime--) {
169+
auto dig = channelDigits[itime];
168170
if (!dig) {
169-
starttime++;
170171
continue;
171172
}
172-
int adc = dig->getEnergy() / constants::EMCAL_ADCENERGY; /// conversion Energy <-> ADC := 16 MeV/ADC
173+
int adc = dig->getAmplitudeADC();
173174
if (adc < mPedestal) {
174175
// Stop bunch
176+
// Set the start time to the time sample of previous (passing) digit
177+
currentBunch->mStarttime = itime + 1;
175178
currentBunch = nullptr;
176-
starttime++;
177179
continue;
178180
}
179181
if (!currentBunch) {
180182
// start new bunch
181183
AltroBunch bunch;
182-
bunch.mStarttime = starttime;
183184
result.push_back(bunch);
184185
currentBunch = &(result.back());
185186
}
186187
currentBunch->mADCs.emplace_back(adc);
187-
starttime++;
188188
}
189+
// if we have a last bunch set time start time to the time bin of teh previous digit
190+
if (currentBunch)
191+
currentBunch->mStarttime = itime + 1;
189192
return result;
190193
}
191194

@@ -197,19 +200,19 @@ std::tuple<int, int, int> RawWriter::getOnlineID(int towerID)
197200
auto etaphishift = mGeometry->ShiftOfflineToOnlineCellIndexes(supermoduleID, std::get<0>(etaphi), std::get<1>(etaphi));
198201
int row = std::get<0>(etaphishift), col = std::get<1>(etaphishift);
199202

200-
int sruID = -1;
203+
int ddlInSupermoudel = -1;
201204
if (0 <= row && row < 8)
202-
sruID = 0; // first cable row
205+
ddlInSupermoudel = 0; // first cable row
203206
else if (8 <= row && row < 16 && 0 <= col && col < 24)
204-
sruID = 0; // first half;
207+
ddlInSupermoudel = 0; // first half;
205208
else if (8 <= row && row < 16 && 24 <= col && col < 48)
206-
sruID = 1; // second half;
209+
ddlInSupermoudel = 1; // second half;
207210
else if (16 <= row && row < 24)
208-
sruID = 1; // third cable row
211+
ddlInSupermoudel = 1; // third cable row
209212
if (supermoduleID % 2 == 1)
210-
sruID = 1 - sruID; // swap for odd=C side, to allow us to cable both sides the same
213+
ddlInSupermoudel = 1 - ddlInSupermoudel; // swap for odd=C side, to allow us to cable both sides the same
211214

212-
return std::make_tuple(sruID, row, col);
215+
return std::make_tuple(supermoduleID * 2 + ddlInSupermoudel, row, col);
213216
}
214217

215218
std::tuple<int, int> RawWriter::getLinkAssignment(int ddlID)
@@ -284,7 +287,7 @@ int RawWriter::carryOverMethod(const header::RDHAny* rdh, const gsl::span<char>
284287
gsl::span<const uint32_t> payloadwords(reinterpret_cast<const uint32_t*>(data.data()), data.size() / sizeof(uint32_t));
285288
auto rcutrailer = RCUTrailer::constructFromPayloadWords(payloadwords);
286289

287-
int sizeNoTrailer = maxSize - rcutrailer.getTrailerSize();
290+
int sizeNoTrailer = maxSize - rcutrailer.getTrailerSize() * sizeof(uint32_t);
288291
// calculate payload size for RCU trailer:
289292
// assume actualsize is in byte
290293
// Payload size is defined as the number of 32-bit payload words

0 commit comments

Comments
 (0)