1289064 - [Static Analysis][Dereference before null check] In function XPCConvert::NativeInterface2JSObject

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Andi [:andi]]

The Static Analysis tool Coverity detected that pointer |iid| is dereferenced before being null checked:

dereference:

#ifdef SPIDERMONKEY_PROMISE
>>    if (iid->Equals(NS_GET_IID(nsISupports))) {
>>        // Check for a Promise being returned via nsISupports.  In that
>>        // situation, we want to dig out its underlying JS object and return
>>        // that.
>>        RefPtr<Promise> promise = do_QueryObject(aHelper.Object()); 

null check:

>>    // Go ahead and create an XPCWrappedNative for this object.
>>    AutoMarkingNativeInterfacePtr iface(cx);
>>    if (iid) {
>>        if (Interface)
>>            iface = *Interface;

I don't think the null check has a purpose here since of these lines:

>>    MOZ_ASSERT_IF(Interface, iid);
>>    if (!iid)
>>        iid = &NS_GET_IID(nsISupports);

And if we expand NS_GET_IID macro we get that iid becomes the address of:

 template<typename T>                                                 
  const nsIID the_interface::COMTypeInfo<the_interface, T>::kIID

Comment 1

[Andi [:andi]]

Attachment: Bug 1289064 - eliminate null check on |iid| in XPCConvert::NativeInterface2JSObject.

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

Comment 2

[Bobby Holley (:bholley)]

Comment on attachment 8774289 [details]
Bug 1289064 - eliminate null check on |iid| in XPCConvert::NativeInterface2JSObject.

https://reviewboard.mozilla.org/r/66808/#review63706

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9e9ba7b85410
eliminate null check on |iid| in XPCConvert::NativeInterface2JSObject. r=bholley

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9e9ba7b85410