[LLVM Analyzer][DQM] Fix dead assignments #46247

iarspider · 2024-10-04T07:28:09Z

PR description:

This fixes clang-analyzer warnings https://cmssdt.cern.ch/SDT/jenkins-artifacts/ib-static-analysis/CMSSW_14_2_X_2024-09-29-0000/el8_amd64_gcc12/llvm-analysis/index.html

PR validation:

Bot tests

cmsbuild · 2024-10-04T07:28:38Z

cms-bot internal usage

cmsbuild · 2024-10-04T07:29:59Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46247/42071

cmsbuild · 2024-10-04T07:30:23Z

A new Pull Request was created by @iarspider for master.

It involves the following packages:

DQM/BeamMonitor (dqm, db)
DQM/DTMonitorClient (dqm)
DQM/DTMonitorModule (dqm)
DQM/L1TMonitorClient (dqm)
DQM/Physics (dqm)
DQM/SiPixelMonitorClient (dqm)
DQM/SiPixelMonitorCluster (dqm)
DQM/SiPixelMonitorDigi (dqm)
DQM/SiPixelMonitorRawData (dqm)
DQM/SiPixelMonitorTrack (dqm)
DQM/SiStripCommissioningAnalysis (dqm)
DQM/SiStripCommissioningClients (dqm)
DQM/SiStripMonitorSummary (dqm)
DQMOffline/Hcal (dqm)
DQMOffline/JetMET (dqm)
DQMOffline/Muon (dqm)
DQMOffline/Trigger (dqm)
DQMServices/Components (dqm)
DQMServices/Core (dqm)

@antoniovagnerini, @atpathak, @cmsbuild, @consuegs, @francescobrivio, @nothingface0, @perrotta, @rvenditti, @syuvivida, @tjavaid can you please review it and eventually sign? Thanks.
@Fedespring, @HuguesBrun, @JanChyczynski, @PonIlya, @abbiendi, @abdoulline, @ahinzmann, @arossi83, @barvic, @battibass, @bellan, @bsunanda, @calderona, @cericeci, @denizsun, @erikbutz, @fioriNTU, @francescobrivio, @gkasieczka, @idebruyn, @jandrea, @jdamgov, @jdolen, @jhgoh, @mariadalfonso, @missirol, @mmarionncern, @mmusich, @mtosi, @nhanvtran, @ptcox, @rappoccio, @rociovilar, @rsreds, @salimcerci, @schoef, @seemasharmafnal, @sroychow, @threus, @trocino, @venturia, @yuanchao this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

iarspider · 2024-10-04T07:32:42Z

DQM/Physics/src/QcdPhotonsDQM.cc

