Bug 1444416: Handle references in the "can run script" checker. r=mystor
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 09 Mar 2018 17:51:59 +0100
changeset 461258 6416568ad06b8e692826f6f8166c706479159eae
parent 461257 fcf5174ce1a6ef32d639958ee3582aa0d30074a8
child 461259 4d8dcb700d09344d701f5ff3c719ae0e2e65c9c8
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmystor
bugs1444416
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1444416: Handle references in the "can run script" checker. r=mystor MozReview-Commit-ID: 8F9HjiFqbGN
build/clang-plugin/CanRunScriptChecker.cpp
build/clang-plugin/tests/TestCanRunScript.cpp
mfbt/StaticAnalysisFunctions.h
--- a/build/clang-plugin/CanRunScriptChecker.cpp
+++ b/build/clang-plugin/CanRunScriptChecker.cpp
@@ -1,26 +1,40 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "CanRunScriptChecker.h"
 #include "CustomMatchers.h"
 
 void CanRunScriptChecker::registerMatchers(MatchFinder *AstMatcher) {
+  auto Refcounted = qualType(hasDeclaration(cxxRecordDecl(isRefCounted())));
   auto InvalidArg =
       // We want to find any expression,
       ignoreTrivials(expr(
           // which has a refcounted pointer type,
-          hasType(pointerType(
-              pointee(hasDeclaration(cxxRecordDecl(isRefCounted()))))),
+          anyOf(
+            hasType(Refcounted),
+            hasType(pointsTo(Refcounted)),
+            hasType(references(Refcounted))
+          ),
           // and which is not this,
           unless(cxxThisExpr()),
           // and which is not a method call on a smart ptr,
           unless(cxxMemberCallExpr(on(hasType(isSmartPtrToRefCounted())))),
+          // and which is not calling operator* on a smart ptr.
+          unless(
+            allOf(
+              cxxOperatorCallExpr(hasOverloadedOperatorName("*")),
+              callExpr(allOf(
+                hasAnyArgument(hasType(isSmartPtrToRefCounted())),
+                argumentCountIs(1)
+              ))
+            )
+          ),
           // and which is not a parameter of the parent function,
           unless(declRefExpr(to(parmVarDecl()))),
           // and which is not a MOZ_KnownLive wrapped value.
           unless(callExpr(callee(functionDecl(hasName("MOZ_KnownLive"))))),
           expr().bind("invalidArg")));
 
   auto OptionalInvalidExplicitArg = anyOf(
       // We want to find any argument which is invalid.
--- a/build/clang-plugin/tests/TestCanRunScript.cpp
+++ b/build/clang-plugin/tests/TestCanRunScript.cpp
@@ -1,9 +1,10 @@
 #include <mozilla/RefPtr.h>
+#include <mozilla/Maybe.h>
 
 #define MOZ_CAN_RUN_SCRIPT __attribute__((annotate("moz_can_run_script")))
 #define MOZ_CAN_RUN_SCRIPT_BOUNDARY __attribute__((annotate("moz_can_run_script_boundary")))
 
 MOZ_CAN_RUN_SCRIPT void test() {
 
 }
 
@@ -133,8 +134,69 @@ MOZ_CAN_RUN_SCRIPT_BOUNDARY void test5()
   RefPtr<RefCountedSubChild> refptr4 = new RefCountedSubChild;
   refptr4->method_test3();
 }
 
 // We should be able to call test5 from a non-can_run_script function.
 void test5_b() {
   test5();
 }
+
+MOZ_CAN_RUN_SCRIPT void test_ref(const RefCountedBase&) {
+
+}
+
+MOZ_CAN_RUN_SCRIPT void test_ref_1() {
+  RefCountedBase* t = new RefCountedBase;
+  test_ref(*t); // expected-error {{arguments must all be strong refs or parent parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument)}}
+}
+
+MOZ_CAN_RUN_SCRIPT void test_ref_2() {
+  RefCountedBase* t = new RefCountedBase;
+  (*t).method_test(); // expected-error {{arguments must all be strong refs or parent parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument)}}
+}
+
+MOZ_CAN_RUN_SCRIPT void test_ref_3() {
+  RefCountedBase* t = new RefCountedBase;
+  auto& ref = *t;
+  test_ref(ref); // expected-error {{arguments must all be strong refs or parent parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument)}}
+}
+
+MOZ_CAN_RUN_SCRIPT void test_ref_4() {
+  RefCountedBase* t = new RefCountedBase;
+  auto& ref = *t;
+  ref.method_test(); // expected-error {{arguments must all be strong refs or parent parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument)}}
+}
+
+MOZ_CAN_RUN_SCRIPT void test_ref_5() {
+  RefPtr<RefCountedBase> t = new RefCountedBase;
+  test_ref(*t);
+}
+
+MOZ_CAN_RUN_SCRIPT void test_ref_6() {
+  RefPtr<RefCountedBase> t = new RefCountedBase;
+  (*t).method_test();
+}
+
+MOZ_CAN_RUN_SCRIPT void test_ref_7() {
+  RefPtr<RefCountedBase> t = new RefCountedBase;
+  auto& ref = *t;
+  MOZ_KnownLive(ref).method_test();
+}
+
+MOZ_CAN_RUN_SCRIPT void test_ref_8() {
+  RefPtr<RefCountedBase> t = new RefCountedBase;
+  auto& ref = *t;
+  test_ref(MOZ_KnownLive(ref));
+}
+
+MOZ_CAN_RUN_SCRIPT void test_maybe() {
+  // FIXME(emilio): This should generate an error, but it's pre-existing!
+  mozilla::Maybe<RefCountedBase*> unsafe;
+  unsafe.emplace(new RefCountedBase);
+  (*unsafe)->method_test();
+}
+
+MOZ_CAN_RUN_SCRIPT void test_maybe_2() {
+  mozilla::Maybe<RefPtr<RefCountedBase>> safe;
+  safe.emplace(new RefCountedBase);
+  (*safe)->method_test();
+}
--- a/mfbt/StaticAnalysisFunctions.h
+++ b/mfbt/StaticAnalysisFunctions.h
@@ -26,16 +26,22 @@
  * that we don't check it if is a strong ref.
  *
  * Example:
  * canRunScript(MOZ_KnownLive(rawPointer));
  */
 template <typename T>
 static MOZ_ALWAYS_INLINE T* MOZ_KnownLive(T* ptr) { return ptr; }
 
+/**
+ * Ditto, but for references.
+ */
+template <typename T>
+static MOZ_ALWAYS_INLINE T& MOZ_KnownLive(T& ref) { return ref; }
+
 extern "C" {
 #endif
 
 /**
  * MOZ_AssertAssignmentTest - used in MOZ_ASSERT in order to test the possible
  * presence of assignment instead of logical comparisons.
  *
  * Example: