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

[GCC 11] Fix error "this pointer is null" in L1Trigger/DTTrigger #34767

Merged
merged 2 commits into from
Aug 18, 2021
Merged

[GCC 11] Fix error "this pointer is null" in L1Trigger/DTTrigger #34767

merged 2 commits into from
Aug 18, 2021

Conversation

iarspider
Copy link
Contributor

@iarspider iarspider commented Aug 4, 2021

Log file: https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/slc7_amd64_gcc11/CMSSW_12_1_X_2021-08-02-1100/L1Trigger/DTTrigger
Error message:

/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/e3e3f354f3bbc00899d20eeb240b2272/opt/cmssw/slc7_amd64_gcc11/cms/cmssw/CMSSW_12_1_X_2021-08-02-1100/src/L1Trigger/DTTrigger/src/DTTrigTest.cc: In member function 'virtual void DTTrigTest::beginRun(const edm::Run&, const edm::EventSetup&)':
  /data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/e3e3f354f3bbc00899d20eeb240b2272/opt/cmssw/slc7_amd64_gcc11/cms/cmssw/CMSSW_12_1_X_2021-08-02-1100/src/L1Trigger/DTTrigger/src/DTTrigTest.cc:198:23: error: 'this' pointer is null [-Werror=nonnull]
   198 |     my_trig->createTUs(iEventSetup);
      |     ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
In file included from /data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/e3e3f354f3bbc00899d20eeb240b2272/opt/cmssw/slc7_amd64_gcc11/cms/cmssw/CMSSW_12_1_X_2021-08-02-1100/src/L1Trigger/DTTrigger/interface/DTTrigTest.h:26,
                 from /data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/e3e3f354f3bbc00899d20eeb240b2272/opt/cmssw/slc7_amd64_gcc11/cms/cmssw/CMSSW_12_1_X_2021-08-02-1100/src/L1Trigger/DTTrigger/src/DTTrigTest.cc:17:
/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/e3e3f354f3bbc00899d20eeb240b2272/opt/cmssw/slc7_amd64_gcc11/cms/cmssw/CMSSW_12_1_X_2021-08-02-1100/src/L1Trigger/DTTrigger/interface/DTTrig.h:78:8: note: in a call to non-static member function 'void DTTrig::createTUs(const edm::EventSetup&)'
   78 |   void createTUs(const edm::EventSetup& iSetup);
      |        ^~~~~~~~~

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

Before submitting your pull requests, make sure you followed this checklist:

@iarspider
Copy link
Contributor Author

@cmsbuild please test for slc7_amd64_gcc11

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 4, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34767/24431

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 4, 2021

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

It involves the following packages:

  • L1Trigger/DTTrigger (l1)

@rekovic, @cecilecaillol can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @battibass this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy, @perrotta you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 4, 2021

-1

Failed Tests: Build
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6ee4f1/17515/summary.html
COMMIT: 22398f2
CMSSW: CMSSW_12_1_X_2021-08-02-1100/slc7_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34767/17515/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6ee4f1/17515/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6ee4f1/17515/git-merge-result

Build

I found compilation error when building:

/cvmfs/cms-ib.cern.ch/nweek-02692/slc7_amd64_gcc11/external/gcc/11.1.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/11.1.1/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lHeterogeneousCoreCUDACore
/cvmfs/cms-ib.cern.ch/nweek-02692/slc7_amd64_gcc11/external/gcc/11.1.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/11.1.1/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lCUDADataFormatsHGCal
/cvmfs/cms-ib.cern.ch/nweek-02692/slc7_amd64_gcc11/external/gcc/11.1.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/11.1.1/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lHeterogeneousCoreCUDAServices
/cvmfs/cms-ib.cern.ch/nweek-02692/slc7_amd64_gcc11/external/gcc/11.1.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/11.1.1/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lCUDADataFormatsCommon
/cvmfs/cms-ib.cern.ch/nweek-02692/slc7_amd64_gcc11/external/gcc/11.1.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/11.1.1/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lHeterogeneousCoreCUDAUtilities
collect2: error: ld returned 1 exit status
gmake: *** [tmp/slc7_amd64_gcc11/src/CondFormats/HGCalObjects/src/CondFormatsHGCalObjects/libCondFormatsHGCalObjects.so] Error 1
Leaving library rule at CondFormats/HGCalObjects
>> Leaving Package CondFormats/HGCalObjects
>> Package CondFormats/HGCalObjects built
>> Entering Package DQM/SiPixelPhase1Summary


@smuzaffar
Copy link
Contributor

smuzaffar commented Aug 4, 2021

