Bug 419624 - Build framework, basic usage, and testsuite for statically checking the codebase using gcc-dehydra, r=luser,dbaron,tglek
authorbenjamin@smedbergs.us
Wed, 27 Feb 2008 11:28:13 -0500
changeset 12116 7dc5a61c3b07fca66e3356d935838087f4fefd81
parent 12115 c8276e89a51380d6f7c27da136c6acff1ed753df
child 12365 99848dc55a1a6fae90e69e03c3819fdf3b3d088f
push idunknown
push userunknown
push dateunknown
reviewersluser, dbaron, tglek
bugs419624
milestone2.0a1pre
Bug 419624 - Build framework, basic usage, and testsuite for statically checking the codebase using gcc-dehydra, r=luser,dbaron,tglek
config/autoconf.mk.in
config/config.mk
configure.in
toolkit/toolkit-makefiles.sh
xpcom/base/nscore.h
xpcom/glue/nsCOMPtr.h
xpcom/static-checking.js
xpcom/tests/Makefile.in
xpcom/tests/static-checker/Makefile.in
xpcom/tests/static-checker/TestFinal.cpp
xpcom/tests/static-checker/TestFinalTemplate.cpp
xpcom/tests/static-checker/TestStack.cpp
xpcom/tests/static-checker/TestStackTemplate.cpp
--- a/config/autoconf.mk.in
+++ b/config/autoconf.mk.in
@@ -98,16 +98,18 @@ MOZ_EXTENSIONS  = @MOZ_EXTENSIONS@
 MOZ_IMG_DECODERS= @MOZ_IMG_DECODERS@
 MOZ_IMG_ENCODERS= @MOZ_IMG_ENCODERS@
 MOZ_JSDEBUGGER  = @MOZ_JSDEBUGGER@
 MOZ_PERF_METRICS = @MOZ_PERF_METRICS@
 MOZ_LEAKY	= @MOZ_LEAKY@
 MOZ_MEMORY      = @MOZ_MEMORY@
 MOZ_JPROF       = @MOZ_JPROF@
 MOZ_SHARK       = @MOZ_SHARK@
+DEHYDRA_PATH    = @DEHYDRA_PATH@
+
 MOZ_XPCTOOLS    = @MOZ_XPCTOOLS@
 ENABLE_EAZEL_PROFILER=@ENABLE_EAZEL_PROFILER@
 EAZEL_PROFILER_CFLAGS=@EAZEL_PROFILER_CFLAGS@
 EAZEL_PROFILER_LIBS=@EAZEL_PROFILER_LIBS@
 GC_LEAK_DETECTOR = @GC_LEAK_DETECTOR@
 NS_TRACE_MALLOC = @NS_TRACE_MALLOC@
 USE_ELF_DYNSTR_GC = @USE_ELF_DYNSTR_GC@
 USE_PREBINDING = @USE_PREBINDING@
