PiotrZSL created this revision.
PiotrZSL added reviewers: njames93, carlosgalvezp, isuckatcs, JonasToth,
baloghadamsoftware.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
During call stack analysis skip called noexcept functions
as they wont throw exceptions, they will crash.
Check will emit warnings for those functions separately.
Fixes: #43667, #49151, #51596, #54668, #54956
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D153458
Files:
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -152,7 +152,7 @@
}
// FIXME: In this case 'a' is convertible to the handler and should be caught
-// but in reality it's thrown. Note that clang doesn't report a warning for
+// but in reality it's thrown. Note that clang doesn't report a warning for
// this either.
void throw_catch_multi_ptr_5() noexcept {
try {
@@ -249,7 +249,7 @@
void throw_derived_catch_base_ptr_c() noexcept {
try {
derived d;
- throw &d;
+ throw &d;
} catch(const base *) {
}
}
@@ -259,7 +259,7 @@
try {
derived d;
const derived *p = &d;
- throw p;
+ throw p;
} catch(base *) {
}
}
@@ -282,7 +282,7 @@
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_private' which should not throw exceptions
try {
B b;
- throw b;
+ throw b;
} catch(A) {
}
}
@@ -291,7 +291,7 @@
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_private_ptr' which should not throw exceptions
try {
B b;
- throw &b;
+ throw &b;
} catch(A *) {
}
}
@@ -300,7 +300,7 @@
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_protected' which should not throw exceptions
try {
C c;
- throw c;
+ throw c;
} catch(A) {
}
}
@@ -309,7 +309,7 @@
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_protected_ptr' which should not throw exceptions
try {
C c;
- throw &c;
+ throw &c;
} catch(A *) {
}
}
@@ -318,7 +318,7 @@
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ambiguous' which should not throw exceptions
try {
E e;
- throw e;
+ throw e;
} catch(A) {
}
}
@@ -327,7 +327,7 @@
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ambiguous_ptr' which should not throw exceptions
try {
E e;
- throw e;
+ throw e;
} catch(A) {
}
}
@@ -703,3 +703,22 @@
throw 1;
return 0;
}
+
+// The following function all incorrectly throw exceptions, *but* calling them
+// should not yield a warning because they are marked as noexcept.
+
+void test_basic_no_throw() noexcept { throw 42; }
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'test_basic_no_throw' which should not throw exceptions
+
+void test_basic_throw() noexcept(false) { throw 42; }
+
+void only_calls_non_throwing() noexcept {
+ test_basic_no_throw();
+}
+
+void calls_non_and_throwing() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'calls_non_and_throwing' which should not throw exceptions
+ test_basic_no_throw();
+ test_basic_throw();
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -254,7 +254,8 @@
- Improved :doc:`bugprone-exception-escape
<clang-tidy/checks/bugprone/exception-escape>` to not emit warnings for
- forward declarations of functions.
+ forward declarations of functions and additionally modified it to skip
+ ``noexcept`` functions during call stack analysis.
- Fixed :doc:`bugprone-exception-escape<clang-tidy/checks/bugprone/exception-escape>`
for coroutines where previously a warning would be emitted with coroutines
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
@@ -316,6 +316,27 @@
}
} // namespace
+static bool cannotThrow(const FunctionDecl *Func) {
+ const auto *FunProto = Func->getType()->getAs<FunctionProtoType>();
+ if (!FunProto)
+ return false;
+
+ switch (FunProto->canThrow()) {
+ case CT_Cannot:
+ return true;
+ case CT_Dependent: {
+ const Expr *NoexceptExpr = FunProto->getNoexceptExpr();
+ bool Result;
+ return NoexceptExpr && !NoexceptExpr->isValueDependent() &&
+ NoexceptExpr->EvaluateAsBooleanCondition(
+ Result, Func->getASTContext(), true) &&
+ Result;
+ }
+ default:
+ return false;
+ };
+}
+
bool ExceptionAnalyzer::ExceptionInfo::filterByCatch(
const Type *HandlerTy, const ASTContext &Context) {
llvm::SmallVector<const Type *, 8> TypesToDelete;
@@ -421,7 +442,8 @@
ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException(
const FunctionDecl *Func, const ExceptionInfo::Throwables &Caught,
llvm::SmallSet<const FunctionDecl *, 32> &CallStack) {
- if (CallStack.count(Func))
+ if (!Func || CallStack.count(Func) ||
+ (!CallStack.empty() && cannotThrow(Func)))
return ExceptionInfo::createNonThrowing();
if (const Stmt *Body = Func->getBody()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
