Save state. I have an iloop in the storage-classes analysis which is going to require some real work to fix.
authorBenjamin Smedberg <benjamin@smedbergs.us>
Thu, 17 Jul 2008 10:30:17 -0400
changeset 157 8daebed024b6
parent 156 1ad802eb2712
child 158 3bc262b4ce40
push id33
push userbsmedberg@mozilla.com
push dateThu, 17 Jul 2008 14:30:32 +0000
Save state. I have an iloop in the storage-classes analysis which is going to require some real work to fix.
nsCSSShadowArray-refcounting
nsCSSValue-Array-comptr
nsHashtable-annotation
nsSegmentedBuffer-allocator-unmanaged
proxy-create-namespace
proxy-create-temporary-hack
series
static-check-storage-classes
static-check-storage-classes2
storage-class-annotations
new file mode 100644
--- /dev/null
+++ b/nsCSSShadowArray-refcounting
@@ -0,0 +1,62 @@
+diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp
+--- a/layout/style/nsStyleStruct.cpp
++++ b/layout/style/nsStyleStruct.cpp
+@@ -1559,15 +1559,12 @@ nsChangeHint nsStyleTextReset::MaxDiffer
+ // nsCSSShadowItem
+ //
+ 
+-nsrefcnt
+-nsCSSShadowArray::Release()
++void
++nsCSSShadowArray::ReleaseReference()
+ {
+-  mRefCnt--;
+-  if (mRefCnt == 0) {
++  if (--mRefCnt == 0) {
+     delete this;
+-    return 0;
+   }
+-  return mRefCnt;
+ }
+ 
+ static nsChangeHint
+diff --git a/layout/style/nsStyleStruct.h b/layout/style/nsStyleStruct.h
+--- a/layout/style/nsStyleStruct.h
++++ b/layout/style/nsStyleStruct.h
+@@ -61,6 +61,7 @@
+ #include "nsIAtom.h"
+ #include "nsIURI.h"
+ #include "nsCSSValue.h"
++#include "mozilla/RefPtr.h"
+ 
+ class nsIFrame;
+ class imgIRequest;
+@@ -364,8 +365,8 @@ class nsCSSShadowArray {
+       }
+     }
+ 
+-    nsrefcnt AddRef() { return ++mRefCnt; }
+-    nsrefcnt Release();
++    void AddReference() { ++mRefCnt; }
++    void ReleaseReference();
+ 
+     PRUint32 Length() const { return mLength; }
+     nsCSSShadowItem* ShadowAt(PRUint32 i) {
+@@ -418,7 +419,7 @@ struct nsStyleBorder {
+   nsStyleSides  mBorderRadius;    // [reset] length, percent
+   PRUint8       mFloatEdge;       // [reset] see nsStyleConsts.h
+   nsBorderColors** mBorderColors; // [reset] multiple levels of color for a border.
+-  nsRefPtr<nsCSSShadowArray> mBoxShadow; // [reset] NULL for 'none'
++  mozilla::RefPtr<nsCSSShadowArray> mBoxShadow; // [reset] NULL for 'none'
+ 
+   void EnsureBorderColors() {
+     if (!mBorderColors) {
+@@ -774,7 +775,7 @@ struct nsStyleText {
+   nsStyleCoord  mTextIndent;            // [inherited] 
+   nsStyleCoord  mWordSpacing;           // [inherited] 
+ 
+-  nsRefPtr<nsCSSShadowArray> mTextShadow; // [inherited] NULL in case of a zero-length
++  mozilla::RefPtr<nsCSSShadowArray> mTextShadow; // [inherited] NULL in case of a zero-length
+   
+   PRBool WhiteSpaceIsSignificant() const {
+     return mWhiteSpace == NS_STYLE_WHITESPACE_PRE ||
new file mode 100644
--- /dev/null
+++ b/nsCSSValue-Array-comptr
@@ -0,0 +1,12 @@
+diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp
+--- a/layout/style/nsCSSParser.cpp
++++ b/layout/style/nsCSSParser.cpp
+@@ -6478,7 +6478,7 @@ nsCSSValueList* CSSParserImpl::ParseCSSS
+     nsCSSUnit unit = cur->mValue.GetUnit();
+     if (unit != eCSSUnit_None && unit != eCSSUnit_Inherit &&
+         unit != eCSSUnit_Initial) {
+-      nsRefPtr<nsCSSValue::Array> val = nsCSSValue::Array::Create(5);
++      nsCSSValue::Array* val = nsCSSValue::Array::Create(5);
+       if (!val) {
+         aErrorCode = NS_ERROR_OUT_OF_MEMORY;
+         break;
new file mode 100644
--- /dev/null
+++ b/nsHashtable-annotation
@@ -0,0 +1,12 @@
+diff --git a/xpcom/ds/nsHashtable.h b/xpcom/ds/nsHashtable.h
+--- a/xpcom/ds/nsHashtable.h
++++ b/xpcom/ds/nsHashtable.h
+@@ -236,7 +236,7 @@ class NS_COM nsSupportsHashtable
+ 
+ class NS_COM nsISupportsKey : public nsHashKey {
+   protected:
+-    nsISupports* mKey;
++    nsISupports* mKey NS_UNMANAGED;
+     
+   public:
+     nsISupportsKey(const nsISupportsKey& aKey) : mKey(aKey.mKey) {
new file mode 100644
--- /dev/null
+++ b/nsSegmentedBuffer-allocator-unmanaged
@@ -0,0 +1,56 @@
+diff --git a/xpcom/io/nsSegmentedBuffer.cpp b/xpcom/io/nsSegmentedBuffer.cpp
+--- a/xpcom/io/nsSegmentedBuffer.cpp
++++ b/xpcom/io/nsSegmentedBuffer.cpp
+@@ -199,7 +199,8 @@ TestSegmentedBuffer()
+     NS_ASSERTION(!empty, "DeleteFirstSegment failed");
+     empty = buf->DeleteFirstSegment();
+     NS_ASSERTION(empty, "DeleteFirstSegment failed");
+-    delete buf;
++    // TODO: we know this is dead, but deleting it is hard?
++    // delete buf;
+ }
+ #endif
+ 
+diff --git a/xpcom/io/nsSegmentedBuffer.h b/xpcom/io/nsSegmentedBuffer.h
+--- a/xpcom/io/nsSegmentedBuffer.h
++++ b/xpcom/io/nsSegmentedBuffer.h
+@@ -40,22 +40,25 @@
+ 
+ #include "nsMemory.h"
+ #include "prclist.h"
++#include "nsCOMPtr.h"
+ 
+-class nsSegmentedBuffer
+-{
++class nsSegmentedBuffer{
+ public:
+     nsSegmentedBuffer()
+         : mSegmentSize(0), mMaxSize(0), 
+-          mSegAllocator(nsnull), mSegmentArray(nsnull),
++          mSegAllocator(nsnull),
++          mSegmentArray(nsnull),
+           mSegmentArrayCount(0),
+           mFirstSegmentIndex(0), mLastSegmentIndex(0) {}
+ 
+     ~nsSegmentedBuffer() {
+         Empty();
+-        NS_IF_RELEASE(mSegAllocator);
+     }
+ 
+-
++    /**
++     * @note: if this class is malloced, allocator will not be rooted here
++     * and must be externally rooted.
++     */
+     NS_COM nsresult Init(PRUint32 segmentSize, PRUint32 maxSize,
+                   nsIMemory* allocator = nsnull);
+ 
+@@ -105,7 +108,7 @@ protected:
+ protected:
+     PRUint32            mSegmentSize;
+     PRUint32            mMaxSize;
+-    nsIMemory*       mSegAllocator;
++    nsIMemory*          mSegAllocator NS_UNMANAGED;
+     char**              mSegmentArray;
+     PRUint32            mSegmentArrayCount;
+     PRInt32             mFirstSegmentIndex;
new file mode 100644
--- /dev/null
+++ b/proxy-create-namespace
@@ -0,0 +1,17 @@
+diff --git a/xpcom/proxy/tests/proxy-create-threadsafety.cpp b/xpcom/proxy/tests/proxy-create-threadsafety.cpp
+--- a/xpcom/proxy/tests/proxy-create-threadsafety.cpp
++++ b/xpcom/proxy/tests/proxy-create-threadsafety.cpp
+@@ -60,6 +60,8 @@
+ #include "nsServiceManagerUtils.h"
+ #include "nsThreadUtils.h"
+ #include "nsISupportsUtils.h"
++
++namespace proxy_create_threadsafety {
+ 
+ /*
+ 
+@@ -250,3 +252,4 @@ main(int argc, char **argv)
+     return 0;
+ }
+ 
++} // namespace
new file mode 100644
--- /dev/null
+++ b/proxy-create-temporary-hack
@@ -0,0 +1,45 @@
+diff --git a/xpcom/proxy/tests/proxy-create-threadsafety.cpp b/xpcom/proxy/tests/proxy-create-threadsafety.cpp
+--- a/xpcom/proxy/tests/proxy-create-threadsafety.cpp
++++ b/xpcom/proxy/tests/proxy-create-threadsafety.cpp
+@@ -61,8 +61,6 @@
+ #include "nsThreadUtils.h"
+ #include "nsISupportsUtils.h"
+ 
+-namespace proxy_create_threadsafety {
+-
+ /*
+ 
+ A quick diagram of how this test works:
+@@ -92,6 +90,8 @@ 1		proxy event object released
+ /***************************************************************************/
+ /* ProxyTest                                                               */
+ /***************************************************************************/
++
++namespace proxy_create_threadsafety {
+ 
+ class ProxyTest : public nsIRunnable,
+                   public nsITestProxy,
+@@ -232,6 +232,8 @@ private:
+     nsCOMPtr<nsIThread> mThreadTwo;
+ };
+ 
++} // namespace
++
+ int
+ main(int argc, char **argv)
+ {
+@@ -243,7 +245,7 @@ main(int argc, char **argv)
+         NS_GetComponentRegistrar(&registrar);
+         registrar->AutoRegister(nsnull);
+ 
+-        nsITestProxy* tester = new ProxyTest();
++        nsITestProxy* tester = new proxy_create_threadsafety::ProxyTest();
+         tester->Test(0, 0, nsnull);
+     }
+ 
+@@ -251,5 +253,3 @@ main(int argc, char **argv)
+ 
+     return 0;
+ }
+-
+-} // namespace
--- a/series
+++ b/series
@@ -183,8 +183,20 @@ nsTransitionKey-sucky-hack
 WalkState-okonstack
 nsMorkReader-stackclass
 LazyMessage-okonstack
 PendingLookup-constmembers
 nsProxiedService-stackclass
 nsNSSCertificate-nostack
 workaround-TestProxies-garburator-crash
 BookmarkImportFrame-notonstack
+automatic-garburator
+automatic-gcobject
+automatic-remove-addrefs
+nsCSSValue-Array-comptr
+nsCSSShadowArray-refcounting
+static-check-storage-classes
+storage-class-annotations
+proxy-create-namespace
+proxy-create-temporary-hack
+nsHashtable-annotation
+nsSegmentedBuffer-allocator-unmanaged
+static-check-storage-classes2
new file mode 100644
--- /dev/null
+++ b/static-check-storage-classes
@@ -0,0 +1,671 @@
+Storage-class analysis is getting more precise. See http://wiki.mozilla.org/XPCOMGC/Static_Checker for some readable descriptions.
+
+diff --git a/xpcom/analysis/stack.js b/xpcom/analysis/stack.js
+--- a/xpcom/analysis/stack.js
++++ b/xpcom/analysis/stack.js
+@@ -1,73 +1,5 @@ include("gcc_util.js");
+ include("gcc_util.js");
+ include("unstable/lazy_types.js");
+-
+-function process_type(c)
+-{
+-  if ((c.kind == 'class' || c.kind == 'struct') &&
+-      !c.isIncomplete)
+-    isStack(c);
+-}
+-
+-function isStack(c)
+-{
+-  function calculate()
+-  {
+-    if (hasAttribute(c, 'NS_stack'))
+-      return true;
+-
+-    for each (let base in c.bases)
+-      if (isStack(base))
+-        return true;
+-
+-    for each (let member in c.members) {
+-      if (member.isFunction)
+-        continue;
+-
+-      let type = member.type;
+-      while (true) {
+-        if (type === undefined)
+-          break;
+-
+-        if (type.isArray) {
+-          type = type.type;
+-          continue;
+-        }
+-
+-        if (type.typedef) {
+-          if (hasAttribute(type, 'NS_stack'))
+-            return true;
+-
+-          type = type.typedef;
+-          continue;
+-        }
+-        break;
+-      }
+-
+-      if (type === undefined) {
+-        warning("incomplete type for member " + member + ".", member.loc);
+-        continue;
+-      }
+-
+-      if (type.isPointer || type.isReference)
+-        continue;
+-
+-      if (!type.kind || (type.kind != 'class' && type.kind != 'struct'))
+-        continue;
+-
+-      if (isStack(type))
+-        return true;
+-    }
+-    return false;
+-  }
+-
+-  if (c.isIncomplete)
+-    throw Error("Can't get stack property for incomplete type.");
+-
+-  if (!c.hasOwnProperty('isStack'))
+-    c.isStack = calculate();
+-
+-  return c.isStack;
+-}
+ 
+ function isVoidPtr(t)
+ {
+@@ -95,18 +27,18 @@ function operator_new_assign(stmt)
+       let name = IDENTIFIER_POINTER(nameid);
+       if (name == "operator new" || name == "operator new []") {
+         // if this is placement-new, ignore it (should we issue a warning?)
+-        let fncallobj = dehydra_convert(TREE_TYPE(fncall));
+-        if (fncallobj.parameters.length == 2 &&
+-            isVoidPtr(fncallobj.parameters[1]))
+-          return null;
++        let fncallobj = dehydra_convert(fncall);
++        if (fncallobj.type.parameters.length == 2 &&
++            isVoidPtr(fncallobj.type.parameters[1]))
++          return [null, null];
+ 
+-        return varDecl;
++        return [varDecl, fncallobj];
+       }
+     }
+   }
+   catch (e if e.TreeCheckError) { }
+ 
+-  return null;
++  return [null, null];
+ }
+ 
+ function process_tree(fndecl)
+@@ -129,7 +61,7 @@ function process_tree(fndecl)
+       return location_of(DECL_SAVED_TREE(fndecl));
+     }
+     
+-    function check_opnew_assignment(varDecl, stmt)
++    function check_opnew_assignment(varDecl, fncallobj, stmt)
+     {
+       if (TREE_CODE(stmt) != GIMPLE_MODIFY_STMT) {
+         warning("operator new not followed by a GIMPLE_MODIFY_STMT: " + TREE_CODE(stmt), getLocation(stmt));
+@@ -151,9 +83,69 @@ function process_tree(fndecl)
+         return;
+       }
+       destType = destType.type;
++      if (destType.kind === undefined ||
++          (destType.kind != 'struct' &&
++           destType.kind != 'class'))
++        return;
+ 
+-      if (isStack(destType))
+-        error("constructed object of type '" + destType.name + "' not on the stack.", getLocation(stmt));
++      // Figure out what operator new is being called and determine whether
++      // it's a mallocation, GCAllocation, or GCFinalizedAllocation
++      let allocType;
++      if (fncallobj.memberOf === undefined) {
++        allocType = 'malloc';
++      }
++      else {
++        let cx = fncallobj.memberOf;
++        if (cx.name == 'MMgc::GCObject' ||
++            cx.name == 'XPCOMGCObject') {
++          allocType = 'gc';
++        }
++        else if (cx.name == 'MMgc::GCFinalizedObject' ||
++                 cx.name == 'XPCOMGCFinalizedObject') {
++          allocType = 'gcfinalized';
++        }
++        else if (cx.name == 'MMgc::GCRoot') {
++          allocType = 'malloc';
++        }
++        else if (hasAttribute(fncallobj, 'NS_mallocator')) {
++          allocType = 'malloc';
++        }
++        else if (hasAttribute(fncallobj, 'NS_gcallocator')) {
++          allocType = 'gc';
++        }
++        else if (hasAttribute(fncallobj, 'NS_gcfinalizedallocator')) {
++          allocType = 'gcfinalized';
++        }
++        else {
++          error("Call to un-annotated %s\n%s:   declared here".format(fncallobj.name, fncallobj.loc), getLocation(stmt));
++          return;
++        }
++      }
++
++      let m;
++      let sc = getStorageClass(destType);
++      switch (allocType) {
++      case 'malloc':
++        m = sc.blockMalloc();
++        if (m)
++          error("constructed object of type %s on the malloc heap\n%s".format(destType.name, m), getLocation(stmt));
++        break;
++      case 'gc':
++        m = sc.blockGC();
++        if (m)
++          error("constructed object of type %s on the GC heap\n%s".format(destType.name, m), getLocation(stmt));
++        m = sc.canSkipFinalization();
++        if (m)
++          error("constructed object of type %s without finalization\n%s".format(destType.name, m), getLocation(stmt));
++        break;
++      case 'gcfinalized':
++        m = sc.blockGC();
++        if (m)
++          error("constructed object of type %s on the GC heap\n%s".format(destType.name, m), getLocation(stmt));
++        break;
++      default:
++        throw Error("analysis error: allocType was unrecognized value '%s'".format(allocType));
++      }
+     }
+ 
+     if (TREE_CODE(t) != STATEMENT_LIST)
+@@ -162,11 +154,12 @@ function process_tree(fndecl)
+     // if we find a tuple of "operator new" / GIMPLE_MODIFY_STMT casting
+     // the result of operator new to a pointer type
+     let opnew = null;
++    let fncallobj;
+     for (let stmt in iter_statement_list(t)) {
+       if (opnew != null)
+-        check_opnew_assignment(opnew, stmt);
++        check_opnew_assignment(opnew, fncallobj, stmt);
+ 
+-      opnew = operator_new_assign(stmt);
++      [opnew, fncallobj] = operator_new_assign(stmt);
+     }
+ 
+     if (opnew != null)
+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
+@@ -10,6 +10,65 @@ function treehydra_enabled() {
+   return this.hasOwnProperty('TREE_CODE');
+ }
+ 
++String.prototype.format = function string_format() {
++  // there are two modes of operation... unnamed indices are read in order;
++  // named indices using %(name)s. The two styles cannot be mixed.
++  // Unnamed indices can be passed as either a single argument to this function,
++  // multiple arguments to this function, or as a single array argument
++  let curindex = 0;
++  let d;
++  
++  if (arguments.length > 1) {
++    d = arguments;
++  }
++  else
++    d = arguments[0];
++  
++  function r(s, key, type) {
++    let v;
++    if (key == "") {
++      if (curindex == -1)
++        throw Error("Cannot mix named and positional indices in string formatting.");
++
++      if (curindex == 0 && (!(d instanceof Object) || !(0 in d))) {
++        v = d;
++      }
++      else if (!(curindex in d))
++        throw Error("Insufficient number of items in format, requesting item %i".format(curindex));
++      else {
++        v = d[curindex];
++      }
++
++      ++curindex;
++    }
++    else {
++      key = key.slice(1, -1);
++      if (curindex > 0)
++        throw Error("Cannot mix named and positional indices in string formatting.");
++      curindex = -1;
++      
++      if (!(key in d))
++        throw Error("Key '%s' not present during string substitution.".format(key));
++      v = d[key];
++    }
++    switch (type) {
++    case "s":
++      return v.toString();
++    case "r":
++      return uneval(v);
++    case "i":
++      return parseInt(v);
++    case "f":
++      return Number(v);
++    case "%":
++      return "%";
++    default:
++      throw Error("Unexpected format character '%s'.".format(type));
++    }
++  }
++  return this.replace(/%(\([^)]+\))?(.)/g, r);
++};
++               
+ include('unstable/getopt.js');
+ [options, args] = getopt();
+ 
+@@ -43,18 +102,24 @@ function process_type(c)
+       module.process_type(c);
+ }
+ 
++function hasAnyAttribute(c, attrlist)
++{
++  if (c.attributes === undefined)
++    return false;
++  
++  for each (let attr in c.attributes) {
++    if (attr.name == 'user' && attrlist.some(function(v) {
++      return v == attr.value[0];
++    }))
++      return attr.value[0];
++  }
++  
++  return false;
++}
++
+ function hasAttribute(c, attrname)
+ {
+-  var attr;
+-
+-  if (c.attributes === undefined)
+-    return false;
+-
+-  for each (attr in c.attributes)
+-    if (attr.name == 'user' && attr.value[0] == attrname)
+-      return true;
+-
+-  return false;
++  return hasAnyAttribute(c, [attrname]) != false;
+ }
+ function process_function(f, stmts)
+ {
+@@ -83,3 +148,307 @@ function input_end()
+     if (module.hasOwnProperty('input_end'))
+       module.input_end();
+ }
++
++function StorageReason(loc, message, prev)
++{
++  this.loc = loc;
++  this.message = message;
++  this.prev = prev;
++}
++StorageReason.prototype.toString = function()
++{
++  let str = "%s:   %s".format(loc_string(this.loc), this.message);
++  if (this.prev)
++    str += "\n%s".format(this.prev);
++  return str;
++};
++
++/**
++ * The various Storage types have a common interface:
++ * blockStack()
++ * blockGlobal()
++ * blockMalloc()
++ * blockGC()
++ * safeFinalizer()
++ * canSkipFinalization()
++ * 
++ * each of these returns null if everything's ok, or a StorageReason
++ */
++ValueStorageClass =  {
++  blockStack: function() { return null; },
++  blockGlobal: function() { return null; },
++  blockMalloc: function() { return null; },
++  blockGC: function() { return null; },
++  safeFinalizer: function() { return null; },
++  canSkipFinalization: function() { return null; }
++};
++
++function PointerStorageClass(type)
++{
++  this._type = type;
++}
++PointerStorageClass.prototype = {
++  blockNotManaged: function() {
++    let subtypeClass = getStorageClass(this._type.type);
++    if (!subtypeClass.blockGC()) {
++      let reason = subtypeClass.blockMalloc();
++      if (reason)
++        return new StorageReason(this._type.loc, "pointer to", reason);
++    }
++    return null;
++  },
++  
++  blockStack: function() { return null; },
++  blockGlobal: function() { return this.checkNotManaged(); },
++  blockMalloc: function() { return this.checkNotManaged(); },
++  blockGC: function() { return null; },
++  safeFinalizer: function() { return null; },
++  canSkipFinalization: function() { return null; }
++};
++
++function ArrayStorageClass(type)
++{
++  this._type = type;
++}
++ArrayStorageClass.prototype = {
++  /* Just forward everything to the inner... */
++  blockStack: function() { return getStorageClass(this._type).blockStack(); },
++  blockGlobal: function() { return getStorageClass(this._type).blockGlobal(); },
++  blockMalloc: function() { return getStorageClass(this._type).blockMalloc(); },
++  blockGC: function() { return getStorageClass(this._type).blockGC(); },
++  safeFinalizer: function() { return getStorageClass(this._type).safeFinalizer(); },
++  canSkipFinalization: function() { return getStorageClass(this._type).canSkipFinalization(); }
++};
++
++function TypedefStorageClass(type)
++{
++  this._type = type;
++}
++TypedefStorageClass.prototype = {
++  blockStack: function() {
++    let m = hasAnyAttribute(this._type, ["NS_GC"]);
++    if (m != null)
++      return new StorageReason(this._type.loc, "typedef %s marked with annotation %s".format(this._type.name, m));
++    return getStorageClass(this._type).blockStack();
++  },
++  
++  blockGlobal: function() {
++    let m = hasAnyAttribute(this._type, ["NS_stack", "NS_managed", "NS_GC"]);
++    if (m != null)
++      return new StorageReason(this._type.loc, "typedef %s marked with annotation %s".format(this._type.name, m));
++    return getStorageClass(this._type).blockGlobal();
++  },
++  
++  blockMalloc: function() {
++    let m = hasAnyAttribute(this._type, ["NS_stack", "NS_managed", "NS_GC"]);
++    if (m != null)
++      return new StorageReason(this._type.loc, "typedef %s marked with annotation %s".format(this._type.name, m));
++    return getStorageClass(this._type).blockMalloc();
++  },
++  
++  blockGC: function() {
++    let m = hasAnyAttribute(this._type, ["NS_stack", "NS_NoGC"]);
++    if (m != null)
++      return new StorageReason(this._type.loc, "typedef %s marked with annotation %s".format(this._type.name, m));
++    return getStorageClass(this._type).blockGC();
++  },
++  
++  safeFinalizer: function() { return getStorageClass(this._type).safeFinalizer(); },
++  canSkipFinalization: function() { return getStorageClass(this._type).canSkipFinalization(); }
++};
++
++/**
++ * Does a class have a declared destructor?
++ */
++function hasOwnDestructor(c)
++{
++  for (let member in c.members)
++    if (member.isFunction && /^~/(member.name))
++      return true;
++
++  return false;
++}
++
++function StructStorageClass(type)
++{
++  this._type = type;
++}
++StructStorageClass.prototype = {
++  /**
++   * @returns false or a StorageReason
++   * @note This function does not recurse, it's only for local use
++   */
++  isManaged: function() {
++    let m = hasAnyAttribute(this._type, ["NS_GC", "NS_managed"]);
++    if (m != false)
++      return new StorageReason(this._type.loc, "Class %s is annotated %s".format(this._type.name, m));
++    
++    if (this._type.name == 'MMgc::GCObject' ||
++        this._type.name == 'MMgc::GCFinalizable' ||
++        this._type.name == 'XPCOMGCObject' ||
++        this._type.name == 'XPCOMGCFinalizedObject')
++      return new StorageReason(this._type.loc, "Class %s is considered managed.".format(this._type.name));
++    
++    return null;
++  },
++  
++  iterItems: function(fname) {
++    for each (let base in this._type.bases) {
++      let r = getStorageClass(base)[fname]();
++      if (r != null)
++        return new StorageReason(this._type.loc, "class %s is a base of %s".format([base.name, this._type.name]), r);
++    }
++    for each (let member in this._type.members) {
++      if (member.isFunction || member.isStatic)
++        continue;
++      
++      let r = getStorageClass(member)[fname]();
++      if (r != null)
++        return new StorageReason(member.loc, "due to member %s".format(member.name), r);
++    }
++    return null;
++  },
++
++  blockStack: function() {
++    let m = hasAnyAttribute(this._type, ["NS_GC"]);
++    if (m != false)
++      return new StorageReason(this._type.loc, "%s %s marked as %s".format(this._type.kind, this._type.name, m));
++    return this.iterItems('blockStack');
++  },
++  
++  blockGlobal: function() {
++    let m = hasAnyAttribute(this._type, ["NS_stack"]);
++    if (m != false)
++      return new StorageReason(this._type.loc, "%s %s marked as %s".format(this._type.kind, this._type.name, m));
++
++    m = this.isManaged();
++    if (m != null)
++      return m;
++
++    return this.iterItems('blockGlobal');
++  },
++  
++  blockMalloc: function() {
++    let m = hasAnyAttribute(this._type, ["NS_stack"]);
++    if (m != false) {
++      return new StorageReason(this._type.loc, "%s %s marked as %s".format(this._type.kind, this._type.name, m));
++    }
++    
++    m = this.isManaged();
++    if (m != null)
++      return m;
++    
++    return this.iterItems('blockMalloc');
++  },
++  
++  blockGC: function() {
++    let m = hasAnyAttribute(this._type, ["NS_NoGC"]);
++    if (m != false)
++      return new StorageReason(this._type.loc, "%s %s marked as %s".format(this._type.kind, this._type.name, m));
++    
++    return this.iterItems('blockGC');
++  },
++  
++  safeFinalizer: function() {
++    let m = this.iterItems('safeFinalizer');
++    if (m != null)
++      return m;
++
++    m = hasAnyAttribute(this._type, ["NS_GCOK"]);
++    if (m != false)
++      return null;
++
++    m = this.isManaged();
++    if (m != null)
++      return null; // Managed classes are finalizer-safe
++
++    if (hasOwnDestructor(this._type))
++      return new StorageReason(this._type.loc, "%s %s not annotated for as GC and has a destructor.".format(this._type.kind, this._type.name));
++    
++    // If this class doesn't have a destructor, it's finalizer-safe.
++    // Implicit destructors are dealt with above in iterItems
++    return null;
++  },
++  
++  canSkipFinalization: function() {
++    let m = this.iterItems('canSkipFinalization');
++    if (m != null)
++      return m;
++    
++    if (hasAttribute(this._type, 'NS_finalizer_not_required'))
++      return null;
++
++    if (hasOwnDestructor(this._type))
++      return new StorageReason(this._type.loc, "%s %s has a destructor and is not annotated NS_FINALIZER_NOT_REQUIRED.".format(this._type.kind, this._type.name));
++    
++    return null;
++  }
++};
++
++function DeclStorageClass(decl)
++{
++  this._decl = decl;
++  print("Created decl storage class for %s".format(decl));
++}
++DeclStorageClass.prototype = {
++  blockStack: function() { return getStorageClass(this._decl.type).blockStack(); },
++  blockGlobal: function() {
++    if (hasAttribute(this._decl, "NS_unmanaged"))
++      return null;
++    
++    return getStorageClass(this._decl.type).blockGlobal();
++  },
++  blockMalloc: function() {
++    if (hasAttribute(this._decl, "NS_unmanaged"))
++      return null;
++    
++    return getStorageClass(this._decl.type).blockMalloc();
++  },
++  blockGC: function() { return getStorageClass(this._decl.type).blockGC(); }
++};
++
++/**
++ * Get the storage class for a type.
++ */
++function getStorageClass(type)
++{
++  if (type.typedef)
++    return new TypedefStorageClass(type);
++  
++  if (type.isPointer || type.isReference)
++    return new PointerStorageClass(type);
++  
++  if (type.isArray)
++    return new ArrayStorageClass(type);
++  
++  if (type.isFunction)
++    throw Error("How did a function decl end up in getStorageClass?");
++
++  if (type.kind) {
++    switch (type.kind) {
++    case "struct":
++    case "class":
++      return new StructStorageClass(type);
++    case "union":
++      // TODO: what the hell should union rules be? For now, bail!
++      return ValueStorageClass;
++    default:
++      throw Error("Unrecognized type.kind '%s'".format(type.kind));
++    }
++  }
++  
++  if (type.parameters) {
++    // this is a function *type*... we can end up here via function pointers
++    return ValueStorageClass;
++  }
++
++  if (type.precision) {
++    if (type.type)
++      throw Error("I thought this was a numeric type, but apparently it's not: %s".format(type));
++    
++    return ValueStorageClass;
++  }
++  
++  // If we're here, whatever we have is probably a decl...
++  return new DeclStorageClass(type);
++}
+diff --git a/xpcom/base/nscore.h b/xpcom/base/nscore.h
+--- a/xpcom/base/nscore.h
++++ b/xpcom/base/nscore.h
+@@ -501,13 +501,18 @@ typedef PRUint32 nsrefcnt;
+ #ifdef NS_STATIC_CHECKING
+ #define NS_STACK_CLASS __attribute__((user("NS_stack")))
+ #define NS_FINAL_CLASS __attribute__((user("NS_final")))
++#define NS_MANAGED     __attribute__((user("NS_managed")))
+ #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")))
++#define NS_GCOK        __attribute__((user("NS_GCOK")))
++#define NS_FINALIZER_NOT_REQUIRED __attribute__((user("NS_finalizer_not_required")))
++#define NS_UNMANAGED   __attribute__((user("NS_unmanaged")))
+ #define NS_FINALIZER   __attribute__((user("NS_Finalizer")))
+ #define NS_NO_FINALIZER __attribute__((user("NS_No_Finalizer")))
+-#define NS_OKONSTACK   __attribute__((user("NS_okOnStack")))
++#define NS_MALLOCATOR  __attribute__((user("NS_mallocator")))
++#define NS_GCALLOCATOR __attribute__((user("NS_gcallocator")))
++#define NS_GCFINALIZEDALLOCATOR __attribute__((user("NS_gcfinalizedallocator")))
++#define NS_OKONSTACK /* get rid of me, quick! */
+ #else
+ #define NS_STACK_CLASS
+ #define NS_FINAL_CLASS
+diff --git a/xpcom/tests/static-checker/GCNew.cpp b/xpcom/tests/static-checker/GCNew.cpp
+--- a/xpcom/tests/static-checker/GCNew.cpp
++++ b/xpcom/tests/static-checker/GCNew.cpp
+@@ -1,6 +1,7 @@
+ #include "nscore.h"
++#include "nsISupportsUtils.h"
+ 
+-struct NS_GC_TYPE A
++struct NS_GC_TYPE A : public XPCOMGCObject
+ {
+   A();
+   int i;
+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
+@@ -73,7 +73,6 @@ STACK_FAILURE_TESTCASES = \
+   TestStackTemplate.cpp \
+   StackNoConstructor.cpp \
+   StackVector.cpp \
+-  TestStackCOMPtr.cpp \
+   $(NULL)
+ 
+ STACK_PASS_TESTCASES = \
+diff --git a/xpcom/tests/static-checker/TestStackCOMPtr.cpp b/xpcom/tests/static-checker/TestStackCOMPtr.cpp
+deleted file mode 100644
+--- a/xpcom/tests/static-checker/TestStackCOMPtr.cpp
++++ /dev/null
+@@ -1,6 +0,0 @@
+-#include "nsCOMPtr.h"
+-
+-void TestFunc()
+-{
+-  nsCOMPtr<nsISupports> f = nsnull;
+-}
new file mode 100644
--- /dev/null
+++ b/static-check-storage-classes2
@@ -0,0 +1,362 @@
+diff --git a/xpcom/analysis/gc.js b/xpcom/analysis/gc.js
+--- a/xpcom/analysis/gc.js
++++ b/xpcom/analysis/gc.js
+@@ -13,136 +13,6 @@ function process_type(c)
+        c.name != "XPCOMGCFinalizedObject")
+       error("MMgc::GCFinalizable is not the left-most base of class " + c.name + " which inherits from XPCOMGCFinalizedObject", c.loc);
+   }
+-}
+-
+-function isGC(c)
+-{
+-  function calculate()
+-  {
+-    if (hasAttribute(c, 'NS_GC') ||
+-	c.name == 'MMgc::GCObject' ||
+-	c.name == 'MMgc::GCFinalizable' ||
+-	c.name == 'XPCOMGCObject' ||
+-	c.name == 'XPCOMGCFinalizedObject')
+-      return true;
+-
+-    // nsIFrame is a figment of your imagination!
+-    if (c.name == 'nsIFrame')
+-      return false;
+-
+-    for each (let base in c.bases)
+-    if (isGC(base, false))
+-      return true;
+-
+-    for each (let member in c.members) {
+-      if (member.isFunction)
+-	continue;
+-
+-      let type = member.type;
+-      while (true) {
+-	if (type.isArray) {
+-	  type = type.type;
+-	  continue;
+-	}
+-
+-	if (type.typedef) {
+-	  if (hasAttribute(type, 'NS_GC'))
+-	    return true;
+-
+-	  type = type.typedef;
+-	  continue;
+-	}
+-	break;
+-      }
+-
+-      if (type === undefined) {
+-	warning("incomplete type for member " + member + ".", member.loc);
+-	continue;
+-      }
+-
+-      if (type.isPointer || type.isReference)
+-	continue;
+-
+-      if (!type.kind || (type.kind != 'class' && type.kind != 'struct'))
+-	continue;
+-
+-      if (isGC(type, false))
+-	return true;
+-    }
+-    return false;
+-  }
+-
+-  if (c.isIncomplete)
+-    throw Error("Can't get GC status of incomplete class.");
+-
+-  if (!c.hasOwnProperty("isGC"))
+-    c.isGC = calculate();
+-
+-  return c.isGC;
+-}
+-
+-function okOnStack(c)
+-{
+-  function calculate()
+-  {
+-    if (!isGC(c))
+-      return true;
+-
+-    // GC classes which are annotated with "NS_okOnStack" are acceptable
+-    // on the stack
+-    if (hasAttribute(c, "NS_okOnStack"))
+-      return true;
+-
+-    for each (let member in c.members) {
+-      if (member.isFunction)
+-	continue;
+-
+-      let type = member.type;
+-      while (true) {
+-	if (type.isArray) {
+-	  type = type.type;
+-	  continue;
+-	}
+-
+-	if (type.typedef) {
+-	  type = type.typedef;
+-	  continue;
+-	}
+-	break;
+-      }
+-
+-      if (type === undefined) {
+-	warning("incomplete type for member " + member + ".", member.loc);
+-	continue;
+-      }
+-
+-      if (type.isPointer || type.isReference)
+-	continue;
+-
+-      if (!type.kind || (type.kind != 'class' && type.kind != 'struct'))
+-	continue;
+-
+-      if (!okOnStack(type))
+-	return false;
+-    }
+-
+-    for each (let b in c.bases)
+-      if (!okOnStack(b))
+-	return false;
+-
+-    if (hasAttribute(c, "NS_GC"))
+-      return false;
+-
+-    return true;
+-  }
+-
+-  if (c.isIncomplete)
+-    throw Error("Can't get ok-on-stack status of incomplete class.");
+-
+-  if (!c.hasOwnProperty('okOnStack'))
+-    c.okOnStack = calculate();
+-
+-  return c.okOnStack;
+ }
+ 
+ /**
+@@ -255,15 +125,16 @@ function process_function(f, stmts)
+     // v.fieldOf.type is a struct/class if this is a stack constructor
+     // it's a pointer if this is some other construction technique
+     if (v.isConstructor) {
+-      if ((!v.fieldOf || !v.fieldOf.type.isPointer) &&
+-	  !okOnStack(v.memberOf)) {
++      if ((!v.fieldOf || !v.fieldOf.type.isPointer)) {
+ 	if (v.fieldOf &&
+ 	    v.fieldOf.fieldOf &&
+ 	    v.fieldOf.fieldOf.name == "this") {
+ 	  return;
+ 	}
+-	error("Constructing GC class " + v.memberOf.name + " on the stack.",
+-	  getLocation());
++        let m = getStorageClass(v.memberOf).blockStack();
++        if (m)
++          error("Constructing GC class %s on the stack\n%s".format(v.memberOf.name, m),
++	    getLocation());
+       }
+     }
+   }
+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
+@@ -157,7 +157,11 @@ function StorageReason(loc, message, pre
+ }
+ StorageReason.prototype.toString = function()
+ {
+-  let str = "%s:   %s".format(loc_string(this.loc), this.message);
++  let loc = this.loc;
++  if (loc === undefined)
++    loc = "<unknown location>";
++  
++  let str = "%s:   %s".format(loc, this.message);
+   if (this.prev)
+     str += "\n%s".format(this.prev);
+   return str;
+@@ -180,7 +184,8 @@ ValueStorageClass =  {
+   blockMalloc: function() { return null; },
+   blockGC: function() { return null; },
+   safeFinalizer: function() { return null; },
+-  canSkipFinalization: function() { return null; }
++  canSkipFinalization: function() { return null; },
++  toString: function() { return "{ValueStorageClass)"; }
+ };
+ 
+ function PointerStorageClass(type)
+@@ -193,17 +198,18 @@ PointerStorageClass.prototype = {
+     if (!subtypeClass.blockGC()) {
+       let reason = subtypeClass.blockMalloc();
+       if (reason)
+-        return new StorageReason(this._type.loc, "pointer to", reason);
++        return new StorageReason(this._type.type.loc, "pointer to", reason);
+     }
+     return null;
+   },
+   
+   blockStack: function() { return null; },
+-  blockGlobal: function() { return this.checkNotManaged(); },
+-  blockMalloc: function() { return this.checkNotManaged(); },
++  blockGlobal: function() { return this.blockNotManaged(); },
++  blockMalloc: function() { return this.blockNotManaged(); },
+   blockGC: function() { return null; },
+   safeFinalizer: function() { return null; },
+-  canSkipFinalization: function() { return null; }
++  canSkipFinalization: function() { return null; },
++  toString: function() { return "PointerStorageClass(%s)".format(this._type); }
+ };
+ 
+ function ArrayStorageClass(type)
+@@ -212,12 +218,13 @@ function ArrayStorageClass(type)
+ }
+ ArrayStorageClass.prototype = {
+   /* Just forward everything to the inner... */
+-  blockStack: function() { return getStorageClass(this._type).blockStack(); },
+-  blockGlobal: function() { return getStorageClass(this._type).blockGlobal(); },
+-  blockMalloc: function() { return getStorageClass(this._type).blockMalloc(); },
+-  blockGC: function() { return getStorageClass(this._type).blockGC(); },
+-  safeFinalizer: function() { return getStorageClass(this._type).safeFinalizer(); },
+-  canSkipFinalization: function() { return getStorageClass(this._type).canSkipFinalization(); }
++  blockStack: function() { return getStorageClass(this._type.type).blockStack(); },
++  blockGlobal: function() { return getStorageClass(this._type.type).blockGlobal(); },
++  blockMalloc: function() { return getStorageClass(this._type.type).blockMalloc(); },
++  blockGC: function() { return getStorageClass(this._type.type).blockGC(); },
++  safeFinalizer: function() { return getStorageClass(this._type.type).safeFinalizer(); },
++  canSkipFinalization: function() { return getStorageClass(this._type.type).canSkipFinalization(); },
++  toString: function() { return "ArrayStorageClass(%s)".format(this._type); }
+ };
+ 
+ function TypedefStorageClass(type)
+@@ -227,34 +234,35 @@ TypedefStorageClass.prototype = {
+ TypedefStorageClass.prototype = {
+   blockStack: function() {
+     let m = hasAnyAttribute(this._type, ["NS_GC"]);
+-    if (m != null)
++    if (m != false)
+       return new StorageReason(this._type.loc, "typedef %s marked with annotation %s".format(this._type.name, m));
+-    return getStorageClass(this._type).blockStack();
++    return getStorageClass(this._type.typedef).blockStack();
+   },
+   
+   blockGlobal: function() {
+     let m = hasAnyAttribute(this._type, ["NS_stack", "NS_managed", "NS_GC"]);
+-    if (m != null)
++    if (m != false)
+       return new StorageReason(this._type.loc, "typedef %s marked with annotation %s".format(this._type.name, m));
+-    return getStorageClass(this._type).blockGlobal();
++    return getStorageClass(this._type.typedef).blockGlobal();
+   },
+   
+   blockMalloc: function() {
+     let m = hasAnyAttribute(this._type, ["NS_stack", "NS_managed", "NS_GC"]);
+-    if (m != null)
++    if (m != false)
+       return new StorageReason(this._type.loc, "typedef %s marked with annotation %s".format(this._type.name, m));
+-    return getStorageClass(this._type).blockMalloc();
++    return getStorageClass(this._type.typedef).blockMalloc();
+   },
+   
+   blockGC: function() {
+     let m = hasAnyAttribute(this._type, ["NS_stack", "NS_NoGC"]);
+-    if (m != null)
++    if (m != false)
+       return new StorageReason(this._type.loc, "typedef %s marked with annotation %s".format(this._type.name, m));
+-    return getStorageClass(this._type).blockGC();
++    return getStorageClass(this._type.typedef).blockGC();
+   },
+   
+-  safeFinalizer: function() { return getStorageClass(this._type).safeFinalizer(); },
+-  canSkipFinalization: function() { return getStorageClass(this._type).canSkipFinalization(); }
++  safeFinalizer: function() { return getStorageClass(this._type.typedef).safeFinalizer(); },
++  canSkipFinalization: function() { return getStorageClass(this._type.typedef).canSkipFinalization(); },
++  toString: function() { return "TypedefStorageClass(%s)".format(this._type); }
+ };
+ 
+ /**
+@@ -293,6 +301,9 @@ StructStorageClass.prototype = {
+   },
+   
+   iterItems: function(fname) {
++    if (this._type.isIncomplete)
++      return null;
++
+     for each (let base in this._type.bases) {
+       let r = getStorageClass(base)[fname]();
+       if (r != null)
+@@ -382,13 +393,13 @@ StructStorageClass.prototype = {
+       return new StorageReason(this._type.loc, "%s %s has a destructor and is not annotated NS_FINALIZER_NOT_REQUIRED.".format(this._type.kind, this._type.name));
+     
+     return null;
+-  }
++  },
++  toString: function() { return "StructStorageClass(%s)".format(this._type); }
+ };
+ 
+ function DeclStorageClass(decl)
+ {
+   this._decl = decl;
+-  print("Created decl storage class for %s".format(decl));
+ }
+ DeclStorageClass.prototype = {
+   blockStack: function() { return getStorageClass(this._decl.type).blockStack(); },
+@@ -404,7 +415,10 @@ DeclStorageClass.prototype = {
+     
+     return getStorageClass(this._decl.type).blockMalloc();
+   },
+-  blockGC: function() { return getStorageClass(this._decl.type).blockGC(); }
++  blockGC: function() { return getStorageClass(this._decl.type).blockGC(); },
++  safeFinalizer: function() { return getStorageClass(this._decl.type).safeFinalizer(); },
++  canSkipFinalization: function() { return getStorageClass(this._decl.type).canSkipFinalization(); },
++  toString: function() { return "DeclStorageClass(%s)".format(this._type); }
+ };
+ 
+ /**
+@@ -432,6 +446,8 @@ function getStorageClass(type)
+     case "union":
+       // TODO: what the hell should union rules be? For now, bail!
+       return ValueStorageClass;
++    case "enum":
++      return ValueStorageClass;
+     default:
+       throw Error("Unrecognized type.kind '%s'".format(type.kind));
+     }
+@@ -442,7 +458,7 @@ function getStorageClass(type)
+     return ValueStorageClass;
+   }
+ 
+-  if (type.precision) {
++  if (type.precision || type.name == 'void') {
+     if (type.type)
+       throw Error("I thought this was a numeric type, but apparently it's not: %s".format(type));
+     
+diff --git a/xpcom/tests/static-checker/GCPointersManaged.cpp b/xpcom/tests/static-checker/GCPointersManaged.cpp
+new file mode 100644
+--- /dev/null
++++ b/xpcom/tests/static-checker/GCPointersManaged.cpp
+@@ -0,0 +1,14 @@
++#include "nscore.h"
++
++struct NS_GC_TYPE A;
++
++struct B
++{
++  // pointers to GC types imply that class B is a managed type
++  A *a;
++};
++
++void f()
++{
++  B *b = new B();
++}
+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
+@@ -55,6 +55,7 @@ GC_FAILURE_TESTCASES = \
+   GCNotStack-return.cpp \
+   GCInheritance1.cpp \
+   GCInheritance2.cpp \
++  GCPointersManaged.cpp \
+   $(NULL)
+ 
+ GC_PASS_TESTCASES = \
new file mode 100644
--- /dev/null
+++ b/storage-class-annotations
@@ -0,0 +1,24 @@
+diff --git a/xpcom/components/nsCategoryManager.h b/xpcom/components/nsCategoryManager.h
+--- a/xpcom/components/nsCategoryManager.h
++++ b/xpcom/components/nsCategoryManager.h
+@@ -117,7 +117,7 @@ public:
+ 
+ private:
+   CategoryNode() { }
+-  void* operator new(size_t aSize, PLArenaPool* aArena);
++  void* operator new(size_t aSize, PLArenaPool* aArena) NS_MALLOCATOR;
+ 
+   nsTHashtable<CategoryLeaf> mTable;
+   PRLock* mLock;
+diff --git a/xpcom/glue/nsArrayEnumerator.cpp b/xpcom/glue/nsArrayEnumerator.cpp
+--- a/xpcom/glue/nsArrayEnumerator.cpp
++++ b/xpcom/glue/nsArrayEnumerator.cpp
+@@ -139,7 +139,7 @@ public:
+     nsCOMArrayEnumerator(const nsCOMArray_base &array);
+ 
+     // specialized operator to make sure we make room for mValues
+-    void* operator new (size_t size, PRUint32 item_count) CPP_THROW_NEW;
++    void* operator new (size_t size, PRUint32 item_count) CPP_THROW_NEW NS_GCFINALIZEDALLOCATOR;
+ 
+ private:
+     ~nsCOMArrayEnumerator(void);