Add LLVM conventions check that scans for AST elements (types, stmts, decls)

that allocate heap memory.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@96184 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 194 insertions, 13 deletions

diff --git a/lib/Checker/LLVMConventionsChecker.cpp b/lib/Checker/LLVMConventionsChecker.cpp
index 17a17a8bbf..242f4de2b6 100644
--- a/lib/Checker/LLVMConventionsChecker.cpp
+++ b/lib/Checker/LLVMConventionsChecker.cpp
@@ -12,12 +12,12 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Checker/Checkers/LocalCheckers.h"
 #include "clang/Checker/BugReporter/BugReporter.h"
 #include <string>
-#include <llvm/ADT/StringRef.h>
+#include "llvm/ADT/StringRef.h"
 
 using namespace clang;
 
@@ -25,13 +25,25 @@ using namespace clang;
 // Generic type checking routines.
 //===----------------------------------------------------------------------===//
 
-static bool IsStringRef(QualType T) {
+static bool IsLLVMStringRef(QualType T) {
   const RecordType *RT = T->getAs<RecordType>();
   if (!RT)
     return false;
 
   return llvm::StringRef(QualType(RT, 0).getAsString()) ==
-  "class llvm::StringRef";
+          "class llvm::StringRef";
+}
+
+static bool InStdNamespace(const Decl *D) {
+  const DeclContext *DC = D->getDeclContext();
+  const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(D->getDeclContext());
+  if (!ND)
+    return false;
+  const IdentifierInfo *II = ND->getIdentifier();
+  if (!II || II->getName() != "std")
+    return false;
+  DC = ND->getDeclContext();
+  return isa<TranslationUnitDecl>(DC);
 }
 
 static bool IsStdString(QualType T) {
@@ -43,15 +55,75 @@ static bool IsStdString(QualType T) {
     return false;
 
   const TypedefDecl *TD = TT->getDecl();
-  const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(TD->getDeclContext());
+
+  if (!InStdNamespace(TD))
+    return false;
+
+  return TD->getName() == "string";
+}
+
+static bool InClangNamespace(const Decl *D) {
+  const DeclContext *DC = D->getDeclContext();
+  const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(D->getDeclContext());
   if (!ND)
     return false;
   const IdentifierInfo *II = ND->getIdentifier();
-  if (!II || II->getName() != "std")
+  if (!II || II->getName() != "clang")
+    return false;
+  DC = ND->getDeclContext();
+  return isa<TranslationUnitDecl>(DC);
+}
+
+static bool InLLVMNamespace(const Decl *D) {
+  const DeclContext *DC = D->getDeclContext();
+  const NamespaceDecl *ND = dyn_cast<NamespaceDecl>(D->getDeclContext());
+  if (!ND)
     return false;
+  const IdentifierInfo *II = ND->getIdentifier();
+  if (!II || II->getName() != "llvm")
+    return false;
+  DC = ND->getDeclContext();
+  return isa<TranslationUnitDecl>(DC);
+}
 
-  DeclarationName N = TD->getDeclName();
-  return llvm::StringRef(N.getAsString()) == "string";
+static bool IsClangType(const RecordDecl *RD) {
+  return RD->getName() == "Type" && InClangNamespace(RD);
+}
+
+static bool IsClangDecl(const RecordDecl *RD) {
+  return RD->getName() == "Decl" && InClangNamespace(RD);
+}
+
+static bool IsClangStmt(const RecordDecl *RD) {
+  return RD->getName() == "Stmt" && InClangNamespace(RD);
+}
+
+static bool IsStdVector(QualType T) {
+  const TemplateSpecializationType *TS = T->getAs<TemplateSpecializationType>();
+  if (!TS)
+    return false;
+
+  TemplateName TM = TS->getTemplateName();
+  TemplateDecl *TD = TM.getAsTemplateDecl();
+
+  if (!TD || !InStdNamespace(TD))
+    return false;
+
+  return TD->getName() == "vector";
+}
+
+static bool IsSmallVector(QualType T) {
+  const TemplateSpecializationType *TS = T->getAs<TemplateSpecializationType>();
+  if (!TS)
+    return false;
+
+  TemplateName TM = TS->getTemplateName();
+  TemplateDecl *TD = TM.getAsTemplateDecl();
+
+  if (!TD || !InLLVMNamespace(TD))
+    return false;
+
+  return TD->getName() == "SmallVector";
 }
 
 //===----------------------------------------------------------------------===//
@@ -98,7 +170,7 @@ void StringRefCheckerVisitor::VisitVarDecl(VarDecl *VD) {
 
   // Pattern match for:
   // llvm::StringRef x = call() (where call returns std::string)
-  if (!IsStringRef(VD->getType()))
+  if (!IsLLVMStringRef(VD->getType()))
     return;
   CXXExprWithTemporaries *Ex1 = dyn_cast<CXXExprWithTemporaries>(Init);
   if (!Ex1)
@@ -120,12 +192,117 @@ void StringRefCheckerVisitor::VisitVarDecl(VarDecl *VD) {
     return;
 
   // Okay, badness!  Report an error.
-  BR.EmitBasicReport("StringRef should not be bound to temporary "
-                     "std::string that it outlives", "LLVM Conventions",
+  const char *desc = "StringRef should not be bound to temporary "
+                     "std::string that it outlives";
+
+  BR.EmitBasicReport(desc, "LLVM Conventions", desc,
                      VD->getLocStart(), Init->getSourceRange());
 }
 
 //===----------------------------------------------------------------------===//
+// CHECK: Clang AST nodes should not have fields that can allocate
+//   memory.
+//===----------------------------------------------------------------------===//
+
+static bool AllocatesMemory(QualType T) {
+  return IsStdVector(T) || IsStdString(T) || IsSmallVector(T);
+}
+
+// This type checking could be sped up via dynamic programming.
+static bool IsPartOfAST(const CXXRecordDecl *R) {
+  if (IsClangStmt(R) || IsClangType(R) || IsClangDecl(R))
+    return true;
+
+  for (CXXRecordDecl::base_class_const_iterator I = R->bases_begin(),
+                                                E = R->bases_end(); I!=E; ++I) {
+    CXXBaseSpecifier BS = *I;
+    QualType T = BS.getType();
+    if (const RecordType *baseT = T->getAs<RecordType>()) {
+      CXXRecordDecl *baseD = cast<CXXRecordDecl>(baseT->getDecl());
+      if (IsPartOfAST(baseD))
+        return true;
+    }
+  }
+
+  return false;
+}
+
+namespace {
+class ASTFieldVisitor {
+  llvm::SmallVector<FieldDecl*, 10> FieldChain;
+  CXXRecordDecl *Root;
+  BugReporter &BR;
+public:
+  ASTFieldVisitor(CXXRecordDecl *root, BugReporter &br)
+    : Root(root), BR(br) {}
+
+  void Visit(FieldDecl *D);
+  void ReportError(QualType T);
+};
+} // end anonymous namespace
+
+static void CheckASTMemory(CXXRecordDecl *R, BugReporter &BR) {
+  if (!IsPartOfAST(R))
+    return;
+
+  for (RecordDecl::field_iterator I = R->field_begin(), E = R->field_end();
+       I != E; ++I) {
+    ASTFieldVisitor walker(R, BR);
+    walker.Visit(*I);
+  }
+}
+
+void ASTFieldVisitor::Visit(FieldDecl *D) {
+  FieldChain.push_back(D);
+
+  QualType T = D->getType();
+
+  if (AllocatesMemory(T))
+    ReportError(T);
+
+  if (const RecordType *RT = T->getAs<RecordType>()) {
+    const RecordDecl *RD = RT->getDecl()->getDefinition();
+    for (RecordDecl::field_iterator I = RD->field_begin(), E = RD->field_end();
+         I != E; ++I)
+      Visit(*I);
+  }
+
+  FieldChain.pop_back();
+}
+
+void ASTFieldVisitor::ReportError(QualType T) {
+  llvm::SmallString<1024> buf;
+  llvm::raw_svector_ostream os(buf);
+
+  os << "AST class '" << Root->getName() << "' has a field '"
+     << FieldChain.front()->getName() << "' that allocates heap memory";
+  if (FieldChain.size() > 1) {
+    os << " via the following chain: ";
+    bool isFirst = true;
+    for (llvm::SmallVectorImpl<FieldDecl*>::iterator I=FieldChain.begin(),
+         E=FieldChain.end(); I!=E; ++I) {
+      if (!isFirst)
+        os << '.';
+      else
+        isFirst = false;
+      os << (*I)->getName();
+    }
+  }
+  os << " (type " << FieldChain.back()->getType().getAsString() << ")";
+  os.flush();
+
+  // Note that this will fire for every translation unit that uses this
+  // class.  This is suboptimal, but at least scan-build will merge
+  // duplicate HTML reports.  In the future we need a unified way of merging
+  // duplicate reports across translation units.  For C++ classes we cannot
+  // just report warnings when we see an out-of-line method definition for a
+  // class, as that heuristic doesn't always work (the complete definition of
+  // the class may be in the header file, for example).
+  BR.EmitBasicReport("AST node allocates heap memory", "LLVM Conventions",
+                     os.str(), FieldChain.front()->getLocStart());
+}
+
+//===----------------------------------------------------------------------===//
 // Entry point for all checks.
 //===----------------------------------------------------------------------===//
 
@@ -134,9 +311,13 @@ static void ScanCodeDecls(DeclContext *DC, BugReporter &BR) {
        I!=E ; ++I) {
 
     Decl *D = *I;
-    if (D->getBody()) {
+
+    if (D->getBody())
       CheckStringRefAssignedTemporary(D, BR);
-    }
+
+    if (CXXRecordDecl *R = dyn_cast<CXXRecordDecl>(D))
+      if (R->isDefinition())
+        CheckASTMemory(R, BR);
 
     if (DeclContext *DC_child = dyn_cast<DeclContext>(D))
       ScanCodeDecls(DC_child, BR);