Bug 1418465 - Add an opt-out to the MOZ_CAN_RUN_SCRIPT analysis, r=andi
authorNika Layzell <nika@thelayzells.com>
Fri, 17 Nov 2017 15:12:36 -0500
changeset 438101 9029701afb620ea6f13fdf75fe3943151d3defdf
parent 438100 6ecf891cc6ce23f8b2a2635f171903220343baf4
child 438140 4ce67352b2a9744ac51f0333cea0455e3d59bbf3
push id117
push userfmarier@mozilla.com
push dateTue, 28 Nov 2017 20:17:16 +0000
reviewersandi
bugs1418465
milestone59.0a1
Bug 1418465 - Add an opt-out to the MOZ_CAN_RUN_SCRIPT analysis, r=andi MozReview-Commit-ID: 2YKncUdrT5p
build/clang-plugin/CanRunScriptChecker.cpp
build/clang-plugin/tests/TestCanRunScript.cpp
mfbt/Attributes.h
--- a/build/clang-plugin/CanRunScriptChecker.cpp
+++ b/build/clang-plugin/CanRunScriptChecker.cpp
@@ -176,17 +176,25 @@ void CanRunScriptChecker::check(const Ma
                     !CanRunScriptFuncs.count(Construct->getConstructor()))) {
     Construct = nullptr;
   }
 
   const FunctionDecl *ParentFunction =
       Result.Nodes.getNodeAs<FunctionDecl>("nonCanRunScriptParentFunction");
   // If the parent function can run script, consider that we didn't find a match
   // because we only care about parent functions which can't run script.
-  if (ParentFunction && CanRunScriptFuncs.count(ParentFunction)) {
+  //
+  // In addition, If the parent function is annotated as a
+  // CAN_RUN_SCRIPT_BOUNDARY, we don't want to complain about it calling a
+  // CAN_RUN_SCRIPT function. This is a mechanism to opt out of the infectious
+  // nature of CAN_RUN_SCRIPT which is necessary in some tricky code like
+  // Bindings.
+  if (ParentFunction &&
+      (CanRunScriptFuncs.count(ParentFunction) ||
+       hasCustomAnnotation(ParentFunction, "moz_can_run_script_boundary"))) {
     ParentFunction = nullptr;
   }
 
   // Get the call range from either the CallExpr or the ConstructExpr.
   SourceRange CallRange;
   if (Call) {
     CallRange = Call->getSourceRange();
   } else if (Construct) {
--- a/build/clang-plugin/tests/TestCanRunScript.cpp
+++ b/build/clang-plugin/tests/TestCanRunScript.cpp
@@ -1,11 +1,12 @@
 #include <mozilla/RefPtr.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() {
 
 }
 
 void test_parent() { // expected-note {{parent function declared here}}
   test(); // expected-error {{functions marked as MOZ_CAN_RUN_SCRIPT can only be called from functions also marked as MOZ_CAN_RUN_SCRIPT}}
 }
@@ -99,9 +100,28 @@ MOZ_CAN_RUN_SCRIPT void test4() {
   RefPtr<RefCountedBase> refptr2 = new RefCountedSubChild;
   refptr2->method_test3();
 
   RefPtr<RefCountedChild> refptr3 = new RefCountedSubChild;
   refptr3->method_test3();
 
   RefPtr<RefCountedSubChild> refptr4 = new RefCountedSubChild;
   refptr4->method_test3();
-}
\ No newline at end of file
+}
+
+MOZ_CAN_RUN_SCRIPT_BOUNDARY void test5() {
+  RefPtr<RefCountedBase> refptr1 = new RefCountedChild;
+  refptr1->method_test3();
+
+  RefPtr<RefCountedBase> refptr2 = new RefCountedSubChild;
+  refptr2->method_test3();
+
+  RefPtr<RefCountedChild> refptr3 = new RefCountedSubChild;
+  refptr3->method_test3();
+
+  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();
+}
--- a/mfbt/Attributes.h
+++ b/mfbt/Attributes.h
@@ -433,16 +433,20 @@
  *   MOZ_CASE_ATTRIBUTE case 5:
  *   MOZ_DEFAULT_ATTRIBUTE default:
  *
  * The static analyses that are performed by the plugin are as follows:
  *
  * MOZ_CAN_RUN_SCRIPT: Applies to functions which can run script. Callers of
  *   this function must also be marked as MOZ_CAN_RUN_SCRIPT, and all refcounted
  *   arguments must be strongly held in the caller.
