[analyzer] Guard against C++ member functions that look like system functions.

C++ method calls and C function calls both appear as CallExprs in the AST.
This was causing crashes for an object that had a 'free' method.

<rdar://problem/11822244>

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

6 files changed, 87 insertions, 43 deletions

diff --git a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
index 6b7867c532..b641c71bc3 100644
--- a/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -299,6 +299,9 @@ void GenericTaintChecker::addSourcesPre(const CallExpr *CE,
                                         CheckerContext &C) const {
   ProgramStateRef State = 0;
   const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+  if (!FDecl || FDecl->getKind() != Decl::Function)
+    return;
+
   StringRef Name = C.getCalleeName(FDecl);
   if (Name.empty())
     return;
@@ -372,7 +375,11 @@ void GenericTaintChecker::addSourcesPost(const CallExpr *CE,
                                          CheckerContext &C) const {
   // Define the attack surface.
   // Set the evaluation function by switching on the callee name.
-  StringRef Name = C.getCalleeName(CE);
+  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+  if (!FDecl || FDecl->getKind() != Decl::Function)
+    return;
+
+  StringRef Name = C.getCalleeName(FDecl);
   if (Name.empty())
     return;
   FnCheck evalFunction = llvm::StringSwitch<FnCheck>(Name)
@@ -406,6 +413,9 @@ bool GenericTaintChecker::checkPre(const CallExpr *CE, CheckerContext &C) const{
     return true;
 
   const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+  if (!FDecl || FDecl->getKind() != Decl::Function)
+    return false;
+
   StringRef Name = C.getCalleeName(FDecl);
   if (Name.empty())
     return false;
diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
index ac41f67d43..969f2ddeb4 100644
--- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
@@ -290,7 +290,11 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE,
   unsigned idx = InvalidIdx;
   ProgramStateRef State = C.getState();
 
-  StringRef funName = C.getCalleeName(CE);
+  const FunctionDecl *FD = C.getCalleeDecl(CE);
+  if (!FD || FD->getKind() != Decl::Function)
+    return;
+  
+  StringRef funName = C.getCalleeName(FD);
   if (funName.empty())
     return;
 
@@ -446,7 +450,11 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE,
 void MacOSKeychainAPIChecker::checkPostStmt(const CallExpr *CE,
                                             CheckerContext &C) const {
   ProgramStateRef State = C.getState();
-  StringRef funName = C.getCalleeName(CE);
+  const FunctionDecl *FD = C.getCalleeDecl(CE);
+  if (!FD || FD->getKind() != Decl::Function)
+    return;
+
+  StringRef funName = C.getCalleeName(FD);
 
   // If a value has been allocated, add it to the set for tracking.
   unsigned idx = getTrackedFunctionIndex(funName, true);
diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index f9deb72336..1f8ec69b2e 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -387,16 +387,15 @@ bool MallocChecker::isAllocationFunction(const FunctionDecl *FD,
   if (!FD)
     return false;
 
-  IdentifierInfo *FunI = FD->getIdentifier();
-  if (!FunI)
-    return false;
-
-  initIdentifierInfo(C);
+  if (FD->getKind() == Decl::Function) {
+    IdentifierInfo *FunI = FD->getIdentifier();
+    initIdentifierInfo(C);
 
-  if (FunI == II_malloc || FunI == II_realloc ||
-      FunI == II_reallocf || FunI == II_calloc || FunI == II_valloc ||
-      FunI == II_strdup || FunI == II_strndup)
-    return true;
+    if (FunI == II_malloc || FunI == II_realloc ||
+        FunI == II_reallocf || FunI == II_calloc || FunI == II_valloc ||
+        FunI == II_strdup || FunI == II_strndup)
+      return true;
+  }
 
   if (Filter.CMallocOptimistic && FD->hasAttrs())
     for (specific_attr_iterator<OwnershipAttr>
@@ -412,14 +411,13 @@ bool MallocChecker::isFreeFunction(const FunctionDecl *FD, ASTContext &C) const
   if (!FD)
     return false;
 
-  IdentifierInfo *FunI = FD->getIdentifier();
-  if (!FunI)
-    return false;
-
-  initIdentifierInfo(C);
+  if (FD->getKind() == Decl::Function) {
+    IdentifierInfo *FunI = FD->getIdentifier();
+    initIdentifierInfo(C);
 
-  if (FunI == II_free || FunI == II_realloc || FunI == II_reallocf)
-    return true;
+    if (FunI == II_free || FunI == II_realloc || FunI == II_reallocf)
+      return true;
+  }
 
   if (Filter.CMallocOptimistic && FD->hasAttrs())
     for (specific_attr_iterator<OwnershipAttr>
@@ -437,29 +435,32 @@ void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext &C) const {
   if (!FD)
     return;
 
-  initIdentifierInfo(C.getASTContext());
-  IdentifierInfo *FunI = FD->getIdentifier();
-  if (!FunI)
-    return;
-
   ProgramStateRef State = C.getState();
-  if (FunI == II_malloc || FunI == II_valloc) {
-    if (CE->getNumArgs() < 1)
-      return;
-    State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
-  } else if (FunI == II_realloc) {
-    State = ReallocMem(C, CE, false);
-  } else if (FunI == II_reallocf) {
-    State = ReallocMem(C, CE, true);
-  } else if (FunI == II_calloc) {
-    State = CallocMem(C, CE);
-  } else if (FunI == II_free) {
-    State = FreeMemAux(C, CE, C.getState(), 0, false);
-  } else if (FunI == II_strdup) {
-    State = MallocUpdateRefState(C, CE, State);
-  } else if (FunI == II_strndup) {
-    State = MallocUpdateRefState(C, CE, State);
-  } else if (Filter.CMallocOptimistic) {
+
+  if (FD->getKind() == Decl::Function) {
+    initIdentifierInfo(C.getASTContext());
+    IdentifierInfo *FunI = FD->getIdentifier();
+
+    if (FunI == II_malloc || FunI == II_valloc) {
+      if (CE->getNumArgs() < 1)
+        return;
+      State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
+    } else if (FunI == II_realloc) {
+      State = ReallocMem(C, CE, false);
+    } else if (FunI == II_reallocf) {
+      State = ReallocMem(C, CE, true);
+    } else if (FunI == II_calloc) {
+      State = CallocMem(C, CE);
+    } else if (FunI == II_free) {
+      State = FreeMemAux(C, CE, State, 0, false);
+    } else if (FunI == II_strdup) {
+      State = MallocUpdateRefState(C, CE, State);
+    } else if (FunI == II_strndup) {
+      State = MallocUpdateRefState(C, CE, State);
+    }
+  }
+
+  if (Filter.CMallocOptimistic) {
     // Check all the attributes, if there are any.
     // There can be multiple of these attributes.
     if (FD->hasAttrs())
diff --git a/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 3745d4ad39..731dd66b46 100644
--- a/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -116,7 +116,7 @@ namespace ento {
 
 bool StreamChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
   const FunctionDecl *FD = C.getCalleeDecl(CE);
-  if (!FD)
+  if (!FD || FD->getKind() != Decl::Function)
     return false;
 
   ASTContext &Ctx = C.getASTContext();
diff --git a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index 60e665fed3..600de659b9 100644
--- a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -325,7 +325,11 @@ void UnixAPIChecker::CheckVallocZero(CheckerContext &C,
 
 void UnixAPIChecker::checkPreStmt(const CallExpr *CE,
                                   CheckerContext &C) const {
-  StringRef FName = C.getCalleeName(CE);
+  const FunctionDecl *FD = C.getCalleeDecl(CE);
+  if (!FD || FD->getKind() != Decl::Function)
+    return;
+
+  StringRef FName = C.getCalleeName(FD);
   if (FName.empty())
     return;
 
diff --git a/test/Analysis/cxx-method-names.cpp b/test/Analysis/cxx-method-names.cpp
new file mode 100644
index 0000000000..8afbb85c17
--- /dev/null
+++ b/test/Analysis/cxx-method-names.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix,osx,experimental.unix,experimental.security.taint -analyzer-store region -verify %s
+
+class Evil {
+public:
+  void system(int); // taint checker
+  void malloc(void *); // taint checker, malloc checker
+  void free(); // malloc checker, keychain checker
+  void fopen(); // stream checker
+  void feof(int, int); // stream checker
+  void open(); // unix api checker
+};
+
+void test(Evil &E) {
+  // no warnings, no crashes
+  E.system(0);
+  E.malloc(0);
+  E.free();
+  E.fopen();
+  E.feof(0,1);
+  E.open();
+}