Bug 867348 - Part 1: Add an analysis that catches calls to certain functions involving arithmetic expressions; r=jrmuizel
authorEhsan Akhgari <ehsan@mozilla.com>
Thu, 18 Dec 2014 15:25:15 -0500
changeset 220566 a07116d74024426c192a5b85df52f0ef7eae6e54
parent 220565 30f9b7719d69714d8383a9aeab8fc7bf259d17c7
child 220567 c9b0bbb1d4f9565da1b91e47029bbc9a7c773e63
push id53144
push usereakhgari@mozilla.com
push dateFri, 19 Dec 2014 18:11:30 +0000
treeherdermozilla-inbound@c9b0bbb1d4f9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs867348
milestone37.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 867348 - Part 1: Add an analysis that catches calls to certain functions involving arithmetic expressions; r=jrmuizel
build/clang-plugin/clang-plugin.cpp
build/clang-plugin/tests/TestNoArithmeticExprInArgument.cpp
build/clang-plugin/tests/moz.build
--- a/build/clang-plugin/clang-plugin.cpp
+++ b/build/clang-plugin/clang-plugin.cpp
@@ -46,18 +46,24 @@ private:
   };
 
   class NonHeapClassChecker : public MatchFinder::MatchCallback {
   public:
     virtual void run(const MatchFinder::MatchResult &Result);
     void noteInferred(QualType T, DiagnosticsEngine &Diag);
   };
 
+  class ArithmeticArgChecker : public MatchFinder::MatchCallback {
+  public:
+    virtual void run(const MatchFinder::MatchResult &Result);
+  };
+
   StackClassChecker stackClassChecker;
   NonHeapClassChecker nonheapClassChecker;
+  ArithmeticArgChecker arithmeticArgChecker;
   MatchFinder astMatcher;
 };
 
 namespace {
 
 bool isInIgnoredNamespace(const Decl *decl) {
   const DeclContext *DC = decl->getDeclContext()->getEnclosingNamespaceContext();
   const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(DC);
@@ -324,16 +330,59 @@ AST_MATCHER(QualType, nonheapClassAggreg
   return getClassAttrs(Node) == NonHeapClass;
 }
 
 /// This matcher will match any function declaration that is declared as a heap
 /// allocator.
 AST_MATCHER(FunctionDecl, heapAllocator) {
   return MozChecker::hasCustomAnnotation(&Node, "moz_heap_allocator");
 }
+
+/// This matcher will match any declaration that is marked as not accepting
+/// arithmetic expressions in its arguments.
+AST_MATCHER(Decl, noArithmeticExprInArgs) {
+  return MozChecker::hasCustomAnnotation(&Node, "moz_no_arith_expr_in_arg");
+}
+
+/// This matcher will match all arithmetic binary operators.
+AST_MATCHER(BinaryOperator, binaryArithmeticOperator) {
+  BinaryOperatorKind opcode = Node.getOpcode();
+  return opcode == BO_Mul ||
+         opcode == BO_Div ||
+         opcode == BO_Rem ||
+         opcode == BO_Add ||
+         opcode == BO_Sub ||
+         opcode == BO_Shl ||
+         opcode == BO_Shr ||
+         opcode == BO_And ||
+         opcode == BO_Xor ||
+         opcode == BO_Or ||
+         opcode == BO_MulAssign ||
+         opcode == BO_DivAssign ||
+         opcode == BO_RemAssign ||
+         opcode == BO_AddAssign ||
+         opcode == BO_SubAssign ||
+         opcode == BO_ShlAssign ||
+         opcode == BO_ShrAssign ||
+         opcode == BO_AndAssign ||
+         opcode == BO_XorAssign ||
+         opcode == BO_OrAssign;
+}
+
+/// This matcher will match all arithmetic unary operators.
+AST_MATCHER(UnaryOperator, unaryArithmeticOperator) {
+  UnaryOperatorKind opcode = Node.getOpcode();
+  return opcode == UO_PostInc ||
+         opcode == UO_PostDec ||
+         opcode == UO_PreInc ||
+         opcode == UO_PreDec ||
+         opcode == UO_Plus ||
+         opcode == UO_Minus ||
+         opcode == UO_Not;
+}
 }
 }
 
 namespace {
 
 bool isPlacementNew(const CXXNewExpr *expr) {
   // Regular new expressions aren't placement new
   if (expr->getNumPlacementArgs() == 0)
@@ -362,16 +411,43 @@ DiagnosticsMatcher::DiagnosticsMatcher()
   // Any heap allocation function that returns a non-heap or a stack class is
   // definitely doing something wrong
   astMatcher.addMatcher(callExpr(callee(functionDecl(allOf(heapAllocator(),
       returns(pointerType(pointee(nonheapClassAggregate()))))))).bind("node"),
     &nonheapClassChecker);
   astMatcher.addMatcher(callExpr(callee(functionDecl(allOf(heapAllocator(),
       returns(pointerType(pointee(stackClassAggregate()))))))).bind("node"),
     &stackClassChecker);
+
+  astMatcher.addMatcher(callExpr(allOf(hasDeclaration(noArithmeticExprInArgs()),
+          anyOf(
+              hasDescendant(binaryOperator(allOf(binaryArithmeticOperator(),
+                  hasLHS(hasDescendant(declRefExpr())),
+                  hasRHS(hasDescendant(declRefExpr()))
+              )).bind("node")),
+              hasDescendant(unaryOperator(allOf(unaryArithmeticOperator(),
+                  hasUnaryOperand(allOf(hasType(builtinType()),
+                                        anyOf(hasDescendant(declRefExpr()), declRefExpr())))
+              )).bind("node"))
+          )
+      )).bind("call"),
+    &arithmeticArgChecker);
+  astMatcher.addMatcher(constructExpr(allOf(hasDeclaration(noArithmeticExprInArgs()),
+          anyOf(
+              hasDescendant(binaryOperator(allOf(binaryArithmeticOperator(),
+                  hasLHS(hasDescendant(declRefExpr())),
+                  hasRHS(hasDescendant(declRefExpr()))
+              )).bind("node")),
+              hasDescendant(unaryOperator(allOf(unaryArithmeticOperator(),
+                  hasUnaryOperand(allOf(hasType(builtinType()),
+                                        anyOf(hasDescendant(declRefExpr()), declRefExpr())))
+              )).bind("node"))
+          )
+      )).bind("call"),
+    &arithmeticArgChecker);
 }
 
 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")) {
@@ -467,16 +543,29 @@ void DiagnosticsMatcher::NonHeapClassChe
   } else if (const FieldDecl *FD = dyn_cast<FieldDecl>(cause)) {
     Diag.Report(FD->getLocation(), memberID) << T << FD << FD->getType();
   }
   
   // Recursively follow this back.
   noteInferred(cast<ValueDecl>(cause)->getType(), Diag);
 }
 
