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,7 @@ if (!MatchedDecl) return; - if (Tracer.throwsException(MatchedDecl)) + if (Tracer.analyze(MatchedDecl).getState() == utils::ExceptionState::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,36 +10,212 @@ #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" 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 { -/// 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. +enum class ExceptionState : std::int8_t { + Throwing, ///< The function can definitly throw given an AST. + NotThrowing, ///< This function can not throw, given an AST. + Unknown, ///< 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(ExceptionState::Unknown); + } + static ExceptionInfo createNonThrowing() { + return ExceptionInfo(ExceptionState::Throwing); + } + + /// By default the exception situation is unknown and must be + /// clarified step-wise. + ExceptionInfo() + : State(ExceptionState::NotThrowing), ContainsUnknown(false) {} + ExceptionInfo(ExceptionState S) + : State(S), ContainsUnknown(S == ExceptionState::Unknown) {} + + ExceptionInfo(const ExceptionInfo&) = default; + ExceptionInfo& operator=(const ExceptionInfo&) = default; + ExceptionInfo(ExceptionInfo&&) = default; + ExceptionInfo& operator=(ExceptionInfo&&) = default; + + ExceptionState getState() const { return State; } + + void registerException(const Type *ExceptionType) { + assert(ExceptionType != nullptr && "Only valid types are accepted"); + State = ExceptionState::Throwing; + ThrownExceptions.insert(ExceptionType); + } + void registerExceptions(const Throwables &Exceptions) { + if (Exceptions.size() == 0) + return; + ThrownExceptions.insert(Exceptions.begin(), Exceptions.end()); + } + + /// 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) { + // Only the following two cases require an update to the local 'State'. + // 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.State == ExceptionState::Throwing) { + State = ExceptionState::Throwing; + } else if (Other.State == ExceptionState::Unknown && + State == ExceptionState::NotThrowing) { + State = ExceptionState::Unknown; + } + + ContainsUnknown = ContainsUnknown || Other.ContainsUnknown; + + ThrownExceptions.insert(Other.ThrownExceptions.begin(), + Other.ThrownExceptions.end()); + return *this; + } + + /// 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) { + 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); + + reevaluateState(); + return TypesToDelete.size() > 0; + } + + /// 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) { + 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); + + reevaluateState(); + return *this; + } + + /// Clear the state to 'NonThrowing' to make the corresponding entity + /// neutral. + void clear() { + State = ExceptionState::NotThrowing; + ContainsUnknown = false; + ThrownExceptions.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 'State' for example after filtering. + void reevaluateState() { + if (ThrownExceptions.size() == 0) + if (ContainsUnknown) + State = ExceptionState::Unknown; + else + State = ExceptionState::NotThrowing; + else + State = ExceptionState::Throwing; + } + + /// Keep track if the entity related to this 'ExceptionInfo' can in princple + /// throw, if it's unknown or if it won't throw. + enum ExceptionState State; + + /// Keep track if the entity contains any unknown elements to keep track + /// of the certainty of decisions and/or correct 'State' transition after + /// filtering. + bool ContainsUnknown; + + /// 'ThrownException' is empty if the 'State' is either 'NotThrowing' or + /// 'Unknown'. + Throwables ThrownExceptions; +}; + +/// 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: 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,50 +8,38 @@ #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 { -ExceptionAnalyzer::TypeVec ExceptionAnalyzer::throwsException( +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. +ExceptionInfo ExceptionAnalyzer::throwsException( + const Stmt *St, const ExceptionInfo::Throwables &Caught, llvm::SmallSet &CallStack) { - TypeVec Results; - + auto Results = ExceptionInfo::createNonThrowing(); if (!St) return Results; @@ -64,23 +52,25 @@ ->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()); + Results.registerException( + ThrownExpr->getType()->getUnqualifiedDesugaredType()); } else { - Results.append(Caught.begin(), Caught.end()); + // 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 +80,61 @@ ->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]; +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.getState() == ExceptionState::NotThrowing || + ExceptionList.getState() == ExceptionState::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()); + ExceptionList.filterIgnoredExceptions(IgnoredExceptions, IgnoreBadAlloc); - // Cache the result of the analysis. - bool FunctionThrows = ExceptionList.size() > 0; - FunctionCache.insert(std::make_pair(Func, FunctionThrows)); - - return FunctionThrows; + return ExceptionList; } - -bool ExceptionAnalyzer::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