+ * MOZ_CAN_RUN_SCRIPT_BOUNDARY: Applies to functions which need to call
+ *   MOZ_CAN_RUN_SCRIPT functions, but should not themselves be considered
+ *   MOZ_CAN_RUN_SCRIPT. This is important for some bindings and low level code
+ *   which need to opt out of the safety checks performed by MOZ_CAN_RUN_SCRIPT.
  * MOZ_MUST_OVERRIDE: Applies to all C++ member functions. All immediate
  *   subclasses must provide an exact override of this method; if a subclass
  *   does not override this method, the compiler will emit an error. This
  *   attribute is not limited to virtual methods, so if it is applied to a
  *   nonvirtual method and the subclass does not provide an equivalent
  *   definition, the compiler will emit an error.
  * MOZ_STACK_CLASS: Applies to all classes. Any class with this annotation is
  *   expected to live on the stack, so it is a compile-time error to use it, or
@@ -592,16 +596,17 @@
  *   and calls to functions or methods marked as MOZ_MAY_CALL_AFTER_MUST_RETURN
  *   may be made after the MUST_RETURN_FROM_CALLER call.
  * MOZ_MAY_CALL_AFTER_MUST_RETURN: Applies to function or method declarations.
  *   Calls to these methods may be made in functions after calls a
  *   MOZ_MUST_RETURN_FROM_CALLER function or method.
  */
 #ifdef MOZ_CLANG_PLUGIN
 #  define MOZ_CAN_RUN_SCRIPT __attribute__((annotate("moz_can_run_script")))
+#  define MOZ_CAN_RUN_SCRIPT_BOUNDARY __attribute__((annotate("moz_can_run_script_boundary")))
 #  define MOZ_MUST_OVERRIDE __attribute__((annotate("moz_must_override")))
 #  define MOZ_STACK_CLASS __attribute__((annotate("moz_stack_class")))
 #  define MOZ_NONHEAP_CLASS __attribute__((annotate("moz_nonheap_class")))
 #  define MOZ_HEAP_CLASS __attribute__((annotate("moz_heap_class")))
 #  define MOZ_NON_TEMPORARY_CLASS __attribute__((annotate("moz_non_temporary_class")))
 #  define MOZ_TRIVIAL_CTOR_DTOR __attribute__((annotate("moz_trivial_ctor_dtor")))
 #  ifdef DEBUG
      /* in debug builds, these classes do have non-trivial constructors. */
@@ -647,16 +652,17 @@
  */
 #  define MOZ_HEAP_ALLOCATOR \
     _Pragma("clang diagnostic push") \
     _Pragma("clang diagnostic ignored \"-Wgcc-compat\"") \
     __attribute__((annotate("moz_heap_allocator"))) \
     _Pragma("clang diagnostic pop")
 #else
 #  define MOZ_CAN_RUN_SCRIPT /* nothing */
+#  define MOZ_CAN_RUN_SCRIPT_BOUNDARY /* nothing */
 #  define MOZ_MUST_OVERRIDE /* nothing */
 #  define MOZ_STACK_CLASS /* nothing */
 #  define MOZ_NONHEAP_CLASS /* nothing */
 #  define MOZ_HEAP_CLASS /* nothing */
 #  define MOZ_NON_TEMPORARY_CLASS /* nothing */
 #  define MOZ_TRIVIAL_CTOR_DTOR /* nothing */
 #  define MOZ_ONLY_USED_TO_AVOID_STATIC_CONSTRUCTORS /* nothing */
 #  define MOZ_IMPLICIT /* nothing */