denizevrenci created this revision.
denizevrenci added reviewers: njames93, PiotrZSL, ChuanqiXu.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
denizevrenci requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
These functions can rethrow a current exception that is caught by the
catch block. We can pass the currently caught excections to the function
declaration analyzer just like the statement analyzer to handle this
case.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D152330
Files:
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-rethrow.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-rethrow.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-rethrow.cpp
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy -std=c++11,c++14,c++17,c++20 %s bugprone-exception-escape %t -- \
+// RUN: -- -fexceptions
+
+void rethrower() {
+ throw;
+}
+
+void callsRethrower() {
+ rethrower();
+}
+
+void callsRethrowerNoexcept() noexcept {
+ rethrower();
+}
+
+int throwsAndCallsRethrower() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'throwsAndCallsRethrower' which should not throw exceptions
+ try {
+ throw 1;
+ } catch(...) {
+ rethrower();
+ }
+}
+
+int throwsAndCallsCallsRethrower() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'throwsAndCallsCallsRethrower' which should not throw exceptions
+ try {
+ throw 1;
+ } catch(...) {
+ callsRethrower();
+ }
+}
+
+void rethrowerNoexcept() noexcept {
+ throw;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
@@ -36,17 +36,18 @@
template <typename Task, typename T, bool ThrowInPromiseConstructor,
bool ThrowInInitialSuspend, bool ThrowInGetReturnObject,
- bool ThrowInUnhandledException>
+ bool ThrowInUnhandledException, bool RethrowInUnhandledException>
struct Promise;
template <
typename T, bool ThrowInTaskConstructor = false,
bool ThrowInPromiseConstructor = false, bool ThrowInInitialSuspend = false,
- bool ThrowInGetReturnObject = false, bool ThrowInUnhandledException = false>
+ bool ThrowInGetReturnObject = false, bool ThrowInUnhandledException = false,
+ bool RethrowInUnhandledException = false>
struct Task {
using promise_type =
Promise<Task, T, ThrowInPromiseConstructor, ThrowInInitialSuspend,
- ThrowInGetReturnObject, ThrowInUnhandledException>;
+ ThrowInGetReturnObject, ThrowInUnhandledException, RethrowInUnhandledException>;
explicit Task(promise_type &p) {
if constexpr (ThrowInTaskConstructor) {
@@ -67,13 +68,13 @@
template <bool ThrowInTaskConstructor, bool ThrowInPromiseConstructor,
bool ThrowInInitialSuspend, bool ThrowInGetReturnObject,
- bool ThrowInUnhandledException>
+ bool ThrowInUnhandledException, bool RethrowInUnhandledException>
struct Task<void, ThrowInTaskConstructor, ThrowInPromiseConstructor,
ThrowInInitialSuspend, ThrowInGetReturnObject,
- ThrowInUnhandledException> {
+ ThrowInUnhandledException, RethrowInUnhandledException> {
using promise_type =
Promise<Task, void, ThrowInPromiseConstructor, ThrowInInitialSuspend,
- ThrowInGetReturnObject, ThrowInUnhandledException>;
+ ThrowInGetReturnObject, ThrowInUnhandledException, RethrowInUnhandledException>;
explicit Task(promise_type &p) {
if constexpr (ThrowInTaskConstructor) {
@@ -92,7 +93,7 @@
template <typename Task, typename T, bool ThrowInPromiseConstructor,
bool ThrowInInitialSuspend, bool ThrowInGetReturnObject,
- bool ThrowInUnhandledException>
+ bool ThrowInUnhandledException, bool RethrowInUnhandledException>
struct Promise {
Promise() {
if constexpr (ThrowInPromiseConstructor) {
@@ -130,6 +131,8 @@
void unhandled_exception() {
if constexpr (ThrowInUnhandledException) {
throw 1;
+ } else if constexpr (RethrowInUnhandledException) {
+ throw;
}
}
@@ -138,9 +141,9 @@
template <typename Task, bool ThrowInPromiseConstructor,
bool ThrowInInitialSuspend, bool ThrowInGetReturnObject,
- bool ThrowInUnhandledException>
+ bool ThrowInUnhandledException, bool RethrowInUnhandledException>
struct Promise<Task, void, ThrowInPromiseConstructor, ThrowInInitialSuspend,
- ThrowInGetReturnObject, ThrowInUnhandledException> {
+ ThrowInGetReturnObject, ThrowInUnhandledException, RethrowInUnhandledException> {
Promise() {
if constexpr (ThrowInPromiseConstructor) {
throw 1;
@@ -170,6 +173,8 @@
void unhandled_exception() {
if constexpr (ThrowInUnhandledException) {
throw 1;
+ } else if constexpr (RethrowInUnhandledException) {
+ throw;
}
}
@@ -266,6 +271,23 @@
co_return a / b;
}
+Task<int, false, false, false, false, false, true>
+i_ShouldNotDiag(const int a, const int b) {
+ co_return a / b;
+}
+
+Task<int, false, false, false, false, false, true>
+i_ShouldNotDiagNoexcept(const int a, const int b) noexcept {
+ co_return a / b;
+}
+
+Task<int, false, false, false, false, false, true>
+i_ShouldDiag(const int a, const int b) noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: an exception may be thrown in function 'i_ShouldDiag' which should not throw exceptions
+ throw 1;
+ co_return a / b;
+}
+
} // namespace coreturn
namespace coyield {
@@ -347,6 +369,23 @@
co_yield a / b;
}
+Task<int, false, false, false, false, false, true>
+i_ShouldNotDiag(const int a, const int b) {
+ co_yield a / b;
+}
+
+Task<int, false, false, false, false, false, true>
+i_ShouldNotDiagNoexcept(const int a, const int b) noexcept {
+ co_yield a / b;
+}
+
+Task<int, false, false, false, false, false, true>
+i_ShouldDiag(const int a, const int b) noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: an exception may be thrown in function 'i_ShouldDiag' which should not throw exceptions
+ throw 1;
+ co_yield a / b;
+}
+
} // namespace coyield
namespace coawait {
@@ -429,6 +468,23 @@
co_await returnOne();
}
+Task<int, false, false, false, false, false, true>
+i_ShouldNotDiag(const int a, const int b) {
+ co_await returnOne();
+}
+
+Task<int, false, false, false, false, false, true>
+i_ShouldNotDiagNoexcept(const int a, const int b) noexcept {
+ co_await returnOne();
+}
+
+Task<int, false, false, false, false, false, true>
+i_ShouldDiag(const int a, const int b) noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: an exception may be thrown in function 'i_ShouldDiag' which should not throw exceptions
+ co_await returnOne();
+ throw 1;
+}
+
} // namespace coawait
} // namespace function
@@ -524,6 +580,26 @@
co_return a / b;
};
+const auto i_ShouldNotDiag =
+ [](const int a,
+ const int b) -> Task<int, false, false, false, false, false, true> {
+ co_return a / b;
+};
+
+const auto i_ShouldNotDiagNoexcept =
+ [](const int a,
+ const int b) -> Task<int, false, false, false, false, false, true> {
+ co_return a / b;
+};
+
+const auto i_ShouldDiag =
+ [](const int a,
+ const int b) noexcept -> Task<int, false, false, false, false, false, true> {
+ // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
+ throw 1;
+ co_return a / b;
+};
+
} // namespace coreturn
namespace coyield {
@@ -615,6 +691,26 @@
co_yield a / b;
};
+const auto i_ShouldNotDiag =
+ [](const int a,
+ const int b) -> Task<int, false, false, false, false, false, true> {
+ co_yield a / b;
+};
+
+const auto i_ShouldNotDiagNoexcept =
+ [](const int a,
+ const int b) -> Task<int, false, false, false, false, false, true> {
+ co_yield a / b;
+};
+
+const auto i_ShouldDiag =
+ [](const int a,
+ const int b) noexcept -> Task<int, false, false, false, false, false, true> {
+ // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
+ throw 1;
+ co_yield a / b;
+};
+
} // namespace coyield
namespace coawait {
@@ -706,6 +802,26 @@
co_await returnOne();
};
+const auto i_ShouldNotDiag =
+ [](const int a,
+ const int b) -> Task<int, false, false, false, false, false, true> {
+ co_await returnOne();
+};
+
+const auto i_ShouldNotDiagNoexcept =
+ [](const int a,
+ const int b) -> Task<int, false, false, false, false, false, true> {
+ co_await returnOne();
+};
+
+const auto i_ShouldDiag =
+ [](const int a,
+ const int b) noexcept -> Task<int, false, false, false, false, false, true> {
+ // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: an exception may be thrown in function 'operator()' which should not throw exceptions
+ co_await returnOne();
+ throw 1;
+};
+
} // namespace coawait
} // namespace lambda
Index: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
===================================================================
--- clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
+++ clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
@@ -132,6 +132,7 @@
private:
ExceptionInfo
throwsException(const FunctionDecl *Func,
+ const ExceptionInfo::Throwables &Caught,
llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
ExceptionInfo
throwsException(const Stmt *St, const ExceptionInfo::Throwables &Caught,
Index: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
+++ clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -419,21 +419,20 @@
}
ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException(
- const FunctionDecl *Func,
+ const FunctionDecl *Func, const ExceptionInfo::Throwables &Caught,
llvm::SmallSet<const FunctionDecl *, 32> &CallStack) {
if (CallStack.count(Func))
return ExceptionInfo::createNonThrowing();
if (const Stmt *Body = Func->getBody()) {
CallStack.insert(Func);
- ExceptionInfo Result =
- throwsException(Body, ExceptionInfo::Throwables(), CallStack);
+ ExceptionInfo Result = throwsException(Body, Caught, CallStack);
// For a constructor, we also have to check the initializers.
if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(Func)) {
for (const CXXCtorInitializer *Init : Ctor->inits()) {
- ExceptionInfo Excs = throwsException(
- Init->getInit(), ExceptionInfo::Throwables(), CallStack);
+ ExceptionInfo Excs =
+ throwsException(Init->getInit(), Caught, CallStack);
Result.merge(Excs);
}
}
@@ -512,12 +511,12 @@
Results.merge(Uncaught);
} else if (const auto *Call = dyn_cast<CallExpr>(St)) {
if (const FunctionDecl *Func = Call->getDirectCallee()) {
- ExceptionInfo Excs = throwsException(Func, CallStack);
+ ExceptionInfo Excs = throwsException(Func, Caught, CallStack);
Results.merge(Excs);
}
} else if (const auto *Construct = dyn_cast<CXXConstructExpr>(St)) {
ExceptionInfo Excs =
- throwsException(Construct->getConstructor(), CallStack);
+ throwsException(Construct->getConstructor(), Caught, CallStack);
Results.merge(Excs);
} else if (const auto *DefaultInit = dyn_cast<CXXDefaultInitExpr>(St)) {
ExceptionInfo Excs =
@@ -525,14 +524,18 @@
Results.merge(Excs);
} else if (const auto *Coro = dyn_cast<CoroutineBodyStmt>(St)) {
for (const Stmt *Child : Coro->childrenExclBody()) {
- ExceptionInfo Excs = throwsException(Child, Caught, CallStack);
- Results.merge(Excs);
+ if (Child != Coro->getExceptionHandler()) {
+ ExceptionInfo Excs = throwsException(Child, Caught, CallStack);
+ Results.merge(Excs);
+ }
}
ExceptionInfo Excs = throwsException(Coro->getBody(), Caught, CallStack);
+ Results.merge(throwsException(Coro->getExceptionHandler(),
+ Excs.getExceptionTypes(), CallStack));
for (const Type *Throwable : Excs.getExceptionTypes()) {
if (const auto ThrowableRec = Throwable->getAsCXXRecordDecl()) {
ExceptionInfo DestructorExcs =
- throwsException(ThrowableRec->getDestructor(), CallStack);
+ throwsException(ThrowableRec->getDestructor(), Caught, CallStack);
Results.merge(DestructorExcs);
}
}
@@ -553,7 +556,8 @@
const auto CacheEntry = FunctionCache.find(Func);
if (CacheEntry == FunctionCache.end()) {
llvm::SmallSet<const FunctionDecl *, 32> CallStack;
- ExceptionList = throwsException(Func, CallStack);
+ ExceptionList =
+ throwsException(Func, ExceptionInfo::Throwables(), CallStack);
// Cache the result of the analysis. This is done prior to filtering
// because it is best to keep as much information as possible.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
