UBSAN: avoid int overflow in GenHFHadronMatcher#50891
UBSAN: avoid int overflow in GenHFHadronMatcher#50891smuzaffar wants to merge 1 commit intocms-sw:masterfrom
Conversation
|
cms-bot internal usage |
|
please test workflow 2500.3001 with #50882 for CMSSW_17_0_UBSAN_X |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-50891/49245 |
|
A new Pull Request was created by @smuzaffar for master. It involves the following packages:
@tvami can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test workflow 2500.3001 |
|
+1 Size: This PR adds an extra 28KB to repository Comparison SummarySummary:
|
|
logs https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2dfd16/53097/runTheMatrix-results/2500.3001_NANOmc150X/step2_NANOmc150X.log shows that there is no more int overflow runtime error @cms-sw/analysis-l2 , can you please review this PR? |
| // Inverting the flavour if bb oscillation detected | ||
| if (isHadronPdgId(pdgId, pdg_1) && isHadronPdgId(pdgId, pdg_2) && pdg_1 * pdg_2 < 0) { | ||
| if (isHadronPdgId(pdgId, pdg_1) && isHadronPdgId(pdgId, pdg_2) && | ||
| ((pdg_1 < 0 && pdg_2 > 0) || (pdg_1 > 0 && pdg_2 < 0))) { |
There was a problem hiding this comment.
Another option would be
std::signbit(pdg_1) xor std::signbit(pdg_2)
There was a problem hiding this comment.
@Dr15Jones , I think std::signbit(pdg_1) xor std::signbit(pdg_2) might not work if one of these variables hold 0
There was a problem hiding this comment.
Agreed, but 0 is not a valid value for a PDG id (according to my internet search). Therefore no particle is ever supposed to have that value.
There was a problem hiding this comment.
ah ok, in that case ((pdg_1 < 0) != (pdg_2 < 0)) should be enough but may be then we should add assert to make sure values are not zero
3 participants
UBSAN IBs show runtime errors like
the code in question is https://github.com/cms-sw/cmssw/blob/master/PhysicsTools/JetMCAlgos/plugins/GenHFHadronMatcher.cc#L982 where ubsan complains about pdg_1 * pdg_2` overflowing int range. This PR proposes to change the logic and instead of multiplication just check the sign of these variables.