Closed Bug 1346725 Opened 3 years ago Closed 3 years ago

[Static Analysis][Dereference before null check] In function DoSampleStackTrace

Categories

(Core :: Gecko Profiler, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: andi, Assigned: andi)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: CID 1402128)

Attachments

(1 file)

The Static Analysis tool Coverity detected that |aSample| is null checked after being dereferenced witch implies that a potential null pointer dereference can happen. 
Looking at the places where DoSampleStackTrace gets called we have:

>>#if defined(USE_NS_STACKWALK) || defined(USE_EHABI_STACKWALK) || \
>>    defined(USE_LUL_STACKWALK)
>>  if (gPS->FeatureStackWalk(aLock)) {
>>    DoNativeBacktrace(aLock, aBuffer, aSample);
>>  } else {
>>    DoSampleStackTrace(aLock, aBuffer, aSample);
>>  }
>>#else
>>  DoSampleStackTrace(aLock, aBuffer, aSample);
>>#endif

from function |Tick|, that also gets called in one place:

>>  sample.isSamplingCurrentThread = true;
>>  sample.timestamp = mozilla::TimeStamp::Now();
>>
>>  Tick(lock, buffer, &sample);

Being |sample| passed as an address we can consider that the null check is useless.
Comment on attachment 8846569 [details]
Bug 1346725 - removed useless null check in DoSampleStackTrace.

https://reviewboard.mozilla.org/r/119622/#review122126
Attachment #8846569 - Flags: review?(mstange) → review+
Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8a30570bc738
removed useless null check in DoSampleStackTrace. r=mstange
https://hg.mozilla.org/mozilla-central/rev/8a30570bc738
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.