Closed Bug 1355383 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Gecko Profiler, enhancement)

enhancement
Not set

Tracking

()

RESOLVED DUPLICATE of bug 1347258
Tracking Status
firefox55 --- affected

People

(Reporter: andi, Assigned: njn, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, good-first-bug, Whiteboard: CID 1400179)

Attachments

(1 file)

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.
Assignee: nobody → n.nethercote
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1347258
aSample cannot be null, remove useless null check
Attachment #8857934 - Flags: review?(sledru)
Attachment #8857934 - Flags: review?(bpostelnicu)
Hello lee, sorry but this patch has been already taken and dealt with by Nicholas. Thank you again for submitting the patch!
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 on attachment 8857934 [details] [diff] [review]
bug 1355383 - remove useless null check (line 604)

Just like Andi said!
Attachment #8857934 - Flags: review?(sledru)
Attachment #8857934 - Flags: review?(bpostelnicu)
You need to log in before you can comment on or make changes to this bug.