[clang-tools-extra] daac014 - [clang-tidy] Check functions called from catch blocks

Deniz Evrenci via cfe-commits cfe-commits at lists.llvm.org
Sun Jun 11 08:40:59 PDT 2023


Author: Deniz Evrenci
Date: 2023-06-11T16:40:29+01:00
New Revision: daac014fec427eda305f93da7891c0122a161bb3

URL: https://github.com/llvm/llvm-project/commit/daac014fec427eda305f93da7891c0122a161bb3
DIFF: https://github.com/llvm/llvm-project/commit/daac014fec427eda305f93da7891c0122a161bb3.diff

LOG: [clang-tidy] Check functions called from catch blocks

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.

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

Added: 
    clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-rethrow.cpp

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
index 690e771414a75..c3e9a13bb1ba2 100644
--- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -419,21 +419,20 @@ void ExceptionAnalyzer::ExceptionInfo::reevaluateBehaviour() {
 }
 
 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 @@ ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException(
     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 @@ ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException(
     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 @@ ExceptionAnalyzer::analyzeImpl(const FunctionDecl *Func) {
   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.

diff  --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
index a40149ac98d84..a1cb111aa3fbf 100644
--- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
+++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
@@ -132,6 +132,7 @@ class ExceptionAnalyzer {
 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,

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
index 9caafe7676f4e..2e748a2374bf9 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
@@ -36,17 +36,18 @@ struct suspend_never {
 
 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 @@ struct Task {
 
 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 @@ struct Task<void, ThrowInTaskConstructor, ThrowInPromiseConstructor,
 
 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 @@ struct Promise {
   void unhandled_exception() {
     if constexpr (ThrowInUnhandledException) {
       throw 1;
+    } else if constexpr (RethrowInUnhandledException) {
+      throw;
     }
   }
 
@@ -138,9 +141,9 @@ struct Promise {
 
 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 @@ struct Promise<Task, void, ThrowInPromiseConstructor, ThrowInInitialSuspend,
   void unhandled_exception() {
     if constexpr (ThrowInUnhandledException) {
       throw 1;
+    } else if constexpr (RethrowInUnhandledException) {
+      throw;
     }
   }
 
@@ -266,6 +271,33 @@ Task<int, false, false, false, false, true> h_ShouldDiag(const int a,
   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>
+j_ShouldNotDiag(const int a, const int b) {
+  if (b == 0)
+    throw b;
+
+  co_return a / b;
+}
+
+Task<int, false, false, false, false, false, true>
+j_ShouldDiag(const int a, const int b) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: an exception may be thrown in function 'j_ShouldDiag' which should not throw exceptions
+  if (b == 0)
+    throw b;
+
+  co_return a / b;
+}
+
 } // namespace coreturn
 
 namespace coyield {
@@ -347,6 +379,33 @@ Task<int, false, false, false, false, true> h_ShouldDiag(const int a,
   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>
+j_ShouldNotDiag(const int a, const int b) {
+  if (b == 0)
+    throw b;
+
+  co_yield a / b;
+}
+
+Task<int, false, false, false, false, false, true>
+j_ShouldDiag(const int a, const int b) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: an exception may be thrown in function 'j_ShouldDiag' which should not throw exceptions
+  if (b == 0)
+    throw b;
+
+  co_yield a / b;
+}
+
 } // namespace coyield
 
 namespace coawait {
@@ -429,6 +488,31 @@ h_ShouldDiag(const int a, const int b) noexcept {
   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>
+j_ShouldNotDiag(const int a, const int b) {
+  co_await returnOne();
+  if (b == 0)
+    throw b;
+}
+
+Task<int, false, false, false, false, false, true>
+j_ShouldDiag(const int a, const int b) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: an exception may be thrown in function 'j_ShouldDiag' which should not throw exceptions
+  co_await returnOne();
+  if (b == 0)
+    throw b;
+}
+
 } // namespace coawait
 
 } // namespace function
@@ -524,6 +608,37 @@ const auto h_ShouldDiag =
   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) noexcept -> Task<int, false, false, false, false, false, true> {
+  co_return a / b;
+};
+
+const auto j_ShouldNotDiag =
+    [](const int a,
+       const int b) -> Task<int, false, false, false, false, false, true> {
+  if (b == 0)
+    throw b;
+
+  co_return a / b;
+};
+
+const auto j_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
+  if (b == 0)
+    throw b;
+
+  co_return a / b;
+};
+
 } // namespace coreturn
 
 namespace coyield {
@@ -615,6 +730,37 @@ const auto h_ShouldDiag =
   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) noexcept -> Task<int, false, false, false, false, false, true> {
+  co_yield a / b;
+};
+
+const auto j_ShouldNotDiag =
+    [](const int a,
+       const int b) -> Task<int, false, false, false, false, false, true> {
+  if (b == 0)
+    throw b;
+
+  co_yield a / b;
+};
+
+const auto j_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
+  if (b == 0)
+    throw b;
+
+  co_yield a / b;
+};
+
 } // namespace coyield
 
 namespace coawait {
@@ -706,6 +852,35 @@ const auto h_ShouldDiag =
   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) noexcept -> Task<int, false, false, false, false, false, true> {
+  co_await returnOne();
+};
+
+const auto j_ShouldNotDiag =
+    [](const int a,
+       const int b) -> Task<int, false, false, false, false, false, true> {
+  co_await returnOne();
+  if (b == 0)
+    throw b;
+};
+
+const auto j_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();
+  if (b == 0)
+    throw b;
+};
+
 } // namespace coawait
 
 } // namespace lambda

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-rethrow.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-rethrow.cpp
new file mode 100644
index 0000000000000..b20333d5b0b3b
--- /dev/null
+++ b/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;
+}


        


More information about the cfe-commits mailing list