JonasToth created this revision.
JonasToth added reviewers: lebedev.ri, aaron.ballman, baloghadamsoftware,
alexfh.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus,
a.sidorin, rnkovacs, xazax.hun.
Herald added a project: clang.
The analsis on the throwing behvaiour on functions and statements gave only
a binary answer whether an exception could occur and if yes which types are
thrown.
This refactoring allows keeping track if there is a unknown factor, because the
code calls to some functions with unavailable source code with no `noexcept`
information.
This 'potential Unknown' information is propagated properly and can be queried
separately.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D57883
Files:
clang-tidy/bugprone/ExceptionEscapeCheck.cpp
clang-tidy/utils/ExceptionAnalyzer.cpp
clang-tidy/utils/ExceptionAnalyzer.h
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<const Type *, 8>;
+ 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<const Type *, 8> 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<const Type *, 8> 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<const Type *, 8>;
+ ExceptionInfo analyze(const FunctionDecl *Func);
- TypeVec throwsException(const FunctionDecl *Func,
- llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
- TypeVec throwsException(const Stmt *St, const TypeVec &Caught,
- llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
- bool isIgnoredExceptionType(const Type *Exception);
+private:
+ ExceptionInfo
+ throwsException(const FunctionDecl *Func,
+ llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
+ ExceptionInfo
+ throwsException(const Stmt *St, const ExceptionInfo::Throwables &Caught,
+ llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
+ bool IgnoreBadAlloc = true;
llvm::StringSet<> IgnoredExceptions;
- llvm::DenseMap<const FunctionDecl *, bool> FunctionCache;
+ std::map<const FunctionDecl *, ExceptionInfo> 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<const FunctionDecl *, 32> &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<FunctionProtoType>()) {
- 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<const FunctionDecl *, 32> &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<CXXTryStmt>(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<CallExpr>(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<const FunctionDecl *, 32> 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<const FunctionDecl *, 32> 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
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(),
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
