Skip to content

Commit

Permalink
[clang-tidy] refactor bugprone-exception-escape analysis into class
Browse files Browse the repository at this point in the history 
Summary:
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.

Reviewers: aaron.ballman, alexfh, hokein, baloghadamsoftware, lebedev.ri

Reviewed By: baloghadamsoftware, lebedev.ri

Subscribers: lebedev.ri, mgorny, xazax.hun, rnkovacs, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D57100

llvm-svn: 352741
  • Loading branch information
@JonasToth
JonasToth committed Jan 31, 2019
1 parent 4b70204 commit 9b12742
Show file tree
Hide file tree
Showing 5 changed files with 223 additions and 151 deletions.
165 changes: 17 additions & 148 deletions clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
View file
@@ -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.
Expand All @@ -10,158 +10,21 @@

#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(
Expand All @@ -173,9 +36,12 @@ ExceptionEscapeCheck::ExceptionEscapeCheck(StringRef Name,
.split(FunctionsThatShouldNotThrowVec, ",", -1, false);
FunctionsThatShouldNotThrow.insert(FunctionsThatShouldNotThrowVec.begin(),
FunctionsThatShouldNotThrowVec.end());

llvm::StringSet<> IgnoredExceptions;
StringRef(RawIgnoredExceptions).split(IgnoredExceptionsVec, ",", -1, false);
IgnoredExceptions.insert(IgnoredExceptionsVec.begin(),
IgnoredExceptionsVec.end());
Tracer.ignoreExceptions(std::move(IgnoredExceptions));
}

void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Expand All @@ -193,22 +59,25 @@ void ExceptionEscapeCheck::registerMatchers(MatchFinder *Finder) {
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
Expand Down
6 changes: 3 additions & 3 deletions clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.h
View file
@@ -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.
Expand All @@ -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 {
Expand All @@ -36,7 +36,7 @@ class ExceptionEscapeCheck : public ClangTidyCheck {
std::string RawIgnoredExceptions;

llvm::StringSet<> FunctionsThatShouldNotThrow;
llvm::StringSet<> IgnoredExceptions;
utils::ExceptionAnalyzer Tracer;
};

} // namespace bugprone
Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/utils/CMakeLists.txt
View file
Expand Up @@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support)
add_clang_library(clangTidyUtils
ASTUtils.cpp
DeclRefExprUtils.cpp
ExceptionAnalyzer.cpp
ExprSequence.cpp
FixItHintUtils.cpp
HeaderFileExtensionsUtils.cpp
Expand Down

0 comments on commit 9b12742

Please sign in to comment.