Bug 856108 - Port static analyses to clang, part 1: stack-class. r=ehsan

build/clang-plugin/Makefile.in

@@ -16,6 +16,7 @@ CPPSRCS := \
 
 TESTSRCS := \
 	TestMustOverride.cpp \
+	TestStackClass.cpp \
 	$(NULL)
 
 OBJS := $(patsubst %.cpp,%.o,$(CPPSRCS))
@@ -30,7 +31,7 @@ $(OBJS): %.o: %.cpp Makefile
 
 $(PLUGIN): $(OBJS)
 	rm -f $@
-	$(CXX) -shared -o $@ $(CXXFLAGS) $(LDFLAGS) $(OBJS)
+	$(CXX) -shared -o $@ $(CXXFLAGS) $(LDFLAGS) $(OBJS) -lclangASTMatchers
 
 TESTFLAGS := -fsyntax-only -Xclang -verify \
 	-Xclang -load -Xclang $(CURDIR)/$(PLUGIN) \

build/clang-plugin/clang-plugin.cpp

@@ -4,9 +4,12 @@
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/Version.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendPluginRegistry.h"
+#include "clang/Frontend/MultiplexConsumer.h"
 #include "clang/Sema/Sema.h"
 
 #define CLANG_VERSION_FULL (CLANG_VERSION_MAJOR * 100 + CLANG_VERSION_MINOR)
@@ -15,11 +18,37 @@ using namespace llvm;
 using namespace clang;
 
 namespace {
+
+using namespace clang::ast_matchers;
+class DiagnosticsMatcher {
+public:
+  DiagnosticsMatcher();
+
+  ASTConsumer *makeASTConsumer() {
+    return astMatcher.newASTConsumer();
+  }
+
+private:
+  class StackClassChecker : public MatchFinder::MatchCallback {
+  public:
+    virtual void run(const MatchFinder::MatchResult &Result);
+  };
+
+  StackClassChecker stackClassChecker;
+  MatchFinder astMatcher;
+};
+
 class MozChecker : public ASTConsumer, public RecursiveASTVisitor<MozChecker> {
   DiagnosticsEngine &Diag;
   const CompilerInstance &CI;
+  DiagnosticsMatcher matcher;
 public:
   MozChecker(const CompilerInstance &CI) : Diag(CI.getDiagnostics()), CI(CI) {}
+
+  ASTConsumer *getOtherConsumer() {
+    return matcher.makeASTConsumer();
+  }
+
   virtual void HandleTranslationUnit(ASTContext &ctx) {
     TraverseDecl(ctx.getTranslationUnitDecl());
   }
@@ -58,6 +87,16 @@ public:
         if (hasCustomAnnotation(*M, "moz_must_override"))
           must_overrides.push_back(*M);
       }
+
+      // While we are looping over parent classes, we'll also check to make sure
+      // that the subclass has the annotation if the superclass does.
+      if (hasCustomAnnotation(parent, "moz_stack_class") &&
+          !hasCustomAnnotation(d, "moz_stack_class")) {
+        unsigned badInheritID = Diag.getDiagnosticIDs()->getCustomDiagID(
+            DiagnosticIDs::Error, "%0 inherits from a stack class %1");
+        Diag.Report(d->getLocation(), badInheritID)
+          << d->getDeclName() << parent->getDeclName();
+      }
     }
 
     for (OverridesVector::iterator it = must_overrides.begin();
@@ -84,11 +123,78 @@ public:
     return true;
   }
 };
