Skip to content

Commit

Permalink
[clang-tidy] Model noexcept more properly in bugprone-exception-escape
Browse files Browse the repository at this point in the history
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

Reviewed By: carlosgalvezp

Differential Revision: https://reviews.llvm.org/D153458
  • Loading branch information
PiotrZSL committed Jul 17, 2023
1 parent a64b3e9 commit 2724507
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 11 deletions.
30 changes: 29 additions & 1 deletion clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,34 @@ bool isQualificationConvertiblePointer(QualType From, QualType To,
}
} // namespace

static bool canThrow(const FunctionDecl *Func) {
const auto *FunProto = Func->getType()->getAs<FunctionProtoType>();
if (!FunProto)
return true;

switch (FunProto->canThrow()) {
case CT_Cannot:
return false;
case CT_Dependent: {
const Expr *NoexceptExpr = FunProto->getNoexceptExpr();
if (!NoexceptExpr)
return true; // no noexept - can throw

if (NoexceptExpr->isValueDependent())
return true; // depend on template - some instance can throw

bool Result = false;
if (!NoexceptExpr->EvaluateAsBooleanCondition(Result, Func->getASTContext(),
/*InConstantContext=*/true))
return true; // complex X condition in noexcept(X), cannot validate,
// assume that may throw
return !Result; // noexcept(false) - can throw
}
default:
return true;
};
}

bool ExceptionAnalyzer::ExceptionInfo::filterByCatch(
const Type *HandlerTy, const ASTContext &Context) {
llvm::SmallVector<const Type *, 8> TypesToDelete;
Expand Down Expand Up @@ -421,7 +449,7 @@ void ExceptionAnalyzer::ExceptionInfo::reevaluateBehaviour() {
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() && !canThrow(Func)))
return ExceptionInfo::createNonThrowing();

if (const Stmt *Body = Func->getBody()) {
Expand Down
3 changes: 2 additions & 1 deletion clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,8 @@ Changes in existing checks

- 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ void throw_catch_multi_ptr_4() noexcept {
}

// 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 {
Expand Down Expand Up @@ -249,7 +249,7 @@ void throw_derived_catch_base_alias() noexcept {
void throw_derived_catch_base_ptr_c() noexcept {
try {
derived d;
throw &d;
throw &d;
} catch(const base *) {
}
}
Expand All @@ -259,7 +259,7 @@ void throw_derived_catch_base_ptr() noexcept {
try {
derived d;
const derived *p = &d;
throw p;
throw p;
} catch(base *) {
}
}
Expand All @@ -282,7 +282,7 @@ void throw_derived_catch_base_private() noexcept {
// 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) {
}
}
Expand All @@ -291,7 +291,7 @@ void throw_derived_catch_base_private_ptr() noexcept {
// 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 *) {
}
}
Expand All @@ -300,7 +300,7 @@ void throw_derived_catch_base_protected() noexcept {
// 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) {
}
}
Expand All @@ -309,7 +309,7 @@ void throw_derived_catch_base_protected_ptr() noexcept {
// 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 *) {
}
}
Expand All @@ -318,7 +318,7 @@ void throw_derived_catch_base_ambiguous() noexcept {
// 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) {
}
}
Expand All @@ -327,7 +327,7 @@ void throw_derived_catch_base_ambiguous_ptr() noexcept {
// 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) {
}
}
Expand Down Expand Up @@ -703,3 +703,22 @@ int main() {
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();
}

0 comments on commit 2724507

Please sign in to comment.