1536737 - * with OwningNonNull isn't considered as safe to use arguments of MOZ_CAN_RUN_SCRIPT methods

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, enhancement)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

OwningNonNull<Element> element(*aElement);
foo->MozCanRunScript(*element);

is not considered as safe. However,

RefPtr<Element> element(aElement);
foo->MozCanRunScript(*element);

is considred as safe.

I think that * for OwningNonNull should be considered as safe like RefPtr.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

(I'm not quite sure about the component for static analysis bugs.)

Comment 2

[Boris Zbarsky [:bzbarsky]]

(I'm not quite sure about the component for static analysis bugs.)

Generally speaking, "Firefox Build System : Source Code Analysis".

What's happening here is that RefPtr actually declares an operator* and we have code in the analysis to allow an overloaded operator* on smartptrs. OwningNonNull does not declare an operator*, so we end up with a UnaryOperator (that is, the default operator*) applied to a member call (the "T* operator()" that OwningNonNull defines).

We do have a check for UnaryOperator that's unaryDereferenceOperator(), but currently only allow function args and "this" with it.

I can change the analysis to allow something like ignoreTrivials(cxxMemberCallExpr(on(KnownLiveSmartPtr))) as the operand for the unary dereference, but I think it's a bit simpler to just declare an operator* on OwningNonNull.

Unfortunately, I can't add a test for this because including OwningNonNull in the analysis test file leads to:

 0:01.22 error: 'warning' diagnostics seen but not expected:
 0:01.22   Line 127: replacement function 'operator new' cannot be declared 'inline'
 0:01.22   Line 134: replacement function 'operator new' cannot be declared 'inline'
 0:01.22   Line 139: replacement function 'operator new[]' cannot be declared 'inline'
 0:01.22   Line 144: replacement function 'operator new[]' cannot be declared 'inline'
 0:01.22   Line 149: replacement function 'operator delete' cannot be declared 'inline'
 0:01.22   Line 154: replacement function 'operator delete' cannot be declared 'inline'
 0:01.22   Line 159: replacement function 'operator delete[]' cannot be declared 'inline'
 0:01.22   Line 164: replacement function 'operator delete[]' cannot be declared 'inline'

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1536737. Give OwningNonNull an operator* to make it play nicer with the MOZ_CAN_RUN_SCRIPT static analysis. r=froydnj

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5c20413ba83d
Give OwningNonNull an operator* to make it play nicer with the MOZ_CAN_RUN_SCRIPT static analysis.  r=froydnj

Comment 5

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/5c20413ba83d