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
DAQ: fix FedRawData source going out of array boundaries #32116
Conversation
mapping, but was allowed in modules, leading to reading out of boundaries. Hardcoded version is no longer used now except in header where it also defines array size.
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32116/19767
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
const size_t FRDHeaderMaxVersion = 6; | ||
const std::array<uint32,FRDHeaderMaxVersion+1> FRDHeaderVersionSize = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
const size_t FRDHeaderMaxVersion = 6; | |
const std::array<uint32,FRDHeaderMaxVersion+1> FRDHeaderVersionSize = { | |
constexpr size_t FRDHeaderMaxVersion = 6; | |
constexpr std::array<uint32,FRDHeaderMaxVersion+1> FRDHeaderVersionSize = { |
? (although the practical impact may be negiligble)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems fine, will add it with code checks fix.
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32116/19768
|
A new Pull Request was created by @smorovic (Srecko Morovic) for master. It involves the following packages: EventFilter/Utilities @perrotta, @smuzaffar, @Dr15Jones, @makortel, @emeschi, @cmsbuild, @slava77, @jpata, @smorovic can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test for slc7_aarch64_gcc820 |
The tests are being triggered in jenkins.
|
-1 Tested at: 24184d2 CMSSW: CMSSW_11_2_X_2020-11-11-2300 I found follow errors while testing this PR Failed tests: Build HeaderConsistency
I found compilation error when building: >> Compiling /home/cmsbld/jenkins_a/workspace/ib-run-pr-tests/CMSSW_11_2_X_2020-11-11-2300/src/IOPool/Streamer/src/StreamerInputSource.cc >> Compiling /home/cmsbld/jenkins_a/workspace/ib-run-pr-tests/CMSSW_11_2_X_2020-11-11-2300/src/IOPool/Streamer/src/StreamerOutputFile.cc >> Compiling /home/cmsbld/jenkins_a/workspace/ib-run-pr-tests/CMSSW_11_2_X_2020-11-11-2300/src/IOPool/Streamer/src/StreamerOutputModuleBase.cc >> Compiling /home/cmsbld/jenkins_a/workspace/ib-run-pr-tests/CMSSW_11_2_X_2020-11-11-2300/src/IOPool/Streamer/src/StreamerOutputModuleCommon.cc In file included from /home/cmsbld/jenkins_a/workspace/ib-run-pr-tests/CMSSW_11_2_X_2020-11-11-2300/src/IOPool/Streamer/src/FRDEventMessage.cc:12: /home/cmsbld/jenkins_a/workspace/ib-run-pr-tests/CMSSW_11_2_X_2020-11-11-2300/src/IOPool/Streamer/interface/FRDEventMessage.h:135:97: error: missing braces around initializer for 'std::__array_traits::_Type' {aka 'unsigned int [7]'} [-Werror=missing-braces] 6 * sizeof(uint32)}; ^ cc1plus: some warnings being treated as errors In file included from /home/cmsbld/jenkins_a/workspace/ib-run-pr-tests/CMSSW_11_2_X_2020-11-11-2300/src/IOPool/Streamer/interface/DumpTools.h:11, from /home/cmsbld/jenkins_a/workspace/ib-run-pr-tests/CMSSW_11_2_X_2020-11-11-2300/src/IOPool/Streamer/src/DumpTools.cc:5: |
Comparison job queued. |
please test for slc7_amd64_gcc820 |
-1 Tested at: 8ab76bd CMSSW: CMSSW_11_2_X_2020-11-11-2300 I found follow errors while testing this PR Failed tests: RelVals AddOn
When I ran the RelVals I found an error in the following workflows: runTheMatrix-results/4.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT/step2_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT.log136.874 step2 runTheMatrix-results/136.874_RunEGamma2018C+RunEGamma2018C+HLTDR2_2018+RECODR2_2018reHLT_skimEGamma_Offline_L1TEgDQM+HARVEST2018_L1TEgDQM/step2_RunEGamma2018C+RunEGamma2018C+HLTDR2_2018+RECODR2_2018reHLT_skimEGamma_Offline_L1TEgDQM+HARVEST2018_L1TEgDQM.log136.731 step2 runTheMatrix-results/136.731_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2/step2_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2.log8.0 step1 runTheMatrix-results/8.0_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS/step1_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS.log136.793 step2 runTheMatrix-results/136.793_RunDoubleEG2017C+RunDoubleEG2017C+HLTDR2_2017+RECODR2_2017reHLT_skimDoubleEG_Prompt+HARVEST2017/step2_RunDoubleEG2017C+RunDoubleEG2017C+HLTDR2_2017+RECODR2_2017reHLT_skimDoubleEG_Prompt+HARVEST2017.log1306.0 step1 runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step1_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log312.0 step1 runTheMatrix-results/312.0_Pyquen_ZeemumuJets_pt10_2760GeV_2021+Pyquen_ZeemumuJets_pt10_2760GeV_2021+DIGIHI2021MIX+RECOHI2021MIX+HARVESTHI2021PPRECO/step1_Pyquen_ZeemumuJets_pt10_2760GeV_2021+Pyquen_ZeemumuJets_pt10_2760GeV_2021+DIGIHI2021MIX+RECOHI2021MIX+HARVESTHI2021PPRECO.log4.22 step2 runTheMatrix-results/4.22_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC/step2_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC.log1330.0 step1 runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15_L1TMuDQM+HARVESTUP15_L1TMuDQM+NANOUP15/step1_ZMM_13+ZMM_13+DIGIUP15+RECOUP15_L1TMuDQM+HARVESTUP15_L1TMuDQM+NANOUP15.log10042.0 step1 runTheMatrix-results/10042.0_ZMM_13+2017+ZMM_13TeV_TuneCUETP8M1_GenSim+Digi+RecoFakeHLT+HARVESTFakeHLT+ALCA+Nano/step1_ZMM_13+2017+ZMM_13TeV_TuneCUETP8M1_GenSim+Digi+RecoFakeHLT+HARVESTFakeHLT+ALCA+Nano.log101.0 step1 runTheMatrix-results/101.0_SingleElectronE120EHCAL+SingleElectronE120EHCAL/step1_SingleElectronE120EHCAL+SingleElectronE120EHCAL.log7.3 step1 runTheMatrix-results/7.3_CosmicsSPLoose_UP18+CosmicsSPLoose_UP18+DIGICOS_UP18+RECOCOS_UP18+ALCACOS_UP18+HARVESTCOS_UP18/step1_CosmicsSPLoose_UP18+CosmicsSPLoose_UP18+DIGICOS_UP18+RECOCOS_UP18+ALCACOS_UP18+HARVESTCOS_UP18.log135.4 step1 runTheMatrix-results/135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step1_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log5.1 step1 runTheMatrix-results/5.1_TTbar+TTbarFS+HARVESTFS/step1_TTbar+TTbarFS+HARVESTFS.log136.8311 step2 runTheMatrix-results/136.8311_RunJetHT2017F_reminiaod+RunJetHT2017F_reminiaod+REMINIAOD_data2017+HARVEST2017_REMINIAOD_data2017/step2_RunJetHT2017F_reminiaod+RunJetHT2017F_reminiaod+REMINIAOD_data2017+HARVEST2017_REMINIAOD_data2017.log136.88811 step2 runTheMatrix-results/136.88811_RunJetHT2018D_reminiaodUL+RunJetHT2018D_reminiaodUL+REMINIAOD_data2018UL+HARVEST2018_REMINIAOD_data2018UL/step2_RunJetHT2018D_reminiaodUL+RunJetHT2018D_reminiaodUL+REMINIAOD_data2018UL+HARVEST2018_REMINIAOD_data2018UL.log136.7611 step2 runTheMatrix-results/136.7611_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM/step2_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM.log1001.0 step2 runTheMatrix-results/1001.0_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVDSIPIXELCALRUN1+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4+ALCAHARVD5/step2_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVDSIPIXELCALRUN1+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4+ALCAHARVD5.log158.01 step2 runTheMatrix-results/158.01_HydjetQ_reminiaodPbPb2018_INPUT+HydjetQ_reminiaodPbPb2018_INPUT+REMINIAODHI2018PPRECOMB+HARVESTHI2018PPRECOMINIAOD/step2_HydjetQ_reminiaodPbPb2018_INPUT+HydjetQ_reminiaodPbPb2018_INPUT+REMINIAODHI2018PPRECOMB+HARVESTHI2018PPRECOMINIAOD.log11634.0 step1 runTheMatrix-results/11634.0_TTbar_14TeV+2021+TTbar_14TeV_TuneCP5_GenSim+Digi+Reco+HARVEST+ALCA/step1_TTbar_14TeV+2021+TTbar_14TeV_TuneCP5_GenSim+Digi+Reco+HARVEST+ALCA.log25202.0 step1 runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25+NANOUP15_PU25/step1_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25+NANOUP15_PU25.log9.0 step1 runTheMatrix-results/9.0_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST/step1_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST.log1000.0 step2 runTheMatrix-results/1000.0_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT/step2_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT.log10824.0 step1 runTheMatrix-results/10824.0_TTbar_13+2018+TTbar_13TeV_TuneCUETP8M1_GenSim+Digi+RecoFakeHLT+HARVESTFakeHLT+ALCA+Nano/step1_TTbar_13+2018+TTbar_13TeV_TuneCUETP8M1_GenSim+Digi+RecoFakeHLT+HARVESTFakeHLT+ALCA+Nano.log25.0 step1 runTheMatrix-results/25.0_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT/step1_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT.log12434.0 step1 runTheMatrix-results/12434.0_TTbar_14TeV+2023+TTbar_14TeV_TuneCP5_GenSim+Digi+Reco+HARVEST+ALCA/step1_TTbar_14TeV+2023+TTbar_14TeV_TuneCP5_GenSim+Digi+Reco+HARVEST+ALCA.log140.53 step2 runTheMatrix-results/140.53_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI/step2_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI.log10024.0 step1 runTheMatrix-results/10024.0_TTbar_13+2017+TTbar_13TeV_TuneCUETP8M1_GenSim+Digi+RecoFakeHLT+HARVESTFakeHLT+ALCA+Nano/step1_TTbar_13+2017+TTbar_13TeV_TuneCUETP8M1_GenSim+Digi+RecoFakeHLT+HARVESTFakeHLT+ALCA+Nano.log10224.0 step1 runTheMatrix-results/10224.0_TTbar_13+2017PU+TTbar_13TeV_TuneCUETP8M1_GenSim+DigiPU+RecoFakeHLTPU+HARVESTFakeHLTPU+Nano/step1_TTbar_13+2017PU+TTbar_13TeV_TuneCUETP8M1_GenSim+DigiPU+RecoFakeHLTPU+HARVESTFakeHLTPU+Nano.log250202.181 step1 runTheMatrix-results/250202.181_TTbar_13UP18+TTbar_13UP18+PREMIXUP18_PU25+DIGIPRMXLOCALUP18_PU25+RECOPRMXUP18_PU25+HARVESTUP18_PU25/step1_TTbar_13UP18+TTbar_13UP18+PREMIXUP18_PU25+DIGIPRMXLOCALUP18_PU25+RECOPRMXUP18_PU25+HARVESTUP18_PU25.log1325.7 step2 runTheMatrix-results/1325.7_TTbar_13_94Xv2NanoAODINPUT+TTbar_13_94Xv2NanoAODINPUT+NANOEDMMC2017_94XMiniAODv2+HARVESTNANOAODMC2017_94XMiniAODv2/step2_TTbar_13_94Xv2NanoAODINPUT+TTbar_13_94Xv2NanoAODINPUT+NANOEDMMC2017_94XMiniAODv2+HARVESTNANOAODMC2017_94XMiniAODv2.log23434.999 step1 runTheMatrix-results/23434.999_TTbar_14TeV+2026D49PU_PMXS1S2PR+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+PREMIX_PremixHLBeamSpot14PU+DigiTriggerPU+RecoGlobalPU+HARVESTGlobalPU/step1_TTbar_14TeV+2026D49PU_PMXS1S2PR+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+PREMIX_PremixHLBeamSpot14PU+DigiTriggerPU+RecoGlobalPU+HARVESTGlobalPU.log23234.0 step1 runTheMatrix-results/23234.0_TTbar_14TeV+2026D49+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal/step1_TTbar_14TeV+2026D49+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal.log28234.0 step1 runTheMatrix-results/28234.0_TTbar_14TeV+2026D60+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal/step1_TTbar_14TeV+2026D60+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal.log140.56 step2 runTheMatrix-results/140.56_RunHI2018+RunHI2018+RECOHID18+HARVESTDHI18/step2_RunHI2018+RunHI2018+RECOHID18+HARVESTDHI18.log
I found errors in the following addon tests: cmsDriver.py TTbar_8TeV_TuneCUETP8M1_cfi --conditions auto:run1_mc --fast -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,EI,VALIDATION --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot Realistic8TeVCollision : FAILED - time: date Thu Nov 12 15:30:40 2020-date Thu Nov 12 15:25:07 2020 s - exit: 34816 |
Comparison job queued. |
+1 |
Comparison job queued. |
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
Comparison is ready Comparison Summary:
|
+1 |
+1
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
Fix FRD format array version check. Version 6 did not get added to the version size mapping, but was allowed in modules, leading to reading out of boundaries.
This by luck did not cause any problems so far on standard gcc builds on x86_64, but caused assertion in IB tests in for CLANG build, as well as ARM and PPC builds where it was spotted (see #32091)
Fix also removes hardcoded version from around the code. It is now defined only in the same header file where it also defines std::array size.
Bugfix doesn't change any other behavior.
Note: it does NOT affect anything used in production DAQ, only version 5 FRD is used there and that is not affected.
PR validation:
Fixes unit test in EventFilter/Utilities/test for CLANG build.