Skip to content
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

[LTO_X - EventFilter/EcalDigiToRaw] Solve -Wstrict-overflow compiler warning #38682

Conversation

aandvalenzuela
Copy link
Contributor

Hello,

We have seen some compiler warnings of the type -Wstrict-overflow in LTO_X IBs (CMSSW_12_5_LTO_X_2022-07-07-1100 and CMSSW_12_5_LTO_X_2022-07-06-1100, for example) in EventFilter/EcalDigiToRaw. See sample stack trace:

>> Building edm plugin tmp/el8_amd64_gcc10/src/EventFilter/EcalDigiToRaw/src/EventFilterEcalDigiToRaw/libEventFilterEcalDigiToRaw.so
/data/cmsbld/jenkins/workspace/build-any-ib/w/el8_amd64_gcc10/external/gcc/10.3.0-84898dea653199466402e67d73657f10/bin/c++ -O2 -pthread -pipe -Werror=main -Werror=pointer-arith -Werror=overlength-strings -Wno-vla -Werror=overflow -std=c++1z -ftree-vectorize -Wstrict-overflow -Werror=array-bounds -Werror=format-contains-nul -Werror=type-limits -fvisibility-inlines-hidden -fno-math-errno --param vect-max-version-for-alias-checks=50 -Xassembler --compress-debug-sections -msse3 -felide-constructors -fmessage-length=0 -Wall -Wno-non-template-friend -Wno-long-long -Wreturn-type -Wextra -Wpessimizing-move -Wclass-memaccess -Wno-cast-function-type -Wno-unused-but-set-parameter -Wno-ignored-qualifiers -Wno-deprecated-copy -Wno-unused-parameter -Wunused -Wparentheses -Wno-deprecated -Werror=return-type -Werror=missing-braces -Werror=unused-value -Werror=unused-label -Werror=address -Werror=format -Werror=sign-compare -Werror=write-strings -Werror=delete-non-virtual-dtor -Werror=strict-aliasing -Werror=narrowing -Werror=unused-but-set-variable -Werror=reorder -Werror=unused-variable -Werror=conversion-null -Werror=return-local-addr -Wnon-virtual-dtor -Werror=switch -fdiagnostics-show-option -Wno-unused-local-typedefs -Wno-attributes -Wno-psabi -Wno-error=unused-variable -DBOOST_DISABLE_ASSERTS -flto -fipa-icf -flto-odr-type-merging -fno-fat-lto-objects -Wodr -shared -Wl,-E -Wl,-z,defs tmp/el8_amd64_gcc10/src/EventFilter/EcalDigiToRaw/src/EventFilterEcalDigiToRaw/BlockFormatter.cc.o tmp/el8_amd64_gcc10/src/EventFilter/EcalDigiToRaw/src/EventFilterEcalDigiToRaw/EcalDigiToRaw.cc.o tmp/el8_amd64_gcc10/src/EventFilter/EcalDigiToRaw/src/EventFilterEcalDigiToRaw/SRBlockFormatter.cc.o tmp/el8_amd64_gcc10/src/EventFilter/EcalDigiToRaw/src/EventFilterEcalDigiToRaw/TCCBlockFormatter.cc.o tmp/el8_amd64_gcc10/src/EventFilter/EcalDigiToRaw/src/EventFilterEcalDigiToRaw/TowerBlockFormatter.cc.o -o tmp/el8_amd64_gcc10/src/EventFilter/EcalDigiToRaw/src/EventFilterEcalDigiToRaw/libEventFilterEcalDigiToRaw.so -Wl,-E -Wl,--hash-style=gnu -L/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/9802c30fc48bdc09db9c7193cb951860/opt/cmssw/el8_amd64_gcc10/cms/cmssw/CMSSW_12_5_LTO_X_2022-07-07-2300/biglib/el8_amd64_gcc10 -L/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/9802c30fc48bdc09db9c7193cb951860/opt/cmssw/el8_amd64_gcc10/cms/cmssw/CMSSW_12_5_LTO_X_2022-07-07-2300/lib/el8_amd64_gcc10 -L/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/9802c30fc48bdc09db9c7193cb951860/opt/cmssw/el8_amd64_gcc10/cms/cmssw/CMSSW_12_5_LTO_X_2022-07-07-2300/external/el8_amd64_gcc10/lib -lGeometryEcalMapping -lCondFormatsDataRecord -lGeometryRecords -lCondFormatsAlignmentRecord -lDataFormatsEcalDigi -lDataFormatsEcalDetId -lFWCoreFramework -lDataFormatsDetId -lDataFormatsFEDRawData -lFWCoreCommon -lFWCoreServiceRegistry -lDataFormatsCommon -lFWCoreParameterSet -lFWCoreMessageLogger -lDataFormatsProvenance -lFWCorePluginManager -lFWCoreReflection -lFWCoreConcurrency -lFWCoreUtilities -lFWCoreVersion -lTree -lNet -lThread -lMathCore -lRIO -lCore -lboost_thread -lboost_date_time -lpcre -lbz2 -luuid -ltbb -llzma -lz -lfmt -lcms-md5 -lcrypt -ldl -lrt -lstdc++fs -ltinyxml2
/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/9802c30fc48bdc09db9c7193cb951860/opt/cmssw/el8_amd64_gcc10/cms/cmssw/CMSSW_12_5_LTO_X_2022-07-07-2300/src/EventFilter/EcalDigiToRaw/src/TCCBlockFormatter.cc: In member function 'DigiToRaw':
  /data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/9802c30fc48bdc09db9c7193cb951860/opt/cmssw/el8_amd64_gcc10/cms/cmssw/CMSSW_12_5_LTO_X_2022-07-07-2300/src/EventFilter/EcalDigiToRaw/src/TCCBlockFormatter.cc:116:42: warning: assuming signed overflow does not occur when simplifying conditional to constant [-Wstrict-overflow]
   116 |     for (int iline = FE_index - 1; iline < FE_index + (Nrows_TCC + 1) * NTCC - 1; iline++) {
      |                                          ^
Leaving library rule at EventFilter/EcalDigiToRaw

Regarding the type of error and following some online discussions of this type of warning fmtlib/fmt/issues/2757, I have tried to workaround it as shown in this PR, but feel free to propose other solutions that could be better regarding the analysis itself.

Many thanks,
Andrea.

@aandvalenzuela
Copy link
Contributor Author

assign daq

@aandvalenzuela
Copy link
Contributor Author

@cmsbuild , please test for CMSSW_12_5_LTO_X

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38682/30972

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @aandvalenzuela (Andrea Valenzuela) for master.

It involves the following packages:

  • EventFilter/EcalDigiToRaw (daq)

@emeschi, @smorovic can you please review it and eventually sign? Thanks.
@rchatter, @argiro, @Martin-Grunewald, @missirol, @thomreis, @simonepigazzini this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b0b260/26129/summary.html
COMMIT: 4c22d77
CMSSW: CMSSW_12_5_LTO_X_2022-07-09-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38682/26129/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 659 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3655935
  • DQMHistoTests: Total failures: 10365
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3645548
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 208 log files, 45 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@perrotta
Copy link
Contributor

hold
(see minutes in https://indico.cern.ch/event/1184002/)

@cmsbuild
Copy link
Contributor

Pull request has been put on hold by @perrotta
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

@cmsbuild cmsbuild added the hold label Jul 28, 2022
@smuzaffar
Copy link
Contributor

-Wstrict-overflow flag is deprecated and we have dropped it (cms-sw/cmsdist#8026). So these changes are not needed

@smuzaffar smuzaffar closed this Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants