From 6cd16c5152afcf00b3097d1326301e84dae55c33 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Tue, 10 Jul 2012 23:13:01 +0000 Subject: [PATCH] [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. llvm-svn: 160029 --- .../Checkers/GenericTaintChecker.cpp | 12 ++- .../Checkers/MacOSKeychainAPIChecker.cpp | 12 ++- .../StaticAnalyzer/Checkers/MallocChecker.cpp | 77 ++++++++++--------- .../StaticAnalyzer/Checkers/StreamChecker.cpp | 2 +- .../Checkers/UnixAPIChecker.cpp | 6 +- clang/test/Analysis/cxx-method-names.cpp | 21 +++++ 6 files changed, 87 insertions(+), 43 deletions(-) create mode 100644 clang/test/Analysis/cxx-method-names.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp index 6b7867c53215..b641c71bc307 100644 --- a/clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp +++ b/clang/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(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/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp index ac41f67d4345..969f2ddeb4ca 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp +++ b/clang/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/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp index f9deb72336f9..1f8ec69b2e82 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp +++ b/clang/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; + if (FD->getKind() == Decl::Function) { + IdentifierInfo *FunI = FD->getIdentifier(); + initIdentifierInfo(C); - 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 @@ -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; + if (FD->getKind() == Decl::Function) { + IdentifierInfo *FunI = FD->getIdentifier(); + initIdentifierInfo(C); - 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 @@ -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/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp index 3745d4ad3943..731dd66b460b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp +++ b/clang/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/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp index 60e665fed3b0..600de659b974 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp +++ b/clang/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/clang/test/Analysis/cxx-method-names.cpp b/clang/test/Analysis/cxx-method-names.cpp new file mode 100644 index 000000000000..8afbb85c1730 --- /dev/null +++ b/clang/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(); +}