-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Initialization in CPEBase to fix UBSAN error #38400
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38400/30599
|
A new Pull Request was created by @OzAmram (Oz Amram) for master. It involves the following packages:
@jpata, @cmsbuild, @clacaputo, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild , please test |
@cmsbuild , please test for CMSSW_12_5_UBSAN_X |
type bugfix,trk |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e0770a/25576/summary.html |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e0770a/25575/summary.html Comparison SummarySummary:
|
@@ -49,7 +49,8 @@ class PixelCPEBase : public PixelClusterParameterEstimator { | |||
const PixelTopology* theTopol; | |||
const RectangularPixelTopology* theRecTopol; | |||
|
|||
GeomDetType::SubDetector thePart; | |||
//set default value of enum to avoid USBAN errors | |||
GeomDetType::SubDetector thePart = GeomDetEnumerators::invalidDet; |
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.
why just this data member and not all the others?
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.
It is the only enum variable and the only one causing the UBSAN error (still very strange to me as the uninitialized value is never used). I could add initialization to the other variables if we want
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.
ah ok. the usual false positive...
+reconstruction
|
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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
This PR adds an initialization for one value of the
DetParam
struct in PixelCPEBase that was causing a UBSAN error (#35036). No changes in output are expected