Index: clang-tidy/bugprone/ExceptionEscapeCheck.cpp =================================================================== --- clang-tidy/bugprone/ExceptionEscapeCheck.cpp +++ clang-tidy/bugprone/ExceptionEscapeCheck.cpp @@ -42,6 +42,7 @@ IgnoredExceptions.insert(IgnoredExceptionsVec.begin(), IgnoredExceptionsVec.end()); Tracer.ignoreExceptions(std::move(IgnoredExceptions)); + Tracer.ignoreBadAlloc(true); } void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { @@ -70,7 +71,8 @@ if (!MatchedDecl) return; - if (Tracer.throwsException(MatchedDecl)) + if (Tracer.analyze(MatchedDecl).getBehaviour() == + utils::ExceptionAnalyzer::State::Throwing) // 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(), Index: clang-tidy/utils/ExceptionAnalyzer.h =================================================================== --- clang-tidy/utils/ExceptionAnalyzer.h +++ clang-tidy/utils/ExceptionAnalyzer.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_ANALYZER_H #include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" #include "llvm/ADT/SmallSet.h" #include "llvm/ADT/StringSet.h" @@ -17,29 +18,131 @@ 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 custom exception types. +/// This class analysis if a `FunctionDecl` can in principle throw an +/// exception, either directly or indirectly. It can be configured to ignore +/// custom exception types. class ExceptionAnalyzer { public: + enum class State : std::int8_t { + Throwing = 0, ///< The function can definitly throw given an AST. + NotThrowing = 1, ///< This function can not throw, given an AST. + Unknown = 2, ///< This can happen for extern functions without available + ///< definition. + }; + + /// Bundle the gathered information about an entity like a function regarding + /// it's exception behaviour. The 'NonThrowing'-state can be considered as the + /// neutral element in terms of information propagation. + /// In the case of 'Throwing' state it is possible that 'getExceptionTypes' + /// does not include *ALL* possible types as there is the possibility that + /// an 'Unknown' function is called that might throw a previously unknown + /// exception at runtime. + class ExceptionInfo { + public: + using Throwables = llvm::SmallSet; + static ExceptionInfo createUnknown() { + return ExceptionInfo(State::Unknown); + } + static ExceptionInfo createNonThrowing() { + return ExceptionInfo(State::Throwing); + } + + /// By default the exception situation is unknown and must be + /// clarified step-wise. + ExceptionInfo() : Behaviour(State::NotThrowing), ContainsUnknown(false) {} + ExceptionInfo(State S) + : Behaviour(S), ContainsUnknown(S == State::Unknown) {} + + ExceptionInfo(const ExceptionInfo &) = default; + ExceptionInfo &operator=(const ExceptionInfo &) = default; + ExceptionInfo(ExceptionInfo &&) = default; + ExceptionInfo &operator=(ExceptionInfo &&) = default; + + State getBehaviour() const { return Behaviour; } + + /// Register a single exception type as recognized potential exception to be + /// thrown. + void registerException(const Type *ExceptionType); + + /// Registers a `SmallVector` of exception types as recognized potential + /// exceptions to be thrown. + void registerExceptions(const Throwables &Exceptions); + + /// Updates the local state according to the other state. That means if + /// for example a function contains multiple statements the 'ExceptionInfo' + /// for the final function is the merged result of each statement. + /// If one of these statements throws the whole function throws and if one + /// part is unknown and the rest is non-throwing the result will be + /// unknown. + ExceptionInfo &merge(const ExceptionInfo &Other); + + /// This method is useful in case 'catch' clauses are analyzed as it is + /// possible to catch multiple exception types by one 'catch' if they + /// are a subclass of the 'catch'ed exception type. + /// Returns 'true' if some exceptions were filtered, otherwise 'false'. + bool filterByCatch(const Type *BaseClass); + + /// Filter the set of thrown exception type against a set of ignored + /// types that shall not be considered in the exception analysis. + /// This includes explicit `std::bad_alloc` ignoring as separate option. + ExceptionInfo & + filterIgnoredExceptions(const llvm::StringSet<> &IgnoredTypes, + bool IgnoreBadAlloc); + + /// Clear the state to 'NonThrowing' to make the corresponding entity + /// neutral. + void clear(); + + /// References the set of known exception types that can escape from the + /// corresponding entity. + const Throwables &getExceptionTypes() const { return ThrownExceptions; } + + /// Signal if the there is any 'Unknown' element within the scope of + /// the related entity. This might be relevant if the entity is 'Throwing' + /// and to ensure that no other exception then 'getExceptionTypes' can + /// occur. If there is an 'Unknown' element this can not be guaranteed. + bool containsUnknownElements() const { return ContainsUnknown; } + + private: + /// Recalculate the 'Behaviour' for example after filtering. + void reevaluateBehaviour(); + + /// Keep track if the entity related to this 'ExceptionInfo' can in princple + /// throw, if it's unknown or if it won't throw. + enum State Behaviour : 2; + + /// Keep track if the entity contains any unknown elements to keep track + /// of the certainty of decisions and/or correct 'Behaviour' transition + /// after filtering. + bool ContainsUnknown : 1; + + /// 'ThrownException' is empty if the 'Behaviour' is either 'NotThrowing' or + /// 'Unknown'. + Throwables ThrownExceptions; + }; + static_assert(sizeof(ExceptionInfo) <= 64u, + "size of exceptioninfo shall be at max one cache line"); + ExceptionAnalyzer() = default; - bool throwsException(const FunctionDecl *Func); + void ignoreBadAlloc(bool ShallIgnore) { IgnoreBadAlloc = ShallIgnore; } void ignoreExceptions(llvm::StringSet<> ExceptionNames) { IgnoredExceptions = std::move(ExceptionNames); } -private: - using TypeVec = llvm::SmallVector; + ExceptionInfo analyze(const FunctionDecl *Func); - TypeVec throwsException(const FunctionDecl *Func, - llvm::SmallSet &CallStack); - TypeVec throwsException(const Stmt *St, const TypeVec &Caught, - llvm::SmallSet &CallStack); - bool isIgnoredExceptionType(const Type *Exception); +private: + ExceptionInfo + throwsException(const FunctionDecl *Func, + llvm::SmallSet &CallStack); + ExceptionInfo + throwsException(const Stmt *St, const ExceptionInfo::Throwables &Caught, + llvm::SmallSet &CallStack); + bool IgnoreBadAlloc = true; llvm::StringSet<> IgnoredExceptions; - llvm::DenseMap FunctionCache; + std::map FunctionCache; }; } // namespace utils } // namespace tidy Index: clang-tidy/utils/ExceptionAnalyzer.cpp =================================================================== --- clang-tidy/utils/ExceptionAnalyzer.cpp +++ clang-tidy/utils/ExceptionAnalyzer.cpp @@ -8,10 +8,44 @@ #include "ExceptionAnalyzer.h" -#include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" - namespace clang { +namespace tidy { +namespace utils { + +void ExceptionAnalyzer::ExceptionInfo::registerException( + const Type *ExceptionType) { + assert(ExceptionType != nullptr && "Only valid types are accepted"); + Behaviour = State::Throwing; + ThrownExceptions.insert(ExceptionType); +} + +void ExceptionAnalyzer::ExceptionInfo::registerExceptions( + const Throwables &Exceptions) { + if (Exceptions.size() == 0) + return; + Behaviour = State::Throwing; + ThrownExceptions.insert(Exceptions.begin(), Exceptions.end()); +} + +ExceptionAnalyzer::ExceptionInfo &ExceptionAnalyzer::ExceptionInfo::merge( + const ExceptionAnalyzer::ExceptionInfo &Other) { + // Only the following two cases require an update to the local + // 'Behaviour'. If the local entity is already throwing there will be no + // change and if the other entity is throwing the merged entity will throw + // as well. + // If one of both entities is 'Unknown' and the other one does not throw + // the merged entity is 'Unknown' as well. + if (Other.Behaviour == State::Throwing) + Behaviour = State::Throwing; + else if (Other.Behaviour == State::Unknown && Behaviour == State::NotThrowing) + Behaviour = State::Unknown; + + ContainsUnknown = ContainsUnknown || Other.ContainsUnknown; + ThrownExceptions.insert(Other.ThrownExceptions.begin(), + Other.ThrownExceptions.end()); + return *this; +} + static bool isBaseOf(const Type *DerivedType, const Type *BaseType) { const auto *DerivedClass = DerivedType->getAsCXXRecordDecl(); const auto *BaseClass = BaseType->getAsCXXRecordDecl(); @@ -22,36 +56,87 @@ [BaseClass](const CXXRecordDecl *Cur) { return Cur != BaseClass; }); } -namespace tidy { -namespace utils { +bool ExceptionAnalyzer::ExceptionInfo::filterByCatch(const Type *BaseClass) { + llvm::SmallVector TypesToDelete; + for (const Type *T : ThrownExceptions) { + if (T == BaseClass || isBaseOf(T, BaseClass)) + TypesToDelete.push_back(T); + } + + for (const Type *T : TypesToDelete) + ThrownExceptions.erase(T); + + reevaluateBehaviour(); + return TypesToDelete.size() > 0; +} + +ExceptionAnalyzer::ExceptionInfo & +ExceptionAnalyzer::ExceptionInfo::filterIgnoredExceptions( + const llvm::StringSet<> &IgnoredTypes, bool IgnoreBadAlloc) { + llvm::SmallVector TypesToDelete; + // Note: Using a 'SmallSet' with 'llvm::remove_if()' is not possible. + // Therefore this slightly hacky implementation is required. + for (const Type *T : ThrownExceptions) { + if (const auto *TD = T->getAsTagDecl()) { + if (TD->getDeclName().isIdentifier()) { + if ((IgnoreBadAlloc && + (TD->getName() == "bad_alloc" && TD->isInStdNamespace())) || + (IgnoredTypes.count(TD->getName()) > 0)) + TypesToDelete.push_back(T); + } + } + } + for (const Type *T : TypesToDelete) + ThrownExceptions.erase(T); + + reevaluateBehaviour(); + return *this; +} + +void ExceptionAnalyzer::ExceptionInfo::clear() { + Behaviour = State::NotThrowing; + ContainsUnknown = false; + ThrownExceptions.clear(); +} -ExceptionAnalyzer::TypeVec ExceptionAnalyzer::throwsException( +void ExceptionAnalyzer::ExceptionInfo::reevaluateBehaviour() { + if (ThrownExceptions.size() == 0) + if (ContainsUnknown) + Behaviour = State::Unknown; + else + Behaviour = State::NotThrowing; + else + Behaviour = State::Throwing; +} + +ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException( const FunctionDecl *Func, llvm::SmallSet &CallStack) { if (CallStack.count(Func)) - return TypeVec(); + return ExceptionInfo::createNonThrowing(); if (const Stmt *Body = Func->getBody()) { CallStack.insert(Func); - const TypeVec Result = throwsException(Body, TypeVec(), CallStack); + ExceptionInfo Result = + throwsException(Body, ExceptionInfo::Throwables(), CallStack); CallStack.erase(Func); return Result; } - TypeVec Result; + auto Result = ExceptionInfo::createUnknown(); if (const auto *FPT = Func->getType()->getAs()) { - for (const QualType Ex : FPT->exceptions()) { - Result.push_back(Ex.getTypePtr()); - } + for (const QualType Ex : FPT->exceptions()) + Result.registerException(Ex.getTypePtr()); } return Result; } -ExceptionAnalyzer::TypeVec ExceptionAnalyzer::throwsException( - const Stmt *St, const TypeVec &Caught, +/// Analyzes a single statment on it's throwing behaviour. This is in principle +/// possible except some 'Unknown' functions are called. +ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException( + const Stmt *St, const ExceptionInfo::Throwables &Caught, llvm::SmallSet &CallStack) { - TypeVec Results; - + auto Results = ExceptionInfo::createNonThrowing(); if (!St) return Results; @@ -59,28 +144,28 @@ if (const auto *ThrownExpr = Throw->getSubExpr()) { const auto *ThrownType = ThrownExpr->getType()->getUnqualifiedDesugaredType(); - if (ThrownType->isReferenceType()) { + 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()); - } + Results.registerException( + ThrownExpr->getType()->getUnqualifiedDesugaredType()); + } else + // A rethrow of a caught exception happens which makes it possible + // to throw all exception that are caught in the 'catch' clause of + // the parent try-catch block. + Results.registerExceptions(Caught); } else if (const auto *Try = dyn_cast(St)) { - TypeVec Uncaught = throwsException(Try->getTryBlock(), Caught, CallStack); + ExceptionInfo Uncaught = + throwsException(Try->getTryBlock(), Caught, CallStack); for (unsigned i = 0; i < Try->getNumHandlers(); ++i) { const CXXCatchStmt *Catch = Try->getHandler(i); + + // Everything is catched through 'catch(...)'. if (!Catch->getExceptionDecl()) { - const TypeVec Rethrown = - throwsException(Catch->getHandlerBlock(), Uncaught, CallStack); - Results.append(Rethrown.begin(), Rethrown.end()); + ExceptionInfo Rethrown = throwsException( + Catch->getHandlerBlock(), Uncaught.getExceptionTypes(), CallStack); + Results.merge(Rethrown); Uncaught.clear(); } else { const auto *CaughtType = @@ -90,64 +175,62 @@ ->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()); + + // If the caught exception will catch multiple previously potential + // thrown types (because it's sensitive to inheritance) the throwing + // situation changes. First of all filter the exception types and + // analyze if the baseclass-exception is rethrown. + if (Uncaught.filterByCatch(CaughtType)) { + ExceptionInfo::Throwables CaughtExceptions; + CaughtExceptions.insert(CaughtType); + ExceptionInfo Rethrown = throwsException(Catch->getHandlerBlock(), + CaughtExceptions, CallStack); + Results.merge(Rethrown); } } } - Results.append(Uncaught.begin(), Uncaught.end()); + Results.merge(Uncaught); } 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()); + ExceptionInfo Excs = throwsException(Func, CallStack); + Results.merge(Excs); } } else { for (const Stmt *Child : St->children()) { - TypeVec Excs = throwsException(Child, Caught, CallStack); - Results.append(Excs.begin(), Excs.end()); + ExceptionInfo Excs = throwsException(Child, Caught, CallStack); + Results.merge(Excs); } } return Results; } -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]; +ExceptionAnalyzer::ExceptionInfo +ExceptionAnalyzer::analyze(const FunctionDecl *Func) { + ExceptionInfo ExceptionList; - llvm::SmallSet CallStack; - TypeVec ExceptionList = throwsException(Func, CallStack); + // Check if the function has already been analyzed and reuse that result. + if (FunctionCache.count(Func) == 0) { + llvm::SmallSet CallStack; + ExceptionList = throwsException(Func, CallStack); + + // Cache the result of the analysis. This is done prior to filtering + // because it is best to keep as much information as possible. + // The results here might be relevant to different analysis passes + // with different needs as well. + FunctionCache.insert(std::make_pair(Func, ExceptionList)); + } else + ExceptionList = FunctionCache[Func]; + + if (ExceptionList.getBehaviour() == State::NotThrowing || + ExceptionList.getBehaviour() == State::Unknown) + return ExceptionList; // 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 ExceptionAnalyzer::isIgnoredExceptionType(const Type *Exception) { - if (const auto *TD = Exception->getAsTagDecl()) { - if (TD->getDeclName().isIdentifier()) - return IgnoredExceptions.count(TD->getName()) > 0; - } - return false; -} + ExceptionList.filterIgnoredExceptions(IgnoredExceptions, IgnoreBadAlloc); + return ExceptionList; +} } // namespace utils } // namespace tidy