please test
GCC 11 tests will keep on failing unless we have a working GCC 11 IB ( at least this PR shows that it fixes the L1Trigger/DTTriger build https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6ee4f1/17515/build-logs/L1Trigger/DTTrigger/log.html )

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 4, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6ee4f1/17529/summary.html
COMMIT: 22398f2
CMSSW: CMSSW_12_1_X_2021-08-03-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34767/17529/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6ee4f1/17529/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6ee4f1/17529/git-merge-result

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 39
  • DQMHistoTests: Total histograms compared: 2998558
  • DQMHistoTests: Total failures: 5
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2998531
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 13814.586 KiB( 38 files compared)
  • DQMHistoSizes: changed ( 10024.0,... ): 767.477 KiB L1T/L1TStage2EMTF
  • Checked 165 log files, 37 edm output root files, 39 DQM output files
  • TriggerResults: found differences in 4 / 38 workflows

@iarspider
Copy link
Contributor Author

please test

@iarspider
Copy link
Contributor Author

please test for slc7_amd64_gcc11

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 7, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34767/24571

  • This PR adds an extra 16KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 7, 2021

Pull request #34767 was updated. @rekovic, @cecilecaillol can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 7, 2021

-1

Failed Tests: Build
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6ee4f1/17631/summary.html
COMMIT: e201b72
CMSSW: CMSSW_12_1_X_2021-08-04-1100/slc7_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34767/17631/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6ee4f1/17631/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6ee4f1/17631/git-merge-result

Build

I found compilation error when building:

/cvmfs/cms-ib.cern.ch/nweek-02692/slc7_amd64_gcc11/external/gcc/11.1.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/11.1.1/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lCondFormatsHcalObjects
/cvmfs/cms-ib.cern.ch/nweek-02692/slc7_amd64_gcc11/external/gcc/11.1.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/11.1.1/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lHeterogeneousCoreCUDACore
/cvmfs/cms-ib.cern.ch/nweek-02692/slc7_amd64_gcc11/external/gcc/11.1.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/11.1.1/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lHeterogeneousCoreCUDAServices
/cvmfs/cms-ib.cern.ch/nweek-02692/slc7_amd64_gcc11/external/gcc/11.1.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/11.1.1/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lCUDADataFormatsCommon
/cvmfs/cms-ib.cern.ch/nweek-02692/slc7_amd64_gcc11/external/gcc/11.1.0/bin/../lib/gcc/x86_64-unknown-linux-gnu/11.1.1/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lHeterogeneousCoreCUDAUtilities
collect2: error: ld returned 1 exit status
gmake: *** [tmp/slc7_amd64_gcc11/src/CalibCalorimetry/HcalTPGAlgos/src/CalibCalorimetryHcalTPGAlgos/libCalibCalorimetryHcalTPGAlgos.so] Error 1
Leaving library rule at CalibCalorimetry/HcalTPGAlgos
>> Subsystem L1Trigger built
>> Leaving Package CalibCalorimetry/HcalTPGAlgos
>> Package CalibCalorimetry/HcalTPGAlgos built


@iarspider
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 8, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6ee4f1/17638/summary.html
COMMIT: e201b72
CMSSW: CMSSW_12_1_X_2021-08-08-0000/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34767/17638/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: 6 differences found in the comparisons
  • DQMHistoTests: Total files compared: 39
  • DQMHistoTests: Total histograms compared: 2999410
  • DQMHistoTests: Total failures: 12
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2999375
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 38 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 165 log files, 37 edm output root files, 39 DQM output files
  • TriggerResults: no differences found

@qliphy
Copy link
Contributor

qliphy commented Aug 10, 2021

kindly ping @cms-sw/l1-l2

@perrotta
Copy link
Contributor

+1

  • @cms-sw/l1-l2 the fix is rather simple and logic, and the error message in gcc11 can hide other relevant error messages: let merge this PR then

@perrotta
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 294a387 into cms-sw:master Aug 18, 2021
@iarspider iarspider deleted the patch-6 branch August 18, 2021 15:48
@@ -194,11 +194,9 @@ void DTTrigTest::beginJob() {
}

void DTTrigTest::beginRun(const edm::Run& iRun, const edm::EventSetup& iEventSetup) {
if (!my_trig) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks to me like they meant to do

if(my_trig) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my_trig is defined as new in the constructor, and only deleted in the destructor: I tend to agree with @iarspider that this check is probably useless
@battibass ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the existence of the my_trig is guaranteed as an invariant of the class, then I agreee there is no need for the check.

Looking at the code, it seems to me like my_trig should just have been a

DTTrig my_trig;

rather than

DTTrig* my_trig;

would would have enforced the guarantee and made memory management easier.

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.

6 participants