[clang-tidy] Check functions called from catch blocks

[denizevrenci]

Summary

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.

[ChuanqiXu]

clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp

132–136

I don't know clang-tidy a lot. But is these two branches different in clang-tidy really?

[denizevrenci]

clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp

132–136

Yes, rethrows are handled differently than throw expressions with arguments. You can find the relevant implementation in ExceptionAnalyzer.cpp:[462:475].

[ChuanqiXu]

LGTM. And please wait for several days for other reviewers.

clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp

132–136

OK, sounds good to me.

[denizevrenci]

Rebase on main

[denizevrenci]

Sort out tests

[denizevrenci]

Sort tests out a bit more

[denizevrenci]

Rebase on main

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h

public:
   }
 
   ExceptionInfo analyze(const FunctionDecl *Func);
   ExceptionInfo analyze(const Stmt *Stmt);
 
 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,
                   llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
 
   ExceptionInfo analyzeImpl(const FunctionDecl *Func);
   ExceptionInfo analyzeImpl(const Stmt *Stmt);
 

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

if (ContainsUnknown)
       Behaviour = State::Unknown;
     else
       Behaviour = State::NotThrowing;
   else
     Behaviour = State::Throwing;
 }
 
 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);
       }
     }
 
     CallStack.erase(Func);
     return Result;
   }
 
for (unsigned I = 0; I < Try->getNumHandlers(); ++I) {
                                                    CaughtExceptions, CallStack);
           Results.merge(Rethrown);
         }
       }
     }
     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 =
         throwsException(DefaultInit->getExpr(), Caught, CallStack);
     Results.merge(Excs);
   } else if (const auto *Coro = dyn_cast<CoroutineBodyStmt>(St)) {
     for (const Stmt *Child : Coro->childrenExclBody()) {
+      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);
       }
     }
   } else {
     for (const Stmt *Child : St->children()) {
       ExceptionInfo Excs = throwsException(Child, Caught, CallStack);
       Results.merge(Excs);
     }
   }
   return Results;
 }
 
 ExceptionAnalyzer::ExceptionInfo
 ExceptionAnalyzer::analyzeImpl(const FunctionDecl *Func) {
   ExceptionInfo ExceptionList;
 
   // Check if the function has already been analyzed and reuse that result.
   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.
     // The results here might be relevant to different analysis passes
     // with different needs as well.
     FunctionCache.try_emplace(Func, ExceptionList);
   } else
     ExceptionList = CacheEntry->getSecond();

clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp

struct suspend_never {
   void await_suspend(coroutine_handle<>) noexcept {}
   void await_resume() noexcept {}
 };
 
 } // namespace std
 
 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) {
       throw 1;
     }
 
     p.return_val = this;
   }
 
   bool await_ready() { return true; }
 
   void await_suspend(std::coroutine_handle<> h) {}
 
   void await_resume() {}
 
   T value;
 };
 
 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) {
       throw 1;
     }
 
     p.return_val = this;
   }
 
   bool await_ready() { return true; }
 
   void await_suspend(std::coroutine_handle<> h) {}
 
   void await_resume() {}
 };
 
 template <typename Task, typename T, bool ThrowInPromiseConstructor,
           bool ThrowInInitialSuspend, bool ThrowInGetReturnObject,
-          bool ThrowInUnhandledException>
+          bool ThrowInUnhandledException, bool RethrowInUnhandledException>
 struct Promise {
   Promise() {
     if constexpr (ThrowInPromiseConstructor) {
       throw 1;
     }
   }
 
   Task get_return_object() {
struct Promise {
   }
 
   template <typename U> std::suspend_never yield_value(U &&val) {
     return_val->value = static_cast<U &&>(val);
     return {};
   }
 
   void unhandled_exception() {
     if constexpr (ThrowInUnhandledException) {
       throw 1;
+    } else if constexpr (RethrowInUnhandledException) {
+      throw;
     }
   }
 
   Task *return_val;
 };
 
 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;
     }
   }
 
   Task get_return_object() {
     if constexpr (ThrowInGetReturnObject) {
struct Promise<Task, void, ThrowInPromiseConstructor, ThrowInInitialSuspend,
 
   std::suspend_never final_suspend() const noexcept { return {}; }
 
   void return_void() {}
 
   void unhandled_exception() {
     if constexpr (ThrowInUnhandledException) {
       throw 1;
+    } else if constexpr (RethrowInUnhandledException) {
+      throw;
     }
   }
 
   Task *return_val;
 };
 
 struct Evil {
   ~Evil() noexcept(false) {

 }
 
 Task<int, false, false, false, false, true> h_ShouldDiag(const int a,
                                                          const int b) noexcept {
   // CHECK-MESSAGES: :[[@LINE-2]]:45: warning: an exception may be thrown in function 'h_ShouldDiag' which should not throw exceptions
   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 {
 
 Task<int> a_ShouldNotDiag(const int a, const int b) {
   if (b == 0)
     throw b;
 

 }
 
 Task<int, false, false, false, false, true> h_ShouldDiag(const int a,
                                                          const int b) noexcept {
   // CHECK-MESSAGES: :[[@LINE-2]]:45: warning: an exception may be thrown in function 'h_ShouldDiag' which should not throw exceptions
   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 {
 
 Task<void> a_ShouldNotDiag(const int a, const int b) {
   if (b == 0)
     throw b;
 

 }
 
 Task<void, false, false, false, false, true>
 h_ShouldDiag(const int a, const int b) noexcept {
   // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: an exception may be thrown in function 'h_ShouldDiag' which should not throw exceptions
   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
 
 namespace lambda {
 
 namespace coreturn {
 

 
 const auto h_ShouldDiag =
     [](const int a,
        const int b) noexcept -> Task<int, 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_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 {
 
 const auto a_ShouldNotDiag = [](const int a, const int b) -> Task<int> {
   if (b == 0)
     throw b;
 

 
 const auto h_ShouldDiag =
     [](const int a,
        const int b) noexcept -> Task<int, 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_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 {
 
 const auto a_ShouldNotDiag = [](const int a, const int b) -> Task<void> {
   if (b == 0)
     throw b;
 

 
 const auto h_ShouldDiag =
     [](const int a,
        const int b) noexcept -> Task<void, 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();
 };
 
+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

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

+// 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;
+}