--- a/config/config.mk
+++ b/config/config.mk
@@ -515,16 +515,26 @@ REQ_INCLUDES_SDK = $(foreach d,$(REQUIRE
 endif
 
 INCLUDES	= $(LOCAL_INCLUDES) $(REQ_INCLUDES) $(REQ_INCLUDES_SDK) -I$(PUBLIC) $(OS_INCLUDES)
 
 ifndef MOZILLA_INTERNAL_API
 INCLUDES	+= -I$(LIBXUL_DIST)/sdk/include
 endif
 
+# The entire tree should be subject to static analysis using the XPCOM
+# script. Additional scripts may be added by specific subdirectories.
+
+DEHYDRA_SCRIPTS = $(topsrcdir)/xpcom/static-checking.js
+
+ifdef DEHYDRA_PATH
+DEHYDRA_FLAGS = -fplugin=$(DEHYDRA_PATH) $(foreach script,$(DEHYDRA_SCRIPTS),-fplugin-arg=$(script))
+OS_CXXFLAGS += $(DEHYDRA_FLAGS)
+endif
+
 CFLAGS		= $(OS_CFLAGS)
 CXXFLAGS	= $(OS_CXXFLAGS)
 LDFLAGS		= $(OS_LDFLAGS) $(MOZ_FIX_LINK_PATHS)
 
 # Allow each module to override the *default* optimization settings
 # by setting MODULE_OPTIMIZE_FLAGS if the developer has not given
 # arguments to --enable-optimize
 ifdef MOZ_OPTIMIZE
--- a/configure.in
+++ b/configure.in
@@ -6156,16 +6156,34 @@ MOZ_ARG_ENABLE_BOOL(shark,
 [  --enable-shark          Enable shark remote profiling (needs CHUD framework)],
     MOZ_SHARK=1,
     MOZ_SHARK= )
 if test -n "$MOZ_SHARK"; then
     AC_DEFINE(MOZ_SHARK)
 fi
 
 dnl ========================================================
+dnl = Enable static checking using gcc-dehydra
+dnl ========================================================
+
+MOZ_ARG_WITH_STRING(static-checking,
+[  --with-static-checking=path/to/gcc_dehydra.so
+                            Enable static checking of code using GCC-dehydra],
+    DEHYDRA_PATH=$withval,
+    DEHYDRA_PATH= )
+
+if test -n "$DEHYDRA_PATH"; then
+    if ! test -f "$DEHYDRA_PATH"; then
+        AC_MSG_ERROR([The dehydra plugin is not at the specified path.])
+    fi
+    AC_DEFINE(NS_STATIC_CHECKING)
+fi
+AC_SUBST(DEHYDRA_PATH)
+
+dnl ========================================================
 dnl = Enable stripping of libs & executables
 dnl ========================================================
 MOZ_ARG_ENABLE_BOOL(strip,
 [  --enable-strip          Enable stripping of libs & executables ],
     ENABLE_STRIP=1,
     ENABLE_STRIP= )
 
 dnl ========================================================
--- a/toolkit/toolkit-makefiles.sh
+++ b/toolkit/toolkit-makefiles.sh
@@ -509,16 +509,17 @@ MAKEFILES_xpcom_obsolete="
   xpcom/obsolete/component/Makefile
 "
 
 MAKEFILES_xpcom_tests="
   xpcom/tests/Makefile
   xpcom/tests/dynamic/Makefile
   xpcom/tests/services/Makefile
   xpcom/tests/windows/Makefile
+  xpcom/tests/static-checker/Makefile
 "
 
 MAKEFILES_xpinstall="
   xpinstall/Makefile
   xpinstall/public/Makefile
   xpinstall/src/Makefile
 "
 
--- a/xpcom/base/nscore.h
+++ b/xpcom/base/nscore.h
@@ -466,9 +466,23 @@ typedef PRUint32 nsrefcnt;
   * If we're being linked as standalone glue, we don't want a dynamic dependency
   * on NSPR libs, so we skip the debug thread-safety checks, and we cannot use
   * the THREADSAFE_ISUPPORTS macros.
   */
 #if defined(XPCOM_GLUE) && !defined(XPCOM_GLUE_USE_NSPR)
 #define XPCOM_GLUE_AVOID_NSPR
 #endif
 
+/**
+ * Static type annotations, enforced when static-checking is enabled:
+ *
+ * NS_STACK_CLASS: a class which must only be instantiated on the stack
+ * NS_FINAL_CLASS: a class which may not be subclassed
+ */
+#ifdef NS_STATIC_CHECKING
+#define NS_STACK_CLASS __attribute__((user("NS_stack")))
+#define NS_FINAL_CLASS __attribute__((user("NS_final")))
+#else
+#define NS_STACK_CLASS
+#define NS_FINAL_CLASS
+#endif
+
 #endif /* nscore_h___ */
--- a/xpcom/glue/nsCOMPtr.h
+++ b/xpcom/glue/nsCOMPtr.h
@@ -179,17 +179,20 @@
         nsCOMPtr < nsGetterAddRefs
 
       The other compilers probably won't complain, so please don't reorder these
       classes, on pain of breaking 4.2 compatibility.
   */
 
 
 template <class T>
-class nsDerivedSafe : public T
+class
+  NS_FINAL_CLASS
+  NS_STACK_CLASS
+nsDerivedSafe : public T
     /*
       No client should ever see or have to type the name of this class.  It is the
       artifact that makes it a compile-time error to call |AddRef| and |Release|
       on a |nsCOMPtr|.  DO NOT USE THIS TYPE DIRECTLY IN YOUR CODE.
 
       See |nsCOMPtr::operator->|, |nsCOMPtr::operator*|, et al.
 
       This type should be a nested class inside |nsCOMPtr<T>|.
@@ -336,17 +339,21 @@ class nsCOMPtr_helper
 
 /*
   |nsQueryInterface| could have been implemented as an |nsCOMPtr_helper| to
   avoid adding specialized machinery in |nsCOMPtr|, But |do_QueryInterface|
   is called often enough that the codesize savings are big enough to
   warrant the specialcasing.
 */
 
-class NS_COM_GLUE nsQueryInterface
+class
+  NS_COM_GLUE
+  NS_STACK_CLASS
+  NS_FINAL_CLASS
+nsQueryInterface
   {
     public:
       explicit
       nsQueryInterface( nsISupports* aRawPtr )
           : mRawPtr(aRawPtr)
         {
           // nothing else to do here
         }
@@ -470,17 +477,18 @@ class NS_COM_GLUE nsGetServiceByContract
     
     nsresult NS_FASTCALL operator()( const nsIID&, void** ) const;
     
  private:
     const char*                 mContractID;
     nsresult*                   mErrorPtr;
 };
 
-class nsCOMPtr_base
+class
+nsCOMPtr_base
     /*
       ...factors implementation for all template versions of |nsCOMPtr|.
 
       This should really be an |nsCOMPtr<nsISupports>|, but this wouldn't work
       because unlike the
 
       Here's the way people normally do things like this
       
@@ -530,17 +538,19 @@ class nsCOMPtr_base
           if ( oldPtr )
             NSCAP_RELEASE(this, oldPtr);
         }
   };
 
 // template <class T> class nsGetterAddRefs;
 
 template <class T>
-class nsCOMPtr
+class
+  NS_FINAL_CLASS
+nsCOMPtr
 #ifdef NSCAP_FEATURE_USE_BASE
     : private nsCOMPtr_base
 #endif
   {
 
 #ifdef NSCAP_FEATURE_USE_BASE
   #define NSCAP_CTOR_BASE(x) nsCOMPtr_base(x)
 #else
new file mode 100644
--- /dev/null
+++ b/xpcom/static-checking.js
@@ -0,0 +1,214 @@
+/* -*- Mode: Java; c-basic-offset: 2; indent-tabs-mode: nil -*- */
+/**
+ * A script for GCC-dehydra to analyze the Mozilla codebase and catch
+ * patterns that are incorrect, but which cannot be detected by a compiler. */
+
+/**
+ * gClassMap maps class names to an object with the following properties:
+ *
+ * .final = true   if the class has been annotated as final, and may not be
+ *                 subclassed
+ * .stack = true   if the class has been annotated as a class which may only
+ *                 be instantiated on the stack
+ */
+var gClassMap = {};
+
+function ClassType(name)
+{
+  this.name = name;
+}
+
+ClassType.prototype = {
+  final: false,
+  stack: false,
+};
+
+function process_class(c)
+{
+  get_class(c, true);
+}
+
+/**
+ * Get the ClassType for a type 'c'
+ *
+ * If allowIncomplete is true and the type is incomplete, this function
+ * will return null.
+ *
+ * If allowIncomplete is false and the type is incomplete, this function will
+ * throw.
+ */
+function get_class(c, allowIncomplete)
+{
+  var classattr, base, member, type, realtype, foundConstructor;
+  var bases = [];
+
+  if (c.isIncomplete) {
+    if (allowIncomplete)
+      return null;
+
+    throw Error("Can't process incomplete type '" + c + "'.");
+  }
+
+  if (gClassMap.hasOwnProperty(c.name)) {
+    return gClassMap[c.name];
+  }
+
+  for each (base in c.bases) {
+      realtype = get_class(base, allowIncomplete);
+      if (realtype == null) {
+        error("Complete type " + c + " has incomplete base " + base);
+        return null;
+      }
+
+      bases.push(realtype);
+  }
+
+  function hasAttribute(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;
+  }
+
+  classattr = new ClassType(c.name);
+  gClassMap[c.name] = classattr;
+
+  // check for .final
+
+  if (hasAttribute('NS_final')) {
+    classattr.final = true;
+  }
+
+  // check for .stack
+
+  if (hasAttribute('NS_stack')) {
+    classattr.stack = true;
+  }
+  else {
+    for each (base in bases) {
+      if (base.stack) {
+        classattr.stack = true;
+        break;
+      }
+    }
+  }
+
+  if (!classattr.stack) {
+    // Check members
+    for each (member in c.members) {
+      if (member.isFunction)
+        continue;
+
+      type = member.type;
+
+      /* recurse through arrays and typedefs */
+      while (true) {
+          if (type === undefined) {
+              break;
+          }
+              
+          if (type.isArray) {
+              type = type.type;
+              continue;
+          }
+          if (type.typedef) {
+              type = type.typedef;
+              continue;
+          }
+          break;
+      }
+
+      if (type === undefined) {
+          warning("incomplete type  for member " + member + ".");
+          continue;
+      }
+
+      if (type.isPointer || type.isReference) {
+          continue;
+      }
+
+      if (!type.kind || (type.kind != 'class' && type.kind != 'struct')) {
+          continue;
+      }
+
+      var membertype = get_class(type, false);
+      if (membertype.stack) {
+        classattr.stack = true;
+        break;
+      }
+    }
+  }
+
+  // Check for errors at declaration-time
+
+  for each (base in bases) {
+    if (base.final) {
+      error("class '" + c.name + "' inherits from final class '" + base.name + "'.");
+    }
+  }
+
+  // At the moment, any class that is .final has to have a constructor, or
+  // we can't detect callsites... this may change with treehydra.
+  if (classattr.stack) {
+    foundConstructor = false;
+    for each (member in c.members) {
+      if (member.isConstructor) {
+        foundConstructor = true;
+        break;
+      }
+    }
+
+    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.");
+    }
+  }
+
+  return classattr;
+}
+
+/**
+ * Unwrap any array of types back to their base type.
+ */
+function unwrapArray(t)
+{
+  while (t.isArray) {
+    t = t.type;
+  }
+  return t;
+}
+
+function process_function(f, stmts)
+{
+  var stmt;
+  function getLocation()
+  {
+    if (stmt.loc)
+      return stmt.loc;
+
+    return f.loc;
+  }
+
+  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.");
+    }
+  }
+
+  for each (stmt in stmts) {
+    iter(processVar, stmt.statements);
+  }
+}
--- a/xpcom/tests/Makefile.in
+++ b/xpcom/tests/Makefile.in
@@ -48,16 +48,20 @@ ifndef MOZ_ENABLE_LIBXUL
 MOZILLA_INTERNAL_API = 1
 endif
 
 DIRS		= dynamic services external
 ifeq ($(OS_ARCH),WINNT)
 DIRS		+= windows
 endif
 
+ifdef DEHYDRA_PATH
+DIRS += static-checker
+endif
+
 REQUIRES	= \
 		  string \
 		  $(NULL)
 
 CPPSRCS		= \
 		nsIFileEnumerator.cpp \
 		nsIFileTest.cpp \
 		TestCallTemplates.cpp \
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/static-checker/Makefile.in
@@ -0,0 +1,66 @@
+# ***** BEGIN LICENSE BLOCK *****
+# Version: MPL 1.1/GPL 2.0/LGPL 2.1
+#
+# The contents of this file are subject to the Mozilla Public License Version
+# 1.1 (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+# http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS IS" basis,
+# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+# for the specific language governing rights and limitations under the
+# License.
+#
+# The Original Code is Mozilla 2.
+#
+# The Initial Developer of the Original Code is
+# the Mozilla Foundation <http://www.mozilla.org>.
+#
+# Portions created by the Initial Developer are Copyright (C) 2008
+# the Initial Developer. All Rights Reserved.
+#
+# Contributor(s):
+# Benjamin Smedberg <benjamin@smedbergs.us> (Author)
+#
+# Alternatively, the contents of this file may be used under the terms of
+# either the GNU General Public License Version 2 or later (the "GPL"), or
+# the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+# in which case the provisions of the GPL or the LGPL are applicable instead
+# of those above. If you wish to allow use of your version of this file only
+# under the terms of either the GPL or the LGPL, and not to allow others to
+# use your version of this file under the terms of the MPL, indicate your
+# decision by deleting the provisions above and replace them with the notice
+# and other provisions required by the GPL or the LGPL. If you do not delete
+# the provisions above, a recipient may use your version of this file under
+# the terms of any one of the MPL, the GPL or the LGPL.
+#
+# ***** END LICENSE BLOCK *****
+
+DEPTH = ../../..
+topsrcdir = @top_srcdir@
+srcdir = @srcdir@
+VPATH = @srcdir@
+
+include $(DEPTH)/config/autoconf.mk
+
+STATIC_FAILURE_TESTCASES = \
+  TestFinal.cpp \
+  TestFinalTemplate.cpp \
+  TestStack.cpp \
+  TestStackTemplate.cpp \
+  $(NULL)
+
+include $(topsrcdir)/config/rules.mk
+
+# We want to compile each file and invert the result to ensure that
+# compilation failed.
+check:: $(STATIC_FAILURE_TESTCASES:.cpp=.s-fail)
+
+%.s-fail: %.cpp Makefile Makefile.in $(DEHYDRA_SCRIPTS)
+	@printf "Compiling $(<F) to check that the static-analysis script is checking properly..."
+	@if $(CCC) $(OUTOPTION)/dev/null -S $(COMPILE_CXXFLAGS) $(_VPATH_SRCS) >$(*F).errlog 2>&1; then \
+	  printf "fail:\nerror: compilation of $(<F) succeeded. It shouldn't have!\n"; \
+	  exit 1; \
+	else \
+	  printf "ok.\n"; \
+	fi
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/static-checker/TestFinal.cpp
@@ -0,0 +1,11 @@
+#include "nscore.h"
+
+struct NS_FINAL_CLASS A
+{
+  int i;
+};
+
+struct B : A
+{
+  int j;
+};
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/static-checker/TestFinalTemplate.cpp
@@ -0,0 +1,12 @@
+#include "nscore.h"
+
+template<class T>
+struct NS_FINAL_CLASS A
+{
+  T i;
+};
+
+struct Bint : A<int>
+{
+  int j;
+};
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/static-checker/TestStack.cpp
@@ -0,0 +1,15 @@
+#include "nscore.h"
+
+struct NS_STACK_CLASS A
+{
+  // BUG: currently classes which are marked NS_STACK_CLASS must have a
+  // constructor
+  A();
+
+  int i;
+};
+
+void* Foo()
+{
+  return new A();
+}
new file mode 100644
--- /dev/null
+++ b/xpcom/tests/static-checker/TestStackTemplate.cpp
@@ -0,0 +1,14 @@
+#include "nscore.h"
+
+template<class T>
+struct NS_STACK_CLASS A
+{
+  A();
+
+  T i;
+};
+
+void *Foo()
+{
+  return new A<int>();
+}