Boy, static checking is hard!
authorBenjamin Smedberg <benjamin@smedbergs.us>
Fri, 02 May 2008 23:11:04 -0400
changeset 120 8efda5af99c6144e195fdab2ad22e5ef9426523c
parent 119 644dd0dd8cc46e1e3bed267850f717306b42d9ba
child 121 bed93b2f797b693ee39558d7bfe3410b1deaaa98
push id11
push userbsmedberg@mozilla.com
push dateMon, 05 May 2008 14:53:12 +0000
Boy, static checking is hard!
static-check-gc-attributes
--- a/static-check-gc-attributes
+++ b/static-check-gc-attributes
@@ -1,28 +1,84 @@
-Add basic static checking of gc types and correct inheritance of GCFinalizable.
+Add basic static checking of gc types and correct inheritance of GCFinalizable. I found out that it's not actually catching 90% of the stack classes, for reasons I haven't figured out. This patch actually kinda sucks, so I'm going to go back to the beginning and make this entire script pure treehydra.
 
+diff --git a/xpcom/base/nscore.h b/xpcom/base/nscore.h
+--- a/xpcom/base/nscore.h
++++ b/xpcom/base/nscore.h
+@@ -496,17 +496,31 @@ typedef PRUint32 nsrefcnt;
+  *
+  * NS_STACK_CLASS: a class which must only be instantiated on the stack
+  * NS_FINAL_CLASS: a class which may not be subclassed
++ * NS_GC_TYPE    : a class which must be GC-allocated
++ * NS_NOGC_TYPE  : a class which must not be GC-allocated
++ * NS_MAYBEGC    : A type which may be part of a GC object which is not
++ *                 finalized. Classes/structs without explicit or implied
++ *                 destructors (POD types) are automatically assumed to be
++ *                 NS_MAYBEGC. The destructor of this type, if any, must be
++ *                 finalizer safe.
++ * NS_MAYBEGCFINALIZED: The destructor of such a type is GC-safe, and is not
++ *                 optional: GC type which has a maybegcfinalized member must
++ *                 be finalized.
+  */
+ #ifdef NS_STATIC_CHECKING
+ #define NS_STACK_CLASS __attribute__((user("NS_stack")))
+ #define NS_FINAL_CLASS __attribute__((user("NS_final")))
+ #define NS_GC_TYPE     __attribute__((user("NS_GC")))
+ #define NS_NOGC_TYPE   __attribute__((user("NS_NoGC")))
++#define NS_MAYBEGC     __attribute__((user("NS_MaybeGC")))
++#define NS_MAYBEGCFINALIZED __attribute__((user("NS_MaybeGCFinalized")))
+ #else
+ #define NS_STACK_CLASS
+ #define NS_FINAL_CLASS
+ #define NS_GC_TYPE
+ #define NS_NOGC_TYPE
++#define NS_MAYBEGC
++#define NS_MAYBEGCFINALIZED
+ #endif
+ 
+ #endif /* nscore_h___ */
 diff --git a/xpcom/static-checking.js b/xpcom/static-checking.js
 --- a/xpcom/static-checking.js
 +++ b/xpcom/static-checking.js
-@@ -10,6 +10,13 @@
+@@ -10,6 +10,22 @@
   *                 subclassed
   * .stack = true   if the class has been annotated as a class which may only
   *                 be instantiated on the stack
 + * .gc    = true   if the class has been annotated as a class which may only
 + *                 exist in the GC heap.
 + * .gcfinalizable = true
 + *                 if the left-most base of this object is MMgc::GCFinalizable
 + *                 exluding XPCOMGCFinalizedObject
-+ * .finalized = true
++ * .xpcomgcfinalizedobject = true
 + *                 if the object derives from XPCOMGCFinalizedObject
++ * .xpcomgcobject = true
++ *                 if the object derives from XPCOMGCObject
++ * .maybegc = true
++ *                 if the class has been annotated as a class which may be a
++ *                 member of a non-finalized GC object, *or* has no destructor,
++ *                 explicit or implied
++ * .maybegcfinalized = true
++ *                 if the class has been annotated as a class which, if it is a
++ *                 member of a GC object, that object must be finalized.
   */
  var gClassMap = {};
  
-@@ -149,6 +156,56 @@
+@@ -21,6 +37,11 @@ ClassType.prototype = {
+ ClassType.prototype = {
+   isFinal: false,
+   stack: false,
++  gc: false,
++  gcfinalizable: false,
++  finalized: false,
++  gcfinalized: false,
++  maybegcfinalized: false
+ };
+ 
+ function process_type(c)
+@@ -149,6 +170,101 @@ function get_class(c, allowIncomplete)
      }
    }
  
 +  // check for .gc
 +
 +  if (hasAttribute('NS_GC') ||
 +      c.name == "XPCOMGCObject" ||
 +      c.name == "XPCOMGCFinalizedObject") {
@@ -66,47 +122,172 @@ diff --git a/xpcom/static-checking.js b/
 +    for each (base in bases) {
 +      if (base.xpcomgcfinalizedobject) {
 +        classattr.xpcomgcfinalizedobject = true;
 +        break;
 +      }
 +    }
 +  }
 +
