1338517 - [Static Analysis][Dereference before null check] In function mergeStacksIntoProfile

Status: RESOLVED INVALID

(Core :: Gecko Profiler, defect)

Description

[Andi [:andi]]

The Static Analysis tool Coverity detected that variable |aSample| might be null thus a null pointer dereference may happen. 

presumably null pointer dereference: 

>>   if (aSample->isSamplingCurrentThread) {
>>     startBufferGen = UINT32_MAX;
>>   } else {
>>     startBufferGen = aInfo.bufferGeneration();
>>   }

null check:

>>    if (aSample && autoWalkJSStack.walkAllowed) {
>>       JS::ProfilingFrameIterator::RegisterState registerState;

But if we check the calling path for mergeStacksIntoProfile we can see that variable |sample| thats passed from:

>>  profile->BeginUnwind();
>>  Tick(&sample);
>>  profile->EndUnwind();

So in order to silence the checker i suggest removing the null check.

Comment 1

[Andi [:andi]]

Attachment: Bug Bug 1338517 - fix false positive for static analysis in mergeStacksIntoProfile.

Review commit: https://reviewboard.mozilla.org/r/111532/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/111532/

Comment 2

[Markus Stange [:mstange]]

Comment on attachment 8836008 [details]
Bug Bug 1338517 - fix false positive for static analysis in mergeStacksIntoProfile.

https://reviewboard.mozilla.org/r/111532/#review112896

Thanks

Comment 3

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s f4575cac3c40 -d b24055689799: rebasing 388345:f4575cac3c40 "Bug Bug 1338517 - fix false positive for static analysis in mergeStacksIntoProfile. r=mstange" (tip)
other [source] changed tools/profiler/core/Sampler.cpp which local [dest] deleted
use (c)hanged version, leave (d)eleted, or leave (u)nresolved? u
unresolved conflicts (see hg resolve, then hg rebase --continue)