Index: clang-tidy/bugprone/ExceptionEscapeCheck.h =================================================================== --- clang-tidy/bugprone/ExceptionEscapeCheck.h +++ clang-tidy/bugprone/ExceptionEscapeCheck.h @@ -1,4 +1,4 @@ -//===--- ExceptionEscapeCheck.h - clang-tidy---------------------*- C++ -*-===// +//===--- ExceptionEscapeCheck.h - clang-tidy --------------------*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -10,7 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_EXCEPTION_ESCAPE_H #include "../ClangTidy.h" - +#include "../utils/ExceptionAnalyzer.h" #include "llvm/ADT/StringSet.h" namespace clang { @@ -36,7 +36,7 @@ std::string RawIgnoredExceptions; llvm::StringSet<> FunctionsThatShouldNotThrow; - llvm::StringSet<> IgnoredExceptions; + utils::ExceptionAnalyzer Tracer; }; } // namespace bugprone Index: clang-tidy/bugprone/ExceptionEscapeCheck.cpp =================================================================== --- clang-tidy/bugprone/ExceptionEscapeCheck.cpp +++ clang-tidy/bugprone/ExceptionEscapeCheck.cpp @@ -1,4 +1,4 @@ -//===--- ExceptionEscapeCheck.cpp - clang-tidy-----------------------------===// +//===--- ExceptionEscapeCheck.cpp - clang-tidy ----------------------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -10,158 +10,21 @@ #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" - #include "llvm/ADT/SmallSet.h" #include "llvm/ADT/StringSet.h" using namespace clang::ast_matchers; -namespace { -typedef llvm::SmallVector TypeVec; -} // namespace - namespace clang { - -static bool isBaseOf(const Type *DerivedType, const Type *BaseType) { - const auto *DerivedClass = DerivedType->getAsCXXRecordDecl(); - const auto *BaseClass = BaseType->getAsCXXRecordDecl(); - if (!DerivedClass || !BaseClass) - return false; - - return !DerivedClass->forallBases( - [BaseClass](const CXXRecordDecl *Cur) { return Cur != BaseClass; }); -} - -static const TypeVec -throwsException(const Stmt *St, const TypeVec &Caught, - llvm::SmallSet &CallStack); - -static const TypeVec -throwsException(const FunctionDecl *Func, - llvm::SmallSet &CallStack) { - if (CallStack.count(Func)) - return TypeVec(); - - if (const Stmt *Body = Func->getBody()) { - CallStack.insert(Func); - const TypeVec Result = throwsException(Body, TypeVec(), CallStack); - CallStack.erase(Func); - return Result; - } - - TypeVec Result; - if (const auto *FPT = Func->getType()->getAs()) { - for (const QualType Ex : FPT->exceptions()) { - Result.push_back(Ex.getTypePtr()); - } - } - return Result; -} - -static const TypeVec -throwsException(const Stmt *St, const TypeVec &Caught, - llvm::SmallSet &CallStack) { - TypeVec Results; - - if (!St) - return Results; - - if (const auto *Throw = dyn_cast(St)) { - if (const auto *ThrownExpr = Throw->getSubExpr()) { - const auto *ThrownType = - ThrownExpr->getType()->getUnqualifiedDesugaredType(); - if (ThrownType->isReferenceType()) { - ThrownType = ThrownType->castAs() - ->getPointeeType() - ->getUnqualifiedDesugaredType(); - } - if (const auto *TD = ThrownType->getAsTagDecl()) { - if (TD->getDeclName().isIdentifier() && TD->getName() == "bad_alloc" - && TD->isInStdNamespace()) - return Results; - } - Results.push_back(ThrownExpr->getType()->getUnqualifiedDesugaredType()); - } else { - Results.append(Caught.begin(), Caught.end()); - } - } else if (const auto *Try = dyn_cast(St)) { - TypeVec Uncaught = throwsException(Try->getTryBlock(), Caught, CallStack); - for (unsigned i = 0; i < Try->getNumHandlers(); ++i) { - const CXXCatchStmt *Catch = Try->getHandler(i); - if (!Catch->getExceptionDecl()) { - const TypeVec Rethrown = - throwsException(Catch->getHandlerBlock(), Uncaught, CallStack); - Results.append(Rethrown.begin(), Rethrown.end()); - Uncaught.clear(); - } else { - const auto *CaughtType = - Catch->getCaughtType()->getUnqualifiedDesugaredType(); - if (CaughtType->isReferenceType()) { - CaughtType = CaughtType->castAs() - ->getPointeeType() - ->getUnqualifiedDesugaredType(); - } - auto NewEnd = - llvm::remove_if(Uncaught, [&CaughtType](const Type *ThrownType) { - return ThrownType == CaughtType || - isBaseOf(ThrownType, CaughtType); - }); - if (NewEnd != Uncaught.end()) { - Uncaught.erase(NewEnd, Uncaught.end()); - const TypeVec Rethrown = throwsException( - Catch->getHandlerBlock(), TypeVec(1, CaughtType), CallStack); - Results.append(Rethrown.begin(), Rethrown.end()); - } - } - } - Results.append(Uncaught.begin(), Uncaught.end()); - } else if (const auto *Call = dyn_cast(St)) { - if (const FunctionDecl *Func = Call->getDirectCallee()) { - TypeVec Excs = throwsException(Func, CallStack); - Results.append(Excs.begin(), Excs.end()); - } - } else { - for (const Stmt *Child : St->children()) { - TypeVec Excs = throwsException(Child, Caught, CallStack); - Results.append(Excs.begin(), Excs.end()); - } - } - return Results; -} - -static const TypeVec throwsException(const FunctionDecl *Func) { - llvm::SmallSet CallStack; - return throwsException(Func, CallStack); -} - -namespace ast_matchers { -AST_MATCHER_P(FunctionDecl, throws, internal::Matcher, InnerMatcher) { - TypeVec ExceptionList = throwsException(&Node); - auto NewEnd = llvm::remove_if( - ExceptionList, [this, Finder, Builder](const Type *Exception) { - return !InnerMatcher.matches(*Exception, Finder, Builder); - }); - ExceptionList.erase(NewEnd, ExceptionList.end()); - return ExceptionList.size(); -} - -AST_MATCHER_P(Type, isIgnored, llvm::StringSet<>, IgnoredExceptions) { - if (const auto *TD = Node.getAsTagDecl()) { - if (TD->getDeclName().isIdentifier()) - return IgnoredExceptions.count(TD->getName()) > 0; - } - return false; -} - +namespace { AST_MATCHER_P(FunctionDecl, isEnabled, llvm::StringSet<>, FunctionsThatShouldNotThrow) { return FunctionsThatShouldNotThrow.count(Node.getNameAsString()) > 0; } -} // namespace ast_matchers +} // namespace namespace tidy { namespace bugprone { - ExceptionEscapeCheck::ExceptionEscapeCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), RawFunctionsThatShouldNotThrow(Options.get( @@ -173,9 +36,12 @@ .split(FunctionsThatShouldNotThrowVec, ",", -1, false); FunctionsThatShouldNotThrow.insert(FunctionsThatShouldNotThrowVec.begin(), FunctionsThatShouldNotThrowVec.end()); + + llvm::StringSet<> IgnoredExceptions; StringRef(RawIgnoredExceptions).split(IgnoredExceptionsVec, ",", -1, false); IgnoredExceptions.insert(IgnoredExceptionsVec.begin(), IgnoredExceptionsVec.end()); + Tracer.ignoreExceptions(std::move(IgnoredExceptions)); } void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { @@ -193,22 +59,25 @@ cxxConstructorDecl(isMoveConstructor()), cxxMethodDecl(isMoveAssignmentOperator()), hasName("main"), hasName("swap"), - isEnabled(FunctionsThatShouldNotThrow)), - throws(unless(isIgnored(IgnoredExceptions)))) + isEnabled(FunctionsThatShouldNotThrow))) .bind("thrower"), this); } void ExceptionEscapeCheck::check(const MatchFinder::MatchResult &Result) { - const FunctionDecl *MatchedDecl = - Result.Nodes.getNodeAs("thrower"); + const auto *MatchedDecl = Result.Nodes.getNodeAs("thrower"); + if (!MatchedDecl) return; - // FIXME: We should provide more information about the exact location where - // the exception is thrown, maybe the full path the exception escapes - diag(MatchedDecl->getLocation(), "an exception may be thrown in function %0 " - "which should not throw exceptions") << MatchedDecl; + if (Tracer.throwsException(MatchedDecl)) + // FIXME: We should provide more information about the exact location where + // the exception is thrown, maybe the full path the exception escapes + diag(MatchedDecl->getLocation(), + "an exception may be thrown in function %0 " + + "which should not throw exceptions") + << MatchedDecl; } } // namespace bugprone Index: clang-tidy/utils/CMakeLists.txt =================================================================== --- clang-tidy/utils/CMakeLists.txt +++ clang-tidy/utils/CMakeLists.txt @@ -3,6 +3,7 @@ add_clang_library(clangTidyUtils ASTUtils.cpp DeclRefExprUtils.cpp + ExceptionAnalyzer.cpp ExprSequence.cpp FixItHintUtils.cpp HeaderFileExtensionsUtils.cpp Index: clang-tidy/utils/ExceptionAnalyzer.h =================================================================== --- /dev/null +++ clang-tidy/utils/ExceptionAnalyzer.h @@ -0,0 +1,50 @@ +//===--- ExceptionAnalyzer.h - clang-tidy -----------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_ANALYZER_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_ANALYZER_H + +#include "clang/AST/ASTContext.h" +#include "llvm/ADT/SmallSet.h" +#include "llvm/ADT/StringSet.h" + +namespace clang { +namespace tidy { +namespace utils { + +/// This class analysis if a `FunctionDecl` can in principle throw an exception, +/// either directly or indirectly. +/// It can be configured to ignore some exception types, especially +/// `std::bad_alloc` can be disabled as using dynamic memory will always +/// give the possibility of an exception. +class ExceptionAnalyzer { +public: + ExceptionAnalyzer() = default; + + bool throwsException(const FunctionDecl *Func); + void ignoreExceptions(llvm::StringSet<> ExceptionNames) { + IgnoredExceptions = std::move(ExceptionNames); + } + +private: + using TypeVec = llvm::SmallVector; + + TypeVec throwsException(const FunctionDecl *Func, + llvm::SmallSet &CallStack); + TypeVec throwsException(const Stmt *St, const TypeVec &Caught, + llvm::SmallSet &CallStack); + bool isIgnoredExceptionType(const Type *Exception); + + llvm::StringSet<> IgnoredExceptions; + llvm::DenseMap FunctionCache; +}; +} // namespace utils +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_ANALYZER_H Index: clang-tidy/utils/ExceptionAnalyzer.cpp =================================================================== --- clang-tidy/utils/ExceptionAnalyzer.cpp +++ clang-tidy/utils/ExceptionAnalyzer.cpp @@ -1,4 +1,4 @@ -//===--- ExceptionEscapeCheck.cpp - clang-tidy-----------------------------===// +//===--- ExceptionAnalyzer.cpp - clang-tidy -------------------------------===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,22 +6,12 @@ // //===----------------------------------------------------------------------===// -#include "ExceptionEscapeCheck.h" +#include "ExceptionAnalyzer.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" -#include "llvm/ADT/SmallSet.h" -#include "llvm/ADT/StringSet.h" - -using namespace clang::ast_matchers; - -namespace { -typedef llvm::SmallVector TypeVec; -} // namespace - namespace clang { - static bool isBaseOf(const Type *DerivedType, const Type *BaseType) { const auto *DerivedClass = DerivedType->getAsCXXRecordDecl(); const auto *BaseClass = BaseType->getAsCXXRecordDecl(); @@ -32,13 +22,12 @@ [BaseClass](const CXXRecordDecl *Cur) { return Cur != BaseClass; }); } -static const TypeVec -throwsException(const Stmt *St, const TypeVec &Caught, - llvm::SmallSet &CallStack); +namespace tidy { +namespace utils { -static const TypeVec -throwsException(const FunctionDecl *Func, - llvm::SmallSet &CallStack) { +ExceptionAnalyzer::TypeVec ExceptionAnalyzer::throwsException( + const FunctionDecl *Func, + llvm::SmallSet &CallStack) { if (CallStack.count(Func)) return TypeVec(); @@ -58,9 +47,9 @@ return Result; } -static const TypeVec -throwsException(const Stmt *St, const TypeVec &Caught, - llvm::SmallSet &CallStack) { +ExceptionAnalyzer::TypeVec ExceptionAnalyzer::throwsException( + const Stmt *St, const TypeVec &Caught, + llvm::SmallSet &CallStack) { TypeVec Results; if (!St) @@ -76,8 +65,8 @@ ->getUnqualifiedDesugaredType(); } if (const auto *TD = ThrownType->getAsTagDecl()) { - if (TD->getDeclName().isIdentifier() && TD->getName() == "bad_alloc" - && TD->isInStdNamespace()) + if (TD->getDeclName().isIdentifier() && TD->getName() == "bad_alloc" && + TD->isInStdNamespace()) return Results; } Results.push_back(ThrownExpr->getType()->getUnqualifiedDesugaredType()); @@ -129,88 +118,37 @@ return Results; } -static const TypeVec throwsException(const FunctionDecl *Func) { +bool ExceptionAnalyzer::throwsException(const FunctionDecl *Func) { + // Check if the function has already been analyzed and reuse that result. + if (FunctionCache.count(Func) > 0) + return FunctionCache[Func]; + llvm::SmallSet CallStack; - return throwsException(Func, CallStack); -} + TypeVec ExceptionList = throwsException(Func, CallStack); -namespace ast_matchers { -AST_MATCHER_P(FunctionDecl, throws, internal::Matcher, InnerMatcher) { - TypeVec ExceptionList = throwsException(&Node); - auto NewEnd = llvm::remove_if( - ExceptionList, [this, Finder, Builder](const Type *Exception) { - return !InnerMatcher.matches(*Exception, Finder, Builder); - }); + // Remove all ignored exceptions from the list of exceptions that can be + // thrown. + auto NewEnd = llvm::remove_if(ExceptionList, [this](const Type *Exception) { + return isIgnoredExceptionType(Exception); + }); ExceptionList.erase(NewEnd, ExceptionList.end()); - return ExceptionList.size(); + + // Cache the result of the analysis. + bool FunctionThrows = ExceptionList.size() > 0; + FunctionCache.insert(std::make_pair(Func, FunctionThrows)); + + return FunctionThrows; } -AST_MATCHER_P(Type, isIgnored, llvm::StringSet<>, IgnoredExceptions) { - if (const auto *TD = Node.getAsTagDecl()) { +bool ExceptionAnalyzer::isIgnoredExceptionType(const Type *Exception) { + if (const auto *TD = Exception->getAsTagDecl()) { if (TD->getDeclName().isIdentifier()) return IgnoredExceptions.count(TD->getName()) > 0; } return false; } -AST_MATCHER_P(FunctionDecl, isEnabled, llvm::StringSet<>, - FunctionsThatShouldNotThrow) { - return FunctionsThatShouldNotThrow.count(Node.getNameAsString()) > 0; -} -} // namespace ast_matchers - -namespace tidy { -namespace bugprone { - -ExceptionEscapeCheck::ExceptionEscapeCheck(StringRef Name, - ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), RawFunctionsThatShouldNotThrow(Options.get( - "FunctionsThatShouldNotThrow", "")), - RawIgnoredExceptions(Options.get("IgnoredExceptions", "")) { - llvm::SmallVector FunctionsThatShouldNotThrowVec, - IgnoredExceptionsVec; - StringRef(RawFunctionsThatShouldNotThrow) - .split(FunctionsThatShouldNotThrowVec, ",", -1, false); - FunctionsThatShouldNotThrow.insert(FunctionsThatShouldNotThrowVec.begin(), - FunctionsThatShouldNotThrowVec.end()); - StringRef(RawIgnoredExceptions).split(IgnoredExceptionsVec, ",", -1, false); - IgnoredExceptions.insert(IgnoredExceptionsVec.begin(), - IgnoredExceptionsVec.end()); -} - -void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "FunctionsThatShouldNotThrow", - RawFunctionsThatShouldNotThrow); - Options.store(Opts, "IgnoredExceptions", RawIgnoredExceptions); -} - -void ExceptionEscapeCheck::registerMatchers(MatchFinder *Finder) { - if (!getLangOpts().CPlusPlus || !getLangOpts().CXXExceptions) - return; - - Finder->addMatcher( - functionDecl(anyOf(isNoThrow(), cxxDestructorDecl(), - cxxConstructorDecl(isMoveConstructor()), - cxxMethodDecl(isMoveAssignmentOperator()), - hasName("main"), hasName("swap"), - isEnabled(FunctionsThatShouldNotThrow)), - throws(unless(isIgnored(IgnoredExceptions)))) - .bind("thrower"), - this); -} - -void ExceptionEscapeCheck::check(const MatchFinder::MatchResult &Result) { - const FunctionDecl *MatchedDecl = - Result.Nodes.getNodeAs("thrower"); - if (!MatchedDecl) - return; - - // FIXME: We should provide more information about the exact location where - // the exception is thrown, maybe the full path the exception escapes - diag(MatchedDecl->getLocation(), "an exception may be thrown in function %0 " - "which should not throw exceptions") << MatchedDecl; -} - -} // namespace bugprone +} // namespace utils } // namespace tidy + } // namespace clang