Improve the documentation around the MOZ_CAN_RUN_SCRIPT analysis. No bug. r=emilio
authorBoris Zbarsky <bzbarsky@mit.edu>
Sat, 16 Mar 2019 12:52:33 +0000
changeset 464492 37761497b6c1
parent 464491 9d4aec1e584c
child 464493 8ee97c045359
push id35716
push useraciure@mozilla.com
push dateSun, 17 Mar 2019 09:42:17 +0000
treeherdermozilla-central@8ee97c045359 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
milestone67.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
Improve the documentation around the MOZ_CAN_RUN_SCRIPT analysis. No bug. r=emilio Differential Revision: https://phabricator.services.mozilla.com/D23762
build/clang-plugin/CanRunScriptChecker.cpp
mfbt/Attributes.h
--- a/build/clang-plugin/CanRunScriptChecker.cpp
+++ b/build/clang-plugin/CanRunScriptChecker.cpp
@@ -1,12 +1,54 @@
 /* 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/. */
 
+/**
+ * This checker implements the "can run script" analysis.  The idea is to detect
+ * functions that can run script that are being passed reference-counted
+ * arguments (including "this") whose refcount might go to zero as a result of
+ * the script running.  We want to prevent that.
+ *
+ * The approach is to attempt to enforce the following invariants on the call
+ * graph:
+ *
+ * 1) Any caller of a MOZ_CAN_RUN_SCRIPT function is itself MOZ_CAN_RUN_SCRIPT.
+ * 2) If a virtual MOZ_CAN_RUN_SCRIPT method overrides a base class method,
+ *    that base class method is also MOZ_CAN_RUN_SCRIPT.
+ *
+ * Invariant 2 ensures that we don't accidentally call a MOZ_CAN_RUN_SCRIPT
+ * function via a base-class virtual call.  Invariant 1 ensures that
+ * the property of being able to run script propagates up the callstack.  There
+ * is an opt-out for invariant 1: A function (declaration _or_ implementation)
+ * can be decorated with MOZ_CAN_RUN_SCRIPT_BOUNDARY to indicate that we do not
+ * require it or any of its callers to be MOZ_CAN_RUN_SCRIPT even if it calls
+ * MOZ_CAN_RUN_SCRIPT functions.
+ *
+ * There are two known holes in invariant 1, apart from the
+ * MOZ_CAN_RUN_SCRIPT_BOUNDARY opt-out:
+ *
+ *  - Functions called via function pointers can be MOZ_CAN_RUN_SCRIPT even if
+ *    their caller is not, because we have no way to determine from the function
+ *    pointer what function is being called.
+ *  - MOZ_CAN_RUN_SCRIPT destructors can happen in functions that are not
+ *    MOZ_CAN_RUN_SCRIPT.
+ *    https://bugzilla.mozilla.org/show_bug.cgi?id=1535523 tracks this.
+ *
+ * Given those invariants we then require that when calling a MOZ_CAN_RUN_SCRIPT
+ * function all refcounted arguments (including "this") satisfy one of three
+ * conditions:
+ *  a) The argument is held via a strong pointer on the stack.
+ *  b) The argument is an argument of the caller (and hence held by a strong
+ *     pointer somewhere higher up the callstack).
+ *  c) The argument is explicitly annotated with MOZ_KnownLive, which indicates
+ *     that something is guaranteed to keep it alive (e.g. it's rooted via a JS
+ *     reflector).
+ */
+
 #include "CanRunScriptChecker.h"
 #include "CustomMatchers.h"
 
 void CanRunScriptChecker::registerMatchers(MatchFinder *AstMatcher) {
   auto Refcounted = qualType(hasDeclaration(cxxRecordDecl(isRefCounted())));
   auto StackSmartPtr =
     ignoreTrivials(
       declRefExpr(to(varDecl(hasAutomaticStorageDuration())),
--- a/mfbt/Attributes.h
+++ b/mfbt/Attributes.h
@@ -537,16 +537,20 @@
  * 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. Note that MOZ_CAN_RUN_SCRIPT
  *   should only be applied to function declarations, not definitions. If you
  *   need to apply it to a definition (eg because both are generated by a macro)
  *   use MOZ_CAN_RUN_SCRIPT_FOR_DEFINITION.
+ *
+ *   MOZ_CAN_RUN_SCRIPT can be applied to XPIDL-generated declarations by
+ *   annotating the method or attribute as [can_run_script] in the .idl file.
+ *
  * MOZ_CAN_RUN_SCRIPT_FOR_DEFINITION: Same as MOZ_CAN_RUN_SCRIPT, but usable on
  *   a definition. If the declaration is in a header file, users of that header
  *   file may not see the annotation.
  * 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