1355383 - [Static Analysis][Dereference before null check] In function MergeStacksIntoProfile

Status: RESOLVED DUPLICATE of bug 1347258

(Core :: Gecko Profiler, enhancement)

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: 

>>    NotNull<PseudoStack*> pseudoStack = aSample->mPseudoStack;

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| that's passed from:

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

Cannot be null.

So the null check should be removed since it's useless.

Comment 2

[Chris Lee]

Attachment: bug 1355383 - remove useless null check (line 604)

aSample cannot be null, remove useless null check

Comment 3

[Andi [:andi]]

Hello lee, sorry but this patch has been already taken and dealt with by Nicholas. Thank you again for submitting the patch!

Comment 4

[Nicholas Nethercote [inactive]]

Yes, I fixed this as part of a larger change in bug 1347258, which had already been filed. Sorry!

The good news is that your patch was correct. I do suggest putting this into your .hgrc file:

> [diff]
> git = true
> showfunc = true
> unified = 8

That will put the diffs into the preferred format.

Thank you for contributing! I hope you can find another good first bug at https://bugzilla.mozilla.org/buglist.cgi?quicksearch=sw%3A%22%5Bgood%20first%20bug%5D%22&limit=0&list_id=13537039

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8857934 [details] [diff] [review]
bug 1355383 - remove useless null check (line 604)

Just like Andi said!