search

cms-sw / cmssw

CMS Offline Software
http://cms-sw.github.io/
Apache License 2.0
1.09k stars 4.32k forks source link

[LLVM Analyzer] [Reconstruction] Invalid bit shift in hgcal::econd::produceERxData #46391

Open iarspider opened 1 month ago

iarspider commented 1 month ago

LLVM Analyzer reports invalid bit-shift in hgcal::econd::produceERxData.

Indeed, the value of nbits_word1 calculated on line 69 https://github.com/cms-sw/cmssw/blob/master/EventFilter/HGCalRawToDigi/src/HGCalRawDataPackingTools.cc#L69 can overflow - if, e.g. nbits is 32.

@cms-sw/reconstruction-l2 could you please check the logic here? Thanks!

iarspider commented 1 month ago

assign reconstruction

cmsbuild commented 1 month ago

New categories assigned: reconstruction

@jfernan2,@mandrenguyen you have been requested to review this Pull request/Issue and eventually sign? Thanks

cmsbuild commented 1 month ago

cms-bot internal usage

cmsbuild commented 1 month ago

A new Issue was created by @iarspider.

@Dr15Jones, @antoniovilela, @makortel, @mandrenguyen, @rappoccio, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

jfernan2 commented 1 month ago

@felicepantaleo @rovere as HGCAL experts, could you please have a look and provide a fix? Thanks

felicepantaleo commented 1 month ago

Thanks for letting us know. Indeed this is not correct.
I'm not the expert nor the author of the code, but my guess is that maybe

uint8_t nbits_word1 = 32 - msb - nbits

should be replaced by:

uint8_t nbits_word1 = 32 - msb;

@pfs , what do you think?