+}
+
+namespace clang {
+namespace ast_matchers {
+
+/// This matcher will match any class with the stack class assertion or an
+/// array of such classes.
+AST_MATCHER(QualType, stackClassAggregate) {
+  QualType t = Node;
+  while (const ArrayType *arrTy = t->getAsArrayTypeUnsafe())
+    t = arrTy->getElementType();
+  CXXRecordDecl *clazz = t->getAsCXXRecordDecl();
+  return clazz && MozChecker::hasCustomAnnotation(clazz, "moz_stack_class");
+}
+}
+}
+
+namespace {
+
+DiagnosticsMatcher::DiagnosticsMatcher() {
+  // Stack class assertion: non-local variables of a stack class are forbidden
+  // (non-localness checked in the callback)
+  astMatcher.addMatcher(varDecl(hasType(stackClassAggregate())).bind("node"),
+    &stackClassChecker);
+  // Stack class assertion: new stack class is forbidden (unless placement new)
+  astMatcher.addMatcher(newExpr(hasType(pointerType(
+      pointee(stackClassAggregate())
+    ))).bind("node"), &stackClassChecker);
+  // Stack class assertion: a stack-class field is permitted only if it's a
+  // member of a class with the annotation
+  astMatcher.addMatcher(fieldDecl(hasType(stackClassAggregate())).bind("field"),
+    &stackClassChecker);
+}
+
+void DiagnosticsMatcher::StackClassChecker::run(
+    const MatchFinder::MatchResult &Result) {
+  DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
+  unsigned stackID = Diag.getDiagnosticIDs()->getCustomDiagID(
+    DiagnosticIDs::Error, "variable of type %0 only valid on the stack");
+  if (const VarDecl *d = Result.Nodes.getNodeAs<VarDecl>("node")) {
+    // Ignore the match if it's a local variable.
+    if (d->hasLocalStorage())
+      return;
+    Diag.Report(d->getLocation(), stackID) << d->getType();
+  } else if (const CXXNewExpr *expr =
+      Result.Nodes.getNodeAs<CXXNewExpr>("node")) {
+    // If it's placement new, then this match doesn't count.
+    if (expr->getNumPlacementArgs() > 0)
+      return;
+    Diag.Report(expr->getStartLoc(), stackID) << expr->getAllocatedType();
+  } else if (const FieldDecl *field =
+      Result.Nodes.getNodeAs<FieldDecl>("field")) {
+    // AST matchers don't let me get the class that contains a field...
+    const RecordDecl *parent = field->getParent();
+    if (!MozChecker::hasCustomAnnotation(parent, "moz_stack_class")) {
+      // We use a more verbose error message here.
+      unsigned stackID = Diag.getDiagnosticIDs()->getCustomDiagID(
+        DiagnosticIDs::Error,
+        "member of type %0 in class %1 that is not a stack class");
+      Diag.Report(field->getLocation(), stackID) << field->getType() <<
+        parent->getDeclName();
+    }
+  }
+}
 
 class MozCheckAction : public PluginASTAction {
 public:
   ASTConsumer *CreateASTConsumer(CompilerInstance &CI, StringRef fileName) {
-    return new MozChecker(CI);
+    MozChecker *checker = new MozChecker(CI);
+
+    ASTConsumer *consumers[] = { checker, checker->getOtherConsumer() };
+    return new MultiplexConsumer(consumers);
   }
 
   bool ParseArgs(const CompilerInstance &CI,

build/clang-plugin/tests/TestStackClass.cpp

@@ -0,0 +1,47 @@
+#define MOZ_STACK_CLASS __attribute__((annotate("moz_stack_class")))
+#include <stddef.h>
+
+struct MOZ_STACK_CLASS Stack {
+  int i;
+  void *operator new(size_t x) { return 0; }
+  void *operator new(size_t blah, char *buffer) { return buffer; }
+};
+
+template <class T>
+struct MOZ_STACK_CLASS TemplateClass {
+  T i;
+};
+
+void gobble(void *) { }
+
+void misuseStackClass(int len) {
+  Stack valid;
+  Stack alsoValid[2];
+  static Stack notValid; // expected-error {{variable of type 'Stack' only valid on the stack}}
+  static Stack alsoNotValid[2]; // expected-error {{variable of type 'Stack [2]' only valid on the stack}}
+
+  gobble(&valid);
+  gobble(&notValid);
+  gobble(&alsoValid[0]);
+
+  gobble(new Stack); // expected-error {{variable of type 'Stack' only valid on the stack}}
+  gobble(new Stack[10]); // expected-error {{variable of type 'Stack' only valid on the stack}}
+  gobble(new TemplateClass<int>); // expected-error {{variable of type 'TemplateClass<int>' only valid on the stack}}
+  gobble(len <= 5 ? &valid : new Stack); // expected-error {{variable of type 'Stack' only valid on the stack}}
+
+  char buffer[sizeof(Stack)];
+  gobble(new (buffer) Stack);
+}
+
+Stack notValid; // expected-error {{variable of type 'Stack' only valid on the stack}}
+struct RandomClass {
+  Stack nonstaticMember; // expected-error {{member of type 'Stack' in class 'RandomClass' that is not a stack class}}
+  static Stack staticMember; // expected-error {{variable of type 'Stack' only valid on the stack}}
+};
+struct MOZ_STACK_CLASS RandomStackClass {
+  Stack nonstaticMember;
+  static Stack staticMember; // expected-error {{variable of type 'Stack' only valid on the stack}}
+};
+
+struct BadInherit : Stack {}; // expected-error {{'BadInherit' inherits from a stack class 'Stack'}}
+struct MOZ_STACK_CLASS GoodInherit : Stack {};

mfbt/Attributes.h

@@ -370,11 +370,19 @@
  *   attribute is not limited to virtual methods, so if it is applied to a
  *   nonvirtual method and the subclass does not provide an equivalent
  *   definition, the compiler will emit an error.
+ * MOZ_STACK_CLASS: Applies to all classes. Any class with this annotation is
+ *   expected to live on the stack, so it is a compile-time error to use it, or
+ *   an array of such objects, as a global or static variable, or as the type of
+ *   a new expression (unless placement new is being used). It may be a base or
+ *   a member of another class only if both classes are marked with this
+ *   annotation.
  */
 #ifdef MOZ_CLANG_PLUGIN
 # define MOZ_MUST_OVERRIDE __attribute__((annotate("moz_must_override")))
+# define MOZ_STACK_CLASS __attribute__((annotate("moz_stack_class")))
 #else
 # define MOZ_MUST_OVERRIDE /* nothing */
+# define MOZ_STACK_CLASS /* nothing */
 #endif /* MOZ_CLANG_PLUGIN */
 
 #endif /* __cplusplus */