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::ExceptionTracer 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. @@ -8,174 +8,41 @@ #include "ExceptionEscapeCheck.h" +#include "../utils/OptionsUtils.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(); - 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( "FunctionsThatShouldNotThrow", "")), RawIgnoredExceptions(Options.get("IgnoredExceptions", "")) { - llvm::SmallVector FunctionsThatShouldNotThrowVec, - IgnoredExceptionsVec; - StringRef(RawFunctionsThatShouldNotThrow) - .split(FunctionsThatShouldNotThrowVec, ",", -1, false); + std::vector FunctionsThatShouldNotThrowVec = + utils::options::parseStringList(RawFunctionsThatShouldNotThrow); FunctionsThatShouldNotThrow.insert(FunctionsThatShouldNotThrowVec.begin(), FunctionsThatShouldNotThrowVec.end()); - StringRef(RawIgnoredExceptions).split(IgnoredExceptionsVec, ",", -1, false); + + std::vector IgnoredExceptionsVec = + utils::options::parseStringList(RawIgnoredExceptions); + + llvm::StringSet<> IgnoredExceptions; IgnoredExceptions.insert(IgnoredExceptionsVec.begin(), IgnoredExceptionsVec.end()); + Tracer.ignoreExceptions(std::move(IgnoredExceptions)); } void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { @@ -193,22 +60,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 ExceptionTracer { +public: + ExceptionTracer() = 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 =================================================================== --- /dev/null +++ clang-tidy/utils/ExceptionAnalyzer.cpp @@ -0,0 +1,146 @@ +#include "ExceptionAnalyzer.h" + +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +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; }); +} + +namespace tidy { +namespace utils { + +ExceptionTracer::TypeVec ExceptionTracer::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; +} + +ExceptionTracer::TypeVec ExceptionTracer::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; +} + +bool ExceptionTracer::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; + TypeVec ExceptionList = throwsException(Func, CallStack); + + // 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()); + + // Cache the result of the analysis. + bool FunctionThrows = ExceptionList.size() > 0; + FunctionCache.insert(std::make_pair(Func, FunctionThrows)); + + return FunctionThrows; +} + +bool ExceptionTracer::isIgnoredExceptionType(const Type *Exception) { + if (const auto *TD = Exception->getAsTagDecl()) { + if (TD->getDeclName().isIdentifier()) + return IgnoredExceptions.count(TD->getName()) > 0; + } + return false; +} + +} // namespace utils +} // namespace tidy + +} // namespace clang Index: test/clang-tidy/bugprone-exception-escape.cpp =================================================================== --- test/clang-tidy/bugprone-exception-escape.cpp +++ test/clang-tidy/bugprone-exception-escape.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -extra-arg=-fexceptions -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.FunctionsThatShouldNotThrow, value: 'enabled1,enabled2,enabled3'}]}" -- +// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -extra-arg=-fexceptions -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1;ignored2'}, {key: bugprone-exception-escape.FunctionsThatShouldNotThrow, value: 'enabled1;enabled2;enabled3'}]}" -- struct throwing_destructor { ~throwing_destructor() {