++  // check for .xpcomgcobject
++
++  if (c.name == 'XPCOMGCObject') {
++    classattr.xpcomgcobject = true;
++  }
++  else {
++    for each (base in bases) {
++      if (base.xpcomgcobject) {
++	classattr.xpcomgcobject = true;
++	break;
++      }
++    }
++  }
++
++  // Check for .maybegc
++
++  if (classattr.gc ||
++      hasAttribute('NS_MaybeGC'))
++    classattr.maybegc = true;
++  else if (hasDestructor(c))
++    classattr.maybegc = false;
++  else {
++    let maybegc = true;
++    for (let t in iter_subtypes(c)) {
++      if (!get_class(t, false).maybegc) {
++	maybegc = false;
++	break;
++      }
++    }
++    classattr.maybegc = maybegc;
++  } 
++
++  // Check for .maybegcfinalized
++
++  if (hasAttribute('NS_MaybeGCFinalized'))
++    classattr.maybegcfinalized = true;
++  else {
++    for (let t in iter_subtypes(c)) {
++      if (get_class(t, false).maybegcfinalized) {
++	classattr.maybegcfinalized = true;
++	break;
++      }
++    }
++  }
++
    // Check for errors at declaration-time
  
    for each (base in bases) {
-@@ -171,6 +228,16 @@
-     if (!foundConstructor) {
-       warning(c.loc + ": class " + c.name + " is marked stack-only but doesn't have a constructor. Static checking can't detect instantiations of this class properly.");
+@@ -173,18 +289,71 @@ function get_class(c, allowIncomplete)
      }
-+  }
-+
+   }
+ 
 +  if (classattr.gc && classattr.stack) {
-+    error("class '" + c.name + "' annotated for both stack and GC allocation.");
++    error(c.loc + ": class '" + c.name + "' annotated for both stack and GC allocation.");
 +  }
 +
 +  if (classattr.xpcomgcfinalizedobject &&
 +      !classattr.gcfinalizable &&
 +      c.name != "XPCOMGCFinalizedObject") {
-+    error("class '" + c.name + "' is XPCOMGCFinalizedObject but not GCFinalizable.");
-   }
++    error(c.loc + ": class '" + c.name + "' is XPCOMGCFinalizedObject but not GCFinalizable.");
++  }
++
++  if (hasAttribute('NS_MaybeGCFinalized') &&
++      !hasDestructor(c)) {
++    error(c.loc + ": class '" + c.name + "' is MaybeGCFinalized but doesn't have a destructor.");
++  }
++
+   return classattr;
+ }
  
-   return classattr;
-@@ -200,6 +267,18 @@
+ /**
+- * Unwrap any array of types back to their base type.
++ * Iterate over the bases of a type as well as any member variables which are
++ * of record type.
+  */
+-function unwrapArray(t)
++function iter_subtypes(c)
+ {
+-  while (t.isArray) {
+-    t = t.type;
++  if (!(c.kind == 'class' || c.kind == 'struct'))
++    throw Error('iter_components called on a non-record type');
++
++  for each (let base in c.bases)
++    yield base;
++
++  for each (let member in c.members) {
++    let t = unwrapSubtype(member.type);
++    if (t.kind == 'class' || t.kind == 'struct')
++      yield t;
+   }
+-  return t;
++}
++
++/**
++ * Does this type have an explicit destructor?
++ */ 
++function hasDestructor(c)
++{
++  if (!(c.kind == 'class' || c.kind == 'struct'))
++    throw Error('hasDestructor called on a non-record type ' + c);
++
++  for each (let member in c.members)
++    if (member.name[0] == "~")
++      return true;
++
++  return false;
++}
++
++/**
++ * Unwrap any typedef or array of types back to their base type.
++ */
++function unwrapSubtype(t)
++{
++  while (true) {
++    if (t.isArray)
++      t = t.type;
++    else if (t.typedef)
++      t = t.typedef;
++    else
++      return t;
++  }
+ }
+ 
+ function process_function(f, stmts)
+@@ -200,12 +369,32 @@ function process_function(f, stmts)
  
    function processVar(v)
    {
+-    if (v.isConstructor &&
+-        v.fieldOf &&
+-        get_class(v.methodOf, false).stack &&
+-        v.fieldOf.type.isPointer) {
+-      error(getLocation() + ": constructed object of type '" +
+-            v.methodOf.name + "' not on the stack.");
 +    if (v.isConstructor) {
-+      if (get_class(v.methodOf, false).gc) {
++      let c = get_class(unwrapSubtype(v.methodOf), false);
++      if (c.gc) {
 +        if (v.isReturn ||
 +            !v.fieldOf ||
-+            (!v.fieldOf.isPointer &&
++            (!v.fieldOf.type.isPointer &&
 +             (v.fieldOf.fieldOf &&
 +              v.fieldOf.fieldOf.name != 'this'))) {
 +          error(getLocation() + ": class '" + v.methodOf.name + "' allocated on the stack.");
++	}
++	else {
++	  warning(getLocation() + ": constructor of GC type ignored: v = " + v);
 +        }
-+      }
-+    }
 +
-     if (v.isConstructor &&
-         v.fieldOf &&
-         get_class(v.methodOf, false).stack &&
++	if (c.maybegcfinalized &&
++	    !c.xpcomgcfinalizedobject) {
++	  error(getLocation() + ": class '" + v.methodOf.name + "' must not be instantiated, because it must be a finalized object.");
++	}
++      }
++
++      if (v.fieldOf &&
++	  c.stack &&
++          v.fieldOf.type.isPointer) {
++	error(getLocation() + ": constructed object of type '" +
++              v.methodOf.name + "' not on the stack.");
++      }
+     }
+   }
+