+void DiagnosticsMatcher::ArithmeticArgChecker::run(
+    const MatchFinder::MatchResult &Result) {
+  DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
+  unsigned errorID = Diag.getDiagnosticIDs()->getCustomDiagID(
+      DiagnosticIDs::Error, "cannot pass an arithmetic expression of built-in types to %0");
+  const Expr *expr = Result.Nodes.getNodeAs<Expr>("node");
+  if (const CallExpr *call = Result.Nodes.getNodeAs<CallExpr>("call")) {
+    Diag.Report(expr->getLocStart(), errorID) << call->getDirectCallee();
+  } else if (const CXXConstructExpr *ctr = Result.Nodes.getNodeAs<CXXConstructExpr>("call")) {
+    Diag.Report(expr->getLocStart(), errorID) << ctr->getConstructor();
+  }
+}
+
 class MozCheckAction : public PluginASTAction {
 public:
   ASTConsumerPtr CreateASTConsumer(CompilerInstance &CI, StringRef fileName) override {
 #if CLANG_VERSION_FULL >= 306
     std::unique_ptr<MozChecker> checker(make_unique<MozChecker>(CI));
 
     std::vector<std::unique_ptr<ASTConsumer>> consumers;
     consumers.push_back(std::move(checker));
new file mode 100644
--- /dev/null
+++ b/build/clang-plugin/tests/TestNoArithmeticExprInArgument.cpp
@@ -0,0 +1,32 @@
+#define MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT __attribute__((annotate("moz_no_arith_expr_in_arg")))
+
+struct X {
+  explicit X(int) MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT;
+  void baz(int) MOZ_NO_ARITHMETIC_EXPR_IN_ARGUMENT;
+};
+
+int operator+(int, X);
+int operator+(X, int);
+int operator++(X);
+
+void badArithmeticsInArgs() {
+  int a;
+  typedef int myint;
+  myint b;
+  X goodObj1(a);
+  goodObj1.baz(b);
+  X badObj1(a + b); // expected-error{{cannot pass an arithmetic expression of built-in types to 'X'}}
+  X badObj2 = X(a ? 0 : ++a); // expected-error{{cannot pass an arithmetic expression of built-in types to 'X'}}
+  X badObj3(~a); // expected-error{{cannot pass an arithmetic expression of built-in types to 'X'}}
+  badObj1.baz(a - 1 - b); // expected-error{{cannot pass an arithmetic expression of built-in types to 'baz'}}
+  badObj1.baz(++a); // expected-error{{cannot pass an arithmetic expression of built-in types to 'baz'}}
+  badObj1.baz(a++); // expected-error{{cannot pass an arithmetic expression of built-in types to 'baz'}}
+  badObj1.baz(a || b);
+  badObj1.baz(a + goodObj1);
+  badObj1.baz(goodObj1 + a);
+  badObj1.baz(++goodObj1);
+  badObj1.baz(-1);
+  badObj1.baz(-1.0);
+  badObj1.baz(1 + 2);
+  badObj1.baz(1 << (sizeof(int)/2));
+}
--- a/build/clang-plugin/tests/moz.build
+++ b/build/clang-plugin/tests/moz.build
@@ -3,14 +3,15 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 SOURCES += [
     'TestBadImplicitConversionCtor.cpp',
     'TestCustomHeap.cpp',
     'TestMustOverride.cpp',
+    'TestNoArithmeticExprInArgument.cpp',
     'TestNonHeapClass.cpp',
     'TestStackClass.cpp',
 ]
 
 DISABLE_STL_WRAPPING = True
 NO_VISIBILITY_FLAGS = True