Index: include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- include/clang/StaticAnalyzer/Checkers/Checkers.td +++ include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -279,6 +279,10 @@ let ParentPackage = CplusplusAlpha in { +def ExceptionMisuseChecker : Checker<"ExceptionMisuse">, + HelpText<"Check for exceptions thrown but never caught or violations of the exception specification of functions">, + DescFile<"ExceptionMisuseChecker.cpp">; + def MisusedMovedObjectChecker: Checker<"MisusedMovedObject">, HelpText<"Method calls on a moved-from object and copying a moved-from " "object will be reported">, Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -34,6 +34,7 @@ DivZeroChecker.cpp DynamicTypePropagation.cpp DynamicTypeChecker.cpp + ExceptionMisuseChecker.cpp ExprInspectionChecker.cpp FixedAddressChecker.cpp GenericTaintChecker.cpp Index: lib/StaticAnalyzer/Checkers/ExceptionMisuseChecker.cpp =================================================================== --- /dev/null +++ lib/StaticAnalyzer/Checkers/ExceptionMisuseChecker.cpp @@ -0,0 +1,495 @@ +#include "ClangSACheckers.h" + +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/Analysis/CallGraph.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Lex/Lexer.h" + +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" + +#include "llvm/ADT/SmallSet.h" + +#include + +using namespace clang; +using namespace ento; + +using llvm::SmallSet; + +namespace { +using Types = std::vector; +typedef std::map FunctionExceptionsMap; + +StringRef BugName = "ExceptionMisuse"; +StringRef BugCategory = "Cpp"; + +class ExceptionMisuseVisitor + : public RecursiveASTVisitor { +public: + explicit ExceptionMisuseVisitor(AnalysisManager &AM, BugReporter &BR, + BugType *BT, StringRef EF, StringRef IE, + bool ReportExnSpec); + ExceptionMisuseVisitor(const ExceptionMisuseVisitor &) = delete; + ExceptionMisuseVisitor &operator=(const ExceptionMisuseVisitor &) = delete; + + bool TraverseCXXTryStmt(CXXTryStmt *); + bool TraverseFunctionDecl(FunctionDecl *); + bool TraverseCXXMethodDecl(CXXMethodDecl *); + bool TraverseCXXConstructorDecl(CXXConstructorDecl *decl); + bool TraverseCXXDestructorDecl(CXXDestructorDecl *decl); + + bool VisitCXXThrowExpr(const CXXThrowExpr *); + bool VisitCallExpr(const CallExpr *); + +private: + bool checkLocation(clang::SourceLocation loc); + + template bool checkLocation(const TNode *node) { + return checkLocation(node->getLocStart()); + } + + void reportBug(const clang::Decl *node, llvm::StringRef message); + + llvm::StringRef getSourceCode(const clang::SourceRange &range); + + template llvm::StringRef getSourceCode(const TNode *node); + + clang::ento::AnalysisManager &getAnalysisManager() { return AMgr; } + + void traverseCore(FunctionDecl *decl, bool isDtorOrMoveOp = false); + bool canCatch(QualType excType, QualType caughtType); + void checkExceptionSpecification(const FunctionDecl *fd, + const Types &exceptions); + std::string getExceptionListAsString(const Types &exceptions) const; + + bool isEnabledFunction(const std::string &name) const { + return EnabledFuncs.count(name); + } + + bool isIgnoredException(const std::string &name) const { + return IgnoredExceptions.count(name); + } + + AnalysisManager &AMgr; + BugReporter &BugRep; + BugType *const BugTy; + bool WasReThrow; + Types CurrentThrows; // actually alive exceptions + FunctionExceptionsMap + ExceptionsThrown; // fn name -> vector of possibly throwed exceptions + std::vector CallStack; // trace call hierarchy + SmallSet EnabledFuncs; // allowed functions + SmallSet IgnoredExceptions; // ignored exceptions + bool ReportExnSpec; +}; + +bool isInSysHeader(clang::SourceLocation loc, + const clang::SourceManager &sourceManager); + +const clang::Decl * +SearchValidEnclosingDecl(clang::ento::AnalysisManager &AMgr, + const clang::ast_type_traits::DynTypedNode &keyNode); + +bool CheckValidEnclosingDeclContextSignature( + const clang::ast_type_traits::DynTypedNode &pNode); + +ExceptionMisuseVisitor::ExceptionMisuseVisitor(AnalysisManager &AM, + BugReporter &BR, BugType *BT, + StringRef EF, StringRef IE, + bool ReportExnSpec) + : AMgr(AM), BugRep(BR), BugTy(BT), WasReThrow(false), + ReportExnSpec(ReportExnSpec) { + // In the beginning we have to parse list formatted options + SmallVector EnabledFuncsVec; + EF.split(EnabledFuncsVec, ";", -1, false); + EnabledFuncs.insert(EnabledFuncsVec.begin(), EnabledFuncsVec.end()); + EnabledFuncs.insert("swap"); + EnabledFuncs.insert("main"); + + SmallVector IgnoredExceptionsVec; + IE.split(IgnoredExceptionsVec, ";", -1, false); + IgnoredExceptions.insert(IgnoredExceptionsVec.begin(), + IgnoredExceptionsVec.end()); + IgnoredExceptions.insert("bad_alloc"); +} + +std::string ExceptionMisuseVisitor::getExceptionListAsString( + const Types &exceptions) const { + std::string exceptionStr; + for (const QualType &type : exceptions) { + exceptionStr.append(type.getAsString()).append(", "); + } + + return exceptionStr; +} + +// An exception handler with the given caught type, can it catch this exception? +bool ExceptionMisuseVisitor::canCatch(QualType excType, QualType caughtType) { + // For subclass relation + const CXXRecordDecl *excTypeDecl = excType->getAsCXXRecordDecl(); + const CXXRecordDecl *cghtTypeDecl = + caughtType->isReferenceType() + ? caughtType->getPointeeType()->getAsCXXRecordDecl() + : caughtType->getAsCXXRecordDecl(); + + // Exact type equality or subclass relation is allowed + return (caughtType == excType || (excTypeDecl && cghtTypeDecl && + excTypeDecl->isDerivedFrom(cghtTypeDecl))); +} + +void ExceptionMisuseVisitor::checkExceptionSpecification( + const FunctionDecl *callee, const Types &exceptions) { + // Analyse functions exception specification (throw(etc.) and noexcept) + if (!ReportExnSpec) + return; // disabled spec checking -> done, go back + + // the function prototype stores the exception related informations, + const auto *calleeProtoType = + dyn_cast_or_null(callee->getType().getTypePtr()); + + // has exception specifition or not + if (!calleeProtoType || !calleeProtoType->hasExceptionSpec()) + return; + + if (calleeProtoType->isNothrow(AMgr.getASTContext(), false) && + !exceptions.empty()) { + reportBug(callee, "This function should not throw any exception " + "because of its non throwing exception specification."); + return; + } + // is all exception are enabled by specification or not + for (auto e : exceptions) { + for (auto excType : calleeProtoType->exceptions()) { + if (excType != e) + continue; + + reportBug(callee, + "This function should not throw " + e.getAsString() + + " exception, because of its exception specification."); + break; + } + } +} + +void ExceptionMisuseVisitor::traverseCore(FunctionDecl *decl, + bool isDtorOrMoveOp) { + // The traverser methods' common part + std::string name = decl->getQualifiedNameAsString(); + const auto *calleeProtoType = + dyn_cast_or_null(decl->getType().getTypePtr()); + + bool shouldNotThrow = isDtorOrMoveOp || + isEnabledFunction(decl->getNameAsString()) || + isEnabledFunction(name); + + // definition and not traversed already and (dtor or main or enabled) + if (decl->isThisDeclarationADefinition() && + ExceptionsThrown.find(name) == ExceptionsThrown.end() && + (shouldNotThrow || + (calleeProtoType && calleeProtoType->hasExceptionSpec()))) { + + CallStack.push_back(name); + RecursiveASTVisitor::TraverseFunctionDecl(decl); + CallStack.pop_back(); + + // store this function as an already visited function + ExceptionsThrown.emplace(name, CurrentThrows); + + if (CurrentThrows.empty()) + return; + + if (shouldNotThrow) { + // throw any exception -> report + reportBug(decl, "This function should not throw any exception:" + + getExceptionListAsString(CurrentThrows)); + } else { + // check the exception specification validity + checkExceptionSpecification(decl, CurrentThrows); + } + + CurrentThrows.clear(); + } +} + +bool ExceptionMisuseVisitor::TraverseCXXMethodDecl(CXXMethodDecl *decl) { + traverseCore(decl, decl->isMoveAssignmentOperator()); + return true; +} + +bool ExceptionMisuseVisitor::TraverseCXXConstructorDecl( + CXXConstructorDecl *decl) { + traverseCore(decl, decl->isMoveConstructor()); + return true; +} + +bool ExceptionMisuseVisitor::TraverseCXXDestructorDecl( + CXXDestructorDecl *decl) { + traverseCore(decl, true); + return true; +} + +bool ExceptionMisuseVisitor::TraverseFunctionDecl(FunctionDecl *decl) { + traverseCore(decl); + return true; +} + +bool ExceptionMisuseVisitor::TraverseCXXTryStmt(CXXTryStmt *C) { + size_t size = CurrentThrows.size(); + TraverseStmt(C->getTryBlock()); + + Types catchedTypes; + Types reThrows; + + for (size_t i = 0; i < C->getNumHandlers(); ++i) { + CXXCatchStmt *catchStmt = C->getHandler(i); + + bool lastReThrow = WasReThrow; + WasReThrow = false; + + TraverseStmt(catchStmt); + + if (catchStmt->getExceptionDecl()) { + // Ordinary catch block + QualType catchedType = catchStmt->getCaughtType() + .getCanonicalType() + .getNonReferenceType() + .getUnqualifiedType(); + + WasReThrow ? reThrows.push_back(catchedType) + : catchedTypes.push_back(catchedType); + } else { + // Catch all: handle (...) + if (!WasReThrow) { + CurrentThrows.erase( + std::remove_if( + CurrentThrows.begin() + size, CurrentThrows.end(), + [this, &reThrows](QualType currentThrowType) { + return !std::any_of( + reThrows.begin(), reThrows.end(), + [this, ¤tThrowType](QualType reThrowCatchType) { + return canCatch(currentThrowType, reThrowCatchType); + }); + }), + CurrentThrows.end()); + } + break; + } + WasReThrow = lastReThrow; + } + + CurrentThrows.erase( + std::remove_if(CurrentThrows.begin() + size, CurrentThrows.end(), + [this, &catchedTypes](QualType currentThrowType) { + return std::any_of( + catchedTypes.begin(), catchedTypes.end(), + [this, ¤tThrowType](QualType catchedType) { + return canCatch(currentThrowType, catchedType); + }); + }), + CurrentThrows.end()); + + return true; +} + +bool ExceptionMisuseVisitor::VisitCXXThrowExpr(const CXXThrowExpr *expr) { + const Expr *throwExpr = expr->getSubExpr(); + + // callstack is empty -> do not care about this exception + if (CallStack.empty()) + return true; + + if (throwExpr) { + QualType excType = expr->getSubExpr() + ->getType() + .getCanonicalType() + .getNonReferenceType() + .getUnqualifiedType(); + + auto excName = excType.getAsString(); + if (const auto *TT = dyn_cast(excType.getTypePtr())) { + excName = TT->getDecl()->getNameAsString(); + } + + // skip ignored exceptions + if (!isIgnoredException(excName)) { + CurrentThrows.push_back(excType); + } + } else { + // no throw expr -> this is a rethrow + WasReThrow = true; + } + return true; +} + +bool ExceptionMisuseVisitor::VisitCallExpr(const CallExpr *C) { + const FunctionDecl *callee = C->getDirectCallee(); + // if is not exist body for this function we do not care about + if (!callee || !callee->hasBody()) { + return true; + } + std::string name = callee->getQualifiedNameAsString(); + + FunctionExceptionsMap::const_iterator fnIt = ExceptionsThrown.find(name); + // already processed? + if (fnIt == ExceptionsThrown.end()) { + // Avoid from recursion(direct and indirect) + if (std::find(CallStack.begin(), CallStack.end(), name) == + CallStack.end()) { + + int currentSize = CurrentThrows.size(); + CallStack.push_back(name); + + RecursiveASTVisitor::TraverseStmt( + const_cast(callee->getBody())); + + CallStack.pop_back(); + + // get the called function's exceptions + Types currentSlice(CurrentThrows.begin() + currentSize, + CurrentThrows.end()); + + if (!currentSlice.empty()) { + // if enabled, lets create a report, else just check exception + // specification violations + if (isEnabledFunction(callee->getNameAsString()) || + isEnabledFunction(name)) { + reportBug(callee, "This function should not throw any exception: " + + getExceptionListAsString(currentSlice)); + } else { + checkExceptionSpecification(callee, currentSlice); + } + } + // store this function as already visited + ExceptionsThrown.emplace(name, std::move(currentSlice)); + } + + } else { + // just push those exception into the current + CurrentThrows.insert(CurrentThrows.end(), fnIt->second.begin(), + fnIt->second.end()); + } + + return true; +} + +void ExceptionMisuseVisitor::reportBug(const Decl *node, StringRef message) { + if (!checkLocation(node->getLocStart())) + return; + + PathDiagnosticLocation loc = + PathDiagnosticLocation(node, AMgr.getSourceManager()); + SourceRange range = SourceRange(node->getLocation()); + + ast_type_traits::DynTypedNode keyNode = + ast_type_traits::DynTypedNode::create(*node); + + const Decl *parentDecl = nullptr; + parentDecl = SearchValidEnclosingDecl(AMgr, keyNode); + + auto bug_report = llvm::make_unique(*BugTy, message, loc); + bug_report->setDeclWithIssue(parentDecl); + bug_report->addRange(range); + BugRep.emitReport(std::move(bug_report)); +} + +StringRef ExceptionMisuseVisitor::getSourceCode(const SourceRange &range) { + return Lexer::getSourceText(CharSourceRange::getCharRange(range), + AMgr.getSourceManager(), AMgr.getLangOpts()); +} + +template +llvm::StringRef ExceptionMisuseVisitor::getSourceCode(const TNode *node) { + clang::SourceRange range; + range.setBegin(node->getLocStart()); + + // getLocEnd() returns the beginning of the last token + range.setEnd( + node->getLocEnd().getLocWithOffset(clang::Lexer::MeasureTokenLength( + node->getLocEnd(), AMgr.getSourceManager(), AMgr.getLangOpts()))); + return getSourceCode(range); +} + +bool ExceptionMisuseVisitor::checkLocation(SourceLocation loc) { + if (!loc.isValid()) + return false; + + if (isInSysHeader(loc, AMgr.getSourceManager())) + return false; + + return true; +} + +class ExceptionMisuseChecker : public Checker { + +public: + void checkEndOfTranslationUnit(const TranslationUnitDecl *TU, + AnalysisManager &AMgr, BugReporter &BR) const { + auto &Opts = AMgr.getAnalyzerOptions(); + BugTy = llvm::make_unique(this, BugName, BugCategory); + ExceptionMisuseVisitor Visitor( + AMgr, BR, BugTy.get(), + Opts.getOptionAsString("enabled_functions", "", this), + Opts.getOptionAsString("ignored_exceptions", "", this), + Opts.getBooleanOption("report_exn_spec", true, this)); + Visitor.TraverseDecl(const_cast(TU)); + } + +private: + mutable std::unique_ptr BugTy; +}; + +bool isInSysHeader(SourceLocation loc, const SourceManager &sourceManager) { + return sourceManager.isInSystemHeader(loc) || + sourceManager.isInExternCSystemHeader(loc); +} + +const Decl * +SearchValidEnclosingDecl(AnalysisManager &AMgr, + const ast_type_traits::DynTypedNode &keyNode) { + + // go up in the AST searching for valid enclosing declarations + ast_type_traits::DynTypedNode pNode = + AMgr.getASTContext().getParents(keyNode)[0]; + + while (!AMgr.getASTContext().getParents(pNode).empty() && + !(CheckValidEnclosingDeclContextSignature(pNode))) { + pNode = AMgr.getASTContext().getParents(pNode)[0]; + } + return pNode.get(); +} + +bool CheckValidEnclosingDeclContextSignature( + const ast_type_traits::DynTypedNode &pNode) { + + const auto *D = pNode.get(); + if (D == nullptr) + return false; + // Valid enclosing decls which can be used by the hash generator + if (const auto *ND = dyn_cast(D)) { + switch (ND->getKind()) { + case Decl::Namespace: + case Decl::Record: + case Decl::CXXRecord: + case Decl::Enum: + case Decl::CXXConstructor: + case Decl::CXXDestructor: + case Decl::CXXConversion: + case Decl::CXXMethod: + case Decl::Function: + case Decl::ObjCMethod: + return true; + default: + return false; + } + } + return false; +} + +} // end anonymous namespace + +void ento::registerExceptionMisuseChecker(CheckerManager &AMgr) { + AMgr.registerChecker(); +} Index: test/Analysis/exception-misuse.cpp =================================================================== --- /dev/null +++ test/Analysis/exception-misuse.cpp @@ -0,0 +1,247 @@ +// RUN: %clang_analyze_cc1 -std=c++11 -fcxx-exceptions -analyzer-checker=core,cplusplus,alpha.cplusplus.ExceptionMisuse -analyzer-config alpha.cplusplus.ExceptionMisuse:enabled_functions="test2;test3;test4;f;g" -analyzer-config alpha.cplusplus.ExceptionMisuse:ignored_exceptions="E1;E2" %s -verify + +struct X { + ~X() { + try { + try { + } catch (int& a) { + throw 42; + } + } catch (...) { + } + } +}; + +struct Y { + char data; + + Y(Y&&) { // expected-warning{{This function should not throw}} + throw data; + } + + Y& operator=(Y&&) { // expected-warning{{This function should not throw}} + throw data; + } + + ~Y() { // expected-warning{{This function should not throw}} + // nesting + if (data == 'A') throw data; + bar(); + } + void bar(); +}; + +struct MyExceptionBase {}; +struct MyException : public MyExceptionBase {}; + +void control_func() { + throw 1024; +} +void dummy_func(); +void proxy_func() { + control_func(); + throw "MyException"; +} + +struct Z { + void control() { throw 'Z'; } + + ~Z() { // expected-warning{{This function should not throw}} + try { + proxy_func(); + } catch (int& a) { + } catch (...) { + throw; + } + } +}; + +void testFn() { + try { + try { + try { + MyException myexce; + throw myexce; + } catch (int a) { + dummy_func(); + throw; + } + } catch (MyException m) { + } + } catch (...) { + } +} + +struct base {}; +struct buffer : public base {}; + +struct P { + ~P() { + try { + testFn(); + throw 44; + } catch (int& a) { + } catch (...) { + } + } +}; + +struct V { + ~V() { // expected-warning{{This function should not throw}} + try { + try { + MyException myexce; + throw myexce; + } catch (int a) { + dummy_func(); + } + } catch (const MyExceptionBase& m) { + throw; + } catch (buffer) { + } catch (base) { + proxy_func(); + } catch (int& a) { + } catch (int* a) { + } catch (...) { + } + } +}; + +struct W { + ~W() { // expected-warning{{This function should not throw}} + try { + try { + int a = 44; + throw a; + double d = 4.44; + throw d; + testFn(); + } catch (...) { + throw; + } + } catch (int a) { + + } catch (...) { + throw; + } + } +}; + +struct Q { + Q(bool b) { _b = b; } + ~Q() { + try { + buffer b; + if (_b) throw b; + } catch (...) { + } + } + + private: + bool _b; +}; + +namespace N { +void swap(int&, int&) { throw 5; } // expected-warning{{This function should not throw}} + +struct S { + void swap(S& other) { throw 5; } // expected-warning{{This function should not throw}} +}; +} + +int test1() throw() { // expected-warning{{This function should not throw}} + throw 7; + return 0; +} + +void fun2(); +int test2() { // expected-warning{{This function should not throw}} + fun2(); + return 0; +} +void fun2() { throw 7; } + +namespace test3Ns { + void fun3() { throw 7; } + void f() { fun3(); } // expected-warning{{This function should not throw}} +} + +class A {}; +class B {}; +class C {}; + +int test4() { // expected-warning{{This function should not throw}} + try { + throw A(); + } catch (B b) { + } catch (C b) { + } + return 0; +} + +class ExceptionBase {}; +class ExceptionDerived : public ExceptionBase {}; +class ExceptionOther : public ExceptionBase {}; + +void g(int x); + +void h(int x) noexcept(true); // expected-warning{{This function should not throw}} + +void f(int x) throw(char) { + try { + h(x); + } catch (ExceptionOther* d) { + } catch (ExceptionBase* d) { + } catch (double& d) { + } catch (...) { + } +} + +void g(int x) { // expected-warning{{This function should not throw}} + f(x); + throw 9; +} + +void h(int x) noexcept { + try { + g(x); + } catch (int a) { + } + double y = 2.2; + throw 2.2; +} + +// Direct recursion test +namespace Next { +int f(int x) { // expected-warning{{This function should not throw}} + if (x < 1) throw 0; + if (x == 1) + return 1; + else + return x* f(x - 1); +} + +void h(); + +void g() { h(); } + +void h() { g(); } +} + +class E1 {}; +class E2 {}; + +void ignored1() throw() { + throw E1(); // no-warning +} + +void ignored2() throw() { + throw E2(); // no-warning +} + +int main() { + int x = 2; + f(x); + + return 0; +}