@@ -340,7 +340,6 @@ void QcdPhotonsDQM::analyze(const Event& iEvent, const EventSetup& iSetup) {
  float photon_eta = -9.0;
  float photon_phi = -9.0;
  bool photon_passPhotonID = false;
-  bool found_lead_pho = false;


There are exactly three uses of this variable: declaration here, check on line 392 and setting to true on line 393

DQM/SiPixelMonitorCluster/src/SiPixelClusterSource.cc

iarspider · 2024-10-04T07:34:27Z

DQM/SiPixelMonitorDigi/src/SiPixelDigiSource.cc

@@ -1138,6 +1138,7 @@ void SiPixelDigiSource::buildStructure(const edm::EventSetup& iSetup) {
        uint32_t id = detId();
        SiPixelDigiModule* theModule = new SiPixelDigiModule(id, ncols, nrows);

+        /*


Same as https://github.com/cms-sw/cmssw/pull/46247/files#r1787288984

DQM/SiPixelMonitorRawData/src/SiPixelRawDataErrorSource.cc

DQMOffline/Muon/src/CSCOfflineMonitor.cc

iarspider · 2024-10-04T07:38:20Z

DQMOffline/Trigger/plugins/HLTInclusiveVBFSource.cc

-    MonitorElement* NumberOfMatches;
-    MonitorElement* NumberOfEvents;
-
-    //dummy                     = iBooker.bookFloat("dummy");


As far as I can tell, bookFloat doesn't have any side-effects, so the result of all these calls is unused.

yes please remove

@iarspider , looks like bookFloat calls bookME which has some side-effects on the store. So may be we need these bookFloat calls ... @cms-sw/dqm-l2 should know better though

@iarspider , looks like bookFloat calls bookME which has some side-effects on the store. So may be we need these bookFloat calls ... @cms-sw/dqm-l2 should know better though

Not in this case, I think. The onbooking function defaults to NOOP ("functor to be passed as a default argument that does not do anything."), and the makeobject function that is passed to bookME explicitly returns nullptr. But I agree that DQM L2s should know better.

Hello. From my limited understanding, all the bookX methods create instances of MonitorElements inside the DQMStore, and do indeed update the store's state (by updating globalMEs_, localMEs_).

I can see, also, that those MonitorElement pointers are being overwritten later in the code (e.g. here, for RECO_deltaPhi_DiJet), but this is not a guarantee (I think) that those specific MEs which have been booked won't be used later.

If those specific MEs (e.g., RECO_deltaEta_DiJet, RECO_deltaPhi_DiJet) are indeed unused later in this module, booking them doesn't make any sense in the first place, right? In this case, I think it would be safe to remove the bookFloat calls, else, I would expect something to probably break later in the module.

Take this with a grain of salt, as I've by no means looked at the DQM code extensively, yet.

I think it would help if we had an example python config that uses this module, so that we can test its output and verify that those MonitorElements are, indeed, booked and left empty.

There is one, but it's outdated

iarspider · 2024-10-04T07:38:54Z

DQMServices/Components/plugins/DQMStoreStats.cc

@@ -62,25 +62,25 @@ class DQMStoreStatsSubfolder {
    ++totalHistos_;
    totalBins_ += nBins;
    totalEmptyBins_ += nEmptyBins;
-    totalMemory_ += (nBins *= sizeof(float));


nBins is passed by value, makes no sense to modify it

iarspider · 2024-10-04T08:28:28Z

please test

cmsbuild · 2024-10-04T08:49:42Z

-1

Failed Tests: Build ClangBuild
Size: This PR adds an extra 188KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-803098/41955/summary.html
COMMIT: 0f39692
CMSSW: CMSSW_14_2_X_2024-10-03-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/46247/41955/install.sh to create a dev area with all the needed externals and cmssw changes.

Build

I found compilation warning when building: See details on the summary page.

Clang Build

I found compilation warning while trying to compile with clang. Command used:

USER_CUDA_FLAGS='--expt-relaxed-constexpr' USER_CXXFLAGS='-Wno-register -fsyntax-only' scram build -k -j 32 COMPILER='llvm compile'

See details on the summary page.

cmsbuild · 2024-10-21T09:51:10Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46247/42319

There are other open Pull requests which might conflict with changes you have proposed:
- File DQM/L1TMonitorClient/src/L1TOccupancyClient.cc modified in PR(s): [DQM]L1TOccupancyClient: Bug fix and fixes static analyzer warning #46442
- File DQMOffline/Hcal/src/HcalRecHitsDQMClient.cc modified in PR(s): [LLVM Analyzer][DQM] Fix LLVM analyzer warnings #46354

cmsbuild · 2024-10-21T09:51:33Z

Pull request #46247 was updated. @antoniovagnerini, @atpathak, @consuegs, @francescobrivio, @nothingface0, @perrotta, @rvenditti, @syuvivida, @tjavaid can you please check and sign again.

…sterSource

cmsbuild · 2024-10-21T10:25:38Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46247/42321

There are other open Pull requests which might conflict with changes you have proposed:
- File DQM/L1TMonitorClient/src/L1TOccupancyClient.cc modified in PR(s): [DQM]L1TOccupancyClient: Bug fix and fixes static analyzer warning #46442
- File DQMOffline/Hcal/src/HcalRecHitsDQMClient.cc modified in PR(s): [LLVM Analyzer][DQM] Fix LLVM analyzer warnings #46354

cmsbuild · 2024-10-21T10:26:02Z

Pull request #46247 was updated. @antoniovagnerini, @atpathak, @cmsbuild, @consuegs, @francescobrivio, @nothingface0, @perrotta, @rvenditti, @syuvivida, @tjavaid can you please check and sign again.

cmsbuild · 2024-10-21T12:30:57Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46247/42324

There are other open Pull requests which might conflict with changes you have proposed:
- File DQM/L1TMonitorClient/src/L1TOccupancyClient.cc modified in PR(s): [DQM]L1TOccupancyClient: Bug fix and fixes static analyzer warning #46442
- File DQMOffline/Hcal/src/HcalRecHitsDQMClient.cc modified in PR(s): [LLVM Analyzer][DQM] Fix LLVM analyzer warnings #46354

cmsbuild · 2024-10-21T12:31:23Z

Pull request #46247 was updated. @antoniovagnerini, @atpathak, @cmsbuild, @consuegs, @francescobrivio, @nothingface0, @perrotta, @rvenditti, @syuvivida, @tjavaid can you please check and sign again.

cmsbuild · 2024-11-22T13:12:12Z

Milestone for this pull request has been moved to CMSSW_15_0_X. Please open a backport if it should also go in to CMSSW_14_2_X.

cmsbuild added this to the CMSSW_14_2_X milestone Oct 4, 2024

cmsbuild added dqm-pending db-pending pending-signatures tests-pending orp-pending code-checks-pending tracking trk labels Oct 4, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 4, 2024

iarspider commented Oct 4, 2024

View reviewed changes

DQM/SiPixelMonitorCluster/src/SiPixelClusterSource.cc Outdated Show resolved Hide resolved

iarspider commented Oct 4, 2024

View reviewed changes

DQM/SiPixelMonitorRawData/src/SiPixelRawDataErrorSource.cc Outdated Show resolved Hide resolved

iarspider commented Oct 4, 2024

View reviewed changes

DQMOffline/Muon/src/CSCOfflineMonitor.cc Outdated Show resolved Hide resolved

iarspider commented Oct 4, 2024

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Oct 4, 2024

cmsbuild added tests-rejected and removed tests-started labels Oct 4, 2024

iarspider force-pushed the iarspider-patch-20241003-2 branch from 0f39692 to 06746e0 Compare October 4, 2024 09:04

cmsbuild removed tests-rejected code-checks-approved labels Oct 4, 2024

cmsbuild added tests-started and removed tests-pending labels Oct 21, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 21, 2024

Restore accidentally deleted code in SiPixelDigiSource and SiPixelClu…

00c1137

…sterSource

cmsbuild added tests-pending code-checks-pending and removed tests-started code-checks-approved labels Oct 21, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 21, 2024

Cleanup

9e5e2cc

cmsbuild added code-checks-pending and removed code-checks-approved labels Oct 21, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 21, 2024

cmsbuild mentioned this pull request Nov 11, 2024

Implement TH2Poly in DQM Services for HGCal DQM #41932

Open

cmsbuild modified the milestones: CMSSW_14_2_X, CMSSW_15_0_X Nov 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LLVM Analyzer][DQM] Fix dead assignments #46247

[LLVM Analyzer][DQM] Fix dead assignments #46247

iarspider commented Oct 4, 2024

cmsbuild commented Oct 4, 2024 •

edited

Loading

cmsbuild commented Oct 4, 2024

cmsbuild commented Oct 4, 2024

iarspider Oct 4, 2024 •

edited

Loading

iarspider Oct 4, 2024

antoniovagnerini Oct 8, 2024

iarspider Oct 4, 2024

antoniovagnerini Oct 8, 2024

smuzaffar Oct 21, 2024

iarspider Oct 21, 2024

nothingface0 Oct 28, 2024

nothingface0 Oct 28, 2024

iarspider Nov 27, 2024

iarspider Oct 4, 2024

antoniovagnerini Oct 8, 2024

iarspider commented Oct 4, 2024

cmsbuild commented Oct 4, 2024

cmsbuild commented Oct 21, 2024

cmsbuild commented Oct 21, 2024

cmsbuild commented Oct 21, 2024

cmsbuild commented Oct 21, 2024

cmsbuild commented Oct 21, 2024

cmsbuild commented Oct 21, 2024

cmsbuild commented Nov 22, 2024

[LLVM Analyzer][DQM] Fix dead assignments #46247

Are you sure you want to change the base?

[LLVM Analyzer][DQM] Fix dead assignments #46247

Conversation

iarspider commented Oct 4, 2024

PR description:

PR validation:

cmsbuild commented Oct 4, 2024 • edited Loading

cmsbuild commented Oct 4, 2024

cmsbuild commented Oct 4, 2024

iarspider Oct 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iarspider commented Oct 4, 2024

cmsbuild commented Oct 4, 2024

Build

Clang Build

cmsbuild commented Oct 21, 2024

cmsbuild commented Oct 21, 2024

cmsbuild commented Oct 21, 2024

cmsbuild commented Oct 21, 2024

cmsbuild commented Oct 21, 2024

cmsbuild commented Oct 21, 2024

cmsbuild commented Nov 22, 2024

cmsbuild commented Oct 4, 2024 •

edited

Loading

iarspider Oct 4, 2024 •

edited

Loading