static-check-finalizers
author Benjamin Smedberg <benjamin@smedbergs.us>
Sat, 26 Jul 2008 22:49:39 -0400
changeset 167 a4da40849f5436e629c5732f4368c6c48189637f
parent 145 cce9afb1935af093e0876e643dbcdf9b6ed3b0e6
permissions -rw-r--r--
State as of now

Statically check finalizers to ensure that they don't dereference GC pointers or call non-finalizer-safe methods.

diff --git a/config/config.mk b/config/config.mk
--- a/config/config.mk
+++ b/config/config.mk
@@ -535,6 +535,7 @@ TREEHYDRA_MODULES = \
   $(topsrcdir)/xpcom/analysis/outparams.js \
   $(topsrcdir)/xpcom/analysis/stack.js \
   $(topsrcdir)/xpcom/analysis/gc.js \
+  $(topsrcdir)/xpcom/analysis/finalizers.js \
   $(NULL)
 
 DEHYDRA_ARGS = \
diff --git a/xpcom/analysis/finalizers.js b/xpcom/analysis/finalizers.js
new file mode 100644
--- /dev/null
+++ b/xpcom/analysis/finalizers.js
@@ -0,0 +1,109 @@
+// include("gcc_utils.js");
+include("unstable/lazy_types.js");
+
+const verbose = false;
+let LOG;
+
+if (verbose)
+  LOG = print;
+else
+  LOG = function() { };
+
+String.prototype.beginsWith = function(other) {
+  return this.substr(0, other.length) == other;
+};
+
+function process_tree(fndecl)
+{
+  if (!is_finalizer_safe(fndecl))
+    return;
+
+  function walker(t, stack)
+  {
+    function getLocation() {
+      for (let i = stack.length - 1; i >= 0; --i) {
+        let loc = location_of(stack[i]);
+        if (loc !== undefined)
+          return loc;
+      }
+      return location_of(DECL_SAVED_TREE(fndecl));
+    }
+
+    function check_call() {
+      let fn = callable_arg_function_decl(CALL_EXPR_FN(t));
+      if (!is_finalizer_safe(fn)) {
+        error("Finalizer-safe function " + decl_name(fndecl) +
+              " calling unsafe function " + decl_name(fn),
+              getLocation());
+      }
+    }
+
+    function check_ref() {
+      let expr = dehydra_convert(t.operands()[0]);
+      if (expr.name == 'this')
+        return;
+
+      let type = expr.type;
+      if (!type.isPointer && !type.isReference) {
+        error("Dereferencing type that isn't pointer or reference!",
+              getLocation());
+        return;
+      }
+      if (isGC(type.type)) {
+        error("Dereferencing GC type '" + type.type.name + "' within finalizer-safe function " + decl_name(fndecl),
+              getLocation());
+      }
+    }
+
+    let code = TREE_CODE(t);
+    if (code == CALL_EXPR) {
+      check_call();
+    }
+    else if (INDIRECT_REF_P(t)) {
+      check_ref();
+    }
+  }
+
+  let umap = new Map();
+  walk_tree(DECL_SAVED_TREE(fndecl), walker, umap);
+}
+
+function is_finalizer_safe(fndecl)
+{
+  LOG("is_finalizer_safe(" + decl_name(fndecl) + "): ");
+
+  fndecl = dehydra_convert(fndecl);
+  if (hasAttribute(fndecl, 'NS_No_Finalizer')) {
+    LOG("false: has NS_No_Finalizer");
+    return false;
+  }
+
+  if (hasAttribute(fndecl, 'NS_Finalizer')) {
+    LOG("true: has NS_Finalizer");
+    return true;
+  }
+
+  /* Destructors of GC classes must be finalizer safe */
+  if (fndecl.memberOf && fndecl.isDestructor && isGC(fndecl.memberOf)) {
+    LOG("true: is destructor of GC class");
+    return true;
+  }
+
+  /* If a function is declared within our source tree except for NSPR,
+   * it's assumed not to be finalizer-safe. Other functions are assumed
+   * to be finalizer-safe. */
+  let fnloc = fndecl.loc;
+  if (fnloc.file.indexOf('nspr') != -1) {
+    LOG("true: Is NSPR");
+    return true;
+  }
+
+  if (fnloc.file.beginsWith(options.topsrcdir) ||
+      fnloc.file.beginsWith(options.objdir)) {
+    LOG("false: Declared in Mozilla sources");
+    return false;
+  }
+
+  LOG("true: Not declared in Mozilla sources");
+  return true;
+}
diff --git a/xpcom/analysis/gc.js b/xpcom/analysis/gc.js
--- a/xpcom/analysis/gc.js
+++ b/xpcom/analysis/gc.js
@@ -178,6 +178,8 @@ function leftmostFinalizable(c)
   return c.leftmostFinalizable;
 }
 
+global.isGC = isGC;
+
 /**
  * is MMGc::GCFinalizable anywhere in this class lineage?
  */
diff --git a/xpcom/analysis/static-checking.js b/xpcom/analysis/static-checking.js
--- a/xpcom/analysis/static-checking.js
+++ b/xpcom/analysis/static-checking.js
@@ -26,7 +26,8 @@ function LoadModules(modulelist)
 
   let modulenames = modulelist.split(',');
   for each (let modulename in modulenames) {
-    let module = { __proto__: this };
+    let module = { __proto__: this,
+		   global: this };
     include(modulename, module);
     modules.push(module);
   }
diff --git a/xpcom/tests/static-checker/Makefile.in b/xpcom/tests/static-checker/Makefile.in
--- a/xpcom/tests/static-checker/Makefile.in
+++ b/xpcom/tests/static-checker/Makefile.in
@@ -112,10 +112,16 @@ OUTPARAMS_PASS_TESTCASES = \
   o13.cpp \
   $(NULL)
 
+FINALIZER_FAILURE_TESTCASES = \
+  TestFinalizer1.cpp \
+  TestFinalizer2.cpp \
+  $(NULL)
+
 STATIC_FAILURE_TESTCASES = \
   $(GC_FAILURE_TESTCASES) \
   $(FINAL_FAILURE_TESTCASES) \
   $(STACK_FAILURE_TESTCASES) \
+  $(FINALIZER_FAILURE_TESTCASES) \
   $(NULL)
 
 STATIC_WARNING_TESTCASES = \
diff --git a/xpcom/tests/static-checker/TestFinalizer1.cpp b/xpcom/tests/static-checker/TestFinalizer1.cpp
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/static-checker/TestFinalizer1.cpp
@@ -0,0 +1,19 @@
+#include "nsISupports.h"
+#include "nsIArray.h"
+#include "nsCOMPtr.h"
+
+void NS_NO_FINALIZER testfunc();
+
+class A : XPCOMGCFinalizedObject, nsISupports
+{
+  A(nsIArray *a);
+  ~A();
+  nsIArray *const mArray;
+};
+
+A::~A()
+{
+  // The following call is statically incorrect (calling a no-finalizer method)
+  testfunc();
+}
+
diff --git a/xpcom/tests/static-checker/TestFinalizer2.cpp b/xpcom/tests/static-checker/TestFinalizer2.cpp
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/static-checker/TestFinalizer2.cpp
@@ -0,0 +1,20 @@
+#include "nsISupports.h"
+#include "plstr.h"
+
+struct NS_GC_TYPE B : public XPCOMGCObject
+{
+  int a;
+};
+
+class A : public XPCOMGCFinalizedObject, MMgc::GCFinalizable
+{
+  ~A();
+
+  B *mB;
+};
+
+A::~A()
+{
+  // The following call is statically incorrect (dereferencing a GC object)
+  mB->a = 1;
+}