JonasToth created this revision.
JonasToth added reviewers: aaron.ballman, alexfh, hokein, baloghadamsoftware.
Herald added subscribers: cfe-commits, rnkovacs, xazax.hun, mgorny.
The check `bugprone-exception-escape` does an AST-based analysis to determine
if a function might throw an exception and warns based on that information.
The analysis part is refactored into a standalone class similiar to
`ExprMutAnalyzer` that is generally useful.
I intent to use that class in a new check to automatically introduce `noexcept`
if possible.
`bugprone-exception-escape` uses the consistent semicolon-separated list for
configuration now as well.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D57100
Files:
clang-tidy/bugprone/ExceptionEscapeCheck.cpp
clang-tidy/bugprone/ExceptionEscapeCheck.h
clang-tidy/utils/CMakeLists.txt
clang-tidy/utils/ExceptionAnalyzer.cpp
clang-tidy/utils/ExceptionAnalyzer.h
test/clang-tidy/bugprone-exception-escape.cpp
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() {
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<const Type *, 8>;
+
+ 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);
+
+ llvm::StringSet<> IgnoredExceptions;
+ llvm::DenseMap<const FunctionDecl*, bool> 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<const FunctionDecl *, 32> &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<FunctionProtoType>()) {
+ 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<const FunctionDecl *, 32> &CallStack) {
+ TypeVec Results;
+
+ if (!St)
+ return Results;
+
+ if (const auto *Throw = dyn_cast<CXXThrowExpr>(St)) {
+ if (const auto *ThrownExpr = Throw->getSubExpr()) {
+ const auto *ThrownType =
+ ThrownExpr->getType()->getUnqualifiedDesugaredType();
+ if (ThrownType->isReferenceType()) {
+ ThrownType = ThrownType->castAs<ReferenceType>()
+ ->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<CXXTryStmt>(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<ReferenceType>()
+ ->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<CallExpr>(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<const FunctionDecl *, 32> 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: 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/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<const clang::Type *, 8> 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<const FunctionDecl *, 32> &CallStack);
-
-static const TypeVec
-throwsException(const FunctionDecl *Func,
- llvm::SmallSet<const FunctionDecl *, 32> &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<FunctionProtoType>()) {
- 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<const FunctionDecl *, 32> &CallStack) {
- TypeVec Results;
-
- if (!St)
- return Results;
-
- if (const auto *Throw = dyn_cast<CXXThrowExpr>(St)) {
- if (const auto *ThrownExpr = Throw->getSubExpr()) {
- const auto *ThrownType =
- ThrownExpr->getType()->getUnqualifiedDesugaredType();
- if (ThrownType->isReferenceType()) {
- ThrownType = ThrownType->castAs<ReferenceType>()
- ->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<CXXTryStmt>(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<ReferenceType>()
- ->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<CallExpr>(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<const FunctionDecl *, 32> CallStack;
- return throwsException(Func, CallStack);
-}
-
-namespace ast_matchers {
-AST_MATCHER_P(FunctionDecl, throws, internal::Matcher<Type>, 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<StringRef, 8> FunctionsThatShouldNotThrowVec,
- IgnoredExceptionsVec;
- StringRef(RawFunctionsThatShouldNotThrow)
- .split(FunctionsThatShouldNotThrowVec, ",", -1, false);
+ std::vector<std::string> FunctionsThatShouldNotThrowVec =
+ utils::options::parseStringList(RawFunctionsThatShouldNotThrow);
FunctionsThatShouldNotThrow.insert(FunctionsThatShouldNotThrowVec.begin(),
FunctionsThatShouldNotThrowVec.end());
- StringRef(RawIgnoredExceptions).split(IgnoredExceptionsVec, ",", -1, false);
+
+ std::vector<std::string> 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<FunctionDecl>("thrower");
+ const auto *MatchedDecl = Result.Nodes.getNodeAs<FunctionDecl>("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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
