Bug 419624 - Build framework, basic usage, and testsuite for statically checking the codebase using gcc-dehydra, r=luser,dbaron,tglek

config/autoconf.mk.in

@@ -103,6 +103,8 @@ 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@

config/config.mk

@@ -520,6 +520,16 @@ 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)

configure.in

@@ -6160,6 +6160,24 @@ 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 ========================================================

toolkit/toolkit-makefiles.sh

@@ -514,6 +514,7 @@ MAKEFILES_xpcom_tests="
   xpcom/tests/dynamic/Makefile
   xpcom/tests/services/Makefile
   xpcom/tests/windows/Makefile
+  xpcom/tests/static-checker/Makefile
 "
 
 MAKEFILES_xpinstall="

xpcom/base/nscore.h

@@ -471,4 +471,18 @@ typedef PRUint32 nsrefcnt;
 #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___ */

xpcom/glue/nsCOMPtr.h

@@ -184,7 +184,10 @@
 
 
 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|
@@ -341,7 +344,11 @@ class nsCOMPtr_helper
   warrant the specialcasing.
 */
 
-class NS_COM_GLUE nsQueryInterface
+class
+  NS_COM_GLUE
+  NS_STACK_CLASS
+  NS_FINAL_CLASS
+nsQueryInterface
   {
     public:
       explicit
@@ -475,7 +482,8 @@ class NS_COM_GLUE nsGetServiceByContractIDWithError
     nsresult*                   mErrorPtr;
 };
 
-class nsCOMPtr_base
+class
+nsCOMPtr_base
     /*
       ...factors implementation for all template versions of |nsCOMPtr|.
 
@@ -535,7 +543,9 @@ class nsCOMPtr_base
 // template <class T> class nsGetterAddRefs;
 
 template <class T>
-class nsCOMPtr
+class
+  NS_FINAL_CLASS
+nsCOMPtr
 #ifdef NSCAP_FEATURE_USE_BASE
     : private nsCOMPtr_base
 #endif

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);
+  }
+}

xpcom/tests/Makefile.in

@@ -53,6 +53,10 @@ ifeq ($(OS_ARCH),WINNT)
 DIRS		+= windows
 endif
 
+ifdef DEHYDRA_PATH
+DIRS += static-checker
+endif
+
 REQUIRES	= \
 		  string \
 		  $(NULL)

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

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;
+};

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;
+};

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();
+}

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>();
+}