search

cms-sw / cmssw

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

[LLVM Analyzer] Called C++ object pointer is null in ConverterToTTTrack::makeTTTrack #46056

Open iarspider opened 1 week ago

iarspider commented 1 week ago

Analyzer report: link .

I guess lines 42-43 should be moved inside else branch, and phi0 and rinv should be initialized to zero (or some other constant value) around line 28

iarspider commented 1 week ago

assign L1Trigger/TrackFindingTMTT

cmsbuild commented 1 week ago

New categories assigned: l1

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

cmsbuild commented 1 week ago

cms-bot internal usage

cmsbuild commented 1 week 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

makortel commented 1 week ago

On the other hand, trk not being L1fittedTrack (i.e. the dynamic_cast failing in https://github.com/cms-sw/cmssw/blob/354a892ac8287945855334f57a27454c40671f25/L1Trigger/TrackFindingTMTT/src/ConverterToTTTrack.cc#L16 , the if branch) is different from trk being null in the first place. So I'd argue the static analyzer warning is strictly speaking incorrect (because trk can be non-null when fitTrk is null).

Written that, if trk can be null, the makeTTTrack() should check it before accessing it, and if tkr is never expected to be null, it should be passed as const reference instead of a pointer.

dan131riley commented 1 week ago

Frivolous question: what's with the const double& on 42-43,45. Surely those should be plain old const double.