1250134 - [Static Analysis][Dereference after null check] In function nsCycleCollector::CollectWhite

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Andi [:andi]]

The Static Analysis tool Coverity added that pointer |mJSRuntime| is dereferenced after it's null checked:

null checked:
>>  bool hasJSRuntime = !!mJSRuntime;
>>  nsCycleCollectionParticipant* zoneParticipant =
>>    hasJSRuntime ? mJSRuntime->ZoneParticipant() : nullptr; 

dereferencing:
>>        mJSRuntime->AddZoneWaitingForGC(zone);
>>      } else {

Comment 1

[Andi [:andi]]

Attachment: MozReview Request: Bug 1250134 - assert mJSRuntime when IsGrayJS() is true. r?mccr8

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

Comment 2

[Andrew McCreight [:mccr8]]

I can review cycle collector changes.

This null check is actually redundant because IsGrayJS() can only be true if mJSRuntime is true. Maybe there could be an assert for that or something because it isn't directly obvious from the code.

Comment 3

[Andrew McCreight [:mccr8]]

Or something like MOZ_RELEASE_ASSERT(mJSRuntime) if that would make Coverity happier.

Comment 4

[Andi [:andi]]

(In reply to Andrew McCreight [:mccr8] from comment #3)
> Or something like MOZ_RELEASE_ASSERT(mJSRuntime) if that would make Coverity
> happier.
I've added MOZ_ASSERT since coverity runs on a debug build and 't think we need this kind of assert on our release as well.

Comment 5

[Andi [:andi]]

Comment on attachment 8721977 [details]
MozReview Request: Bug 1250134 - assert mJSRuntime when IsGrayJS() is true. r?mccr8

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35815/diff/1-2/

Comment 6

[Andrew McCreight [:mccr8]]

Comment on attachment 8721977 [details]
MozReview Request: Bug 1250134 - assert mJSRuntime when IsGrayJS() is true. r?mccr8

https://reviewboard.mozilla.org/r/35815/#review32685

Thanks.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bf62a6c05fa7

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bf62a6c05fa7