[clang] [clang-tools-extra] [clang]: Propagate `*noreturn` attributes in `CFG` (PR #146355)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 7 02:57:57 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: Andrey Karlov (negativ)
<details>
<summary>Changes</summary>
## Summary
This PR extends `Clang`'s `CFG` analysis to automatically propagate `analyzer_noreturn` attributes through function call chains. When a function exclusively calls no-return functions, it is automatically treated as no-return as well, improving static analysis precision.
## Problem
Currently, simple forwarder functions to no-return calls are not recognized as no-return themselves:
```cpp
void terminate() __attribute__((analyzer_noreturn)) { }
void fatal_error() {
terminate(); // This "never" returns, but fatal_error() is not marked as no-return
}
void handle_error(const std::optional<int> opt) {
if (!opt) {
fatal_error(); // Static analyzer doesn't know this never returns
}
*opt = 1; // False positive: analyzer thinks this is reachable
}
```
## Solution
Enhanced `isImmediateSinkBlock()` in `CFG.cpp` to perform inter-procedural analysis:
1. **Existing behavior preserved**: Direct no-return elements and throw expressions
2. **Enhanced function call analysis**:
- Checks for existing no-return attributes (`analyzer_noreturn`, `noreturn`, `[[noreturn]]`, `_Noreturn`)
- Recognizes well-known C library functions (`exit`, `abort`, `__assert_fail`, etc.)
- Supports namespaced functions (`BloombergLP::bsls::Assert::invokeHandler`, `std::terminate`)
3. **Recursive CFG analysis**: For user-defined functions, builds CFG and checks if all execution paths lead to no-return calls
## Implementation Details
- Modified `isImmediateSinkBlock()` in `lib/Analysis/CFG.cpp`
- Added support for 10+ well-known no-return `C` and `C++` functions
- Inter-procedural analysis limited to functions with available bodies
---
Full diff: https://github.com/llvm/llvm-project/pull/146355.diff
7 Files Affected:
- (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp (+36)
- (modified) clang/include/clang/AST/Decl.h (+5)
- (modified) clang/lib/AST/Decl.cpp (+4)
- (modified) clang/lib/Analysis/CFG.cpp (+74-5)
- (modified) clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp (+2-2)
- (modified) clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp (+39-31)
- (modified) clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp (+81)
``````````diff
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
index 3167b85f0e024..f4f82199a0bb5 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
@@ -141,6 +141,42 @@ void nullable_value_after_swap(BloombergLP::bdlb::NullableValue<int> &opt1, Bloo
}
}
+void assertion_handler_imp() __attribute__((analyzer_noreturn));
+
+void assertion_handler() {
+ do {
+ assertion_handler_imp();
+ } while(0);
+}
+
+void function_calling_analyzer_noreturn(const bsl::optional<int>& opt)
+{
+ if (!opt) {
+ assertion_handler();
+ }
+
+ *opt; // no-warning
+}
+
+void abort();
+
+void do_fail() {
+ abort(); // acts like 'abort()' C-function
+}
+
+void invoke_assertion_handler() {
+ do_fail();
+}
+
+void function_calling_well_known_noreturn(const bsl::optional<int>& opt)
+{
+ if (!opt) {
+ invoke_assertion_handler();
+ }
+
+ *opt; // no-warning
+}
+
template <typename T>
void function_template_without_user(const absl::optional<T> &opt) {
opt.value(); // no-warning
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index f70a039bf3517..6ada8785fd0ba 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2630,6 +2630,11 @@ class FunctionDecl : public DeclaratorDecl,
/// an attribute on its declaration or its type.
bool isNoReturn() const;
+ /// Determines whether this function is known to never return for CFG
+ /// analysis. Checks for noreturn attributes on the function declaration
+ /// or its type, including 'analyzer_noreturn' attribute.
+ bool isAnalyzerNoReturn() const;
+
/// True if the function was a definition but its body was skipped.
bool hasSkippedBody() const { return FunctionDeclBits.HasSkippedBody; }
void setHasSkippedBody(bool Skipped = true) {
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index e0362245d6ecd..19336a8e942cd 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -3595,6 +3595,10 @@ bool FunctionDecl::isNoReturn() const {
return false;
}
+bool FunctionDecl::isAnalyzerNoReturn() const {
+ return isNoReturn() || hasAttr<AnalyzerNoReturnAttr>();
+}
+
bool FunctionDecl::isMemberLikeConstrainedFriend() const {
// C++20 [temp.friend]p9:
// A non-template friend declaration with a requires-clause [or]
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index d960d5130332b..bc47891216e7a 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -41,6 +41,7 @@
#include "llvm/ADT/APSInt.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallPtrSet.h"
@@ -2833,8 +2834,37 @@ CFGBlock *CFGBuilder::VisitCallExpr(CallExpr *C, AddStmtChoice asc) {
if (!FD->isVariadic())
findConstructionContextsForArguments(C);
- if (FD->isNoReturn() || C->isBuiltinAssumeFalse(*Context))
- NoReturn = true;
+ if (!NoReturn)
+ NoReturn = FD->isAnalyzerNoReturn() || C->isBuiltinAssumeFalse(*Context);
+
+ // Some well-known 'noreturn' functions
+ if (!NoReturn)
+ NoReturn = llvm::StringSwitch<bool>(FD->getQualifiedNameAsString())
+ .Case("BloombergLP::bsls::Assert::invokeHandler", true)
+ .Case("std::terminate", true)
+ .Case("std::abort", true)
+ .Case("exit", true)
+ .Case("abort", true)
+ .Case("panic", true)
+ .Case("error", true)
+ .Case("Assert", true)
+ .Case("ziperr", true)
+ .Case("assfail", true)
+ .Case("db_error", true)
+ .Case("__assert", true)
+ .Case("__assert2", true)
+ .Case("_wassert", true)
+ .Case("__assert_rtn", true)
+ .Case("__assert_fail", true)
+ .Case("dtrace_assfail", true)
+ .Case("yy_fatal_error", true)
+ .Case("_XCAssertionFailureHandler", true)
+ .Case("_DTAssertionFailureHandler", true)
+ .Case("_TSAssertionFailureHandler", true)
+ .Case("__builtin_trap", true)
+ .Case("__builtin_unreachable", true)
+ .Default(false);
+
if (FD->hasAttr<NoThrowAttr>())
AddEHEdge = false;
if (isBuiltinAssumeWithSideEffects(FD->getASTContext(), C) ||
@@ -6288,6 +6318,12 @@ void CFGBlock::printTerminatorJson(raw_ostream &Out, const LangOptions &LO,
// There may be many more reasons why a sink would appear during analysis
// (eg. checkers may generate sinks arbitrarily), but here we only consider
// sinks that would be obvious by looking at the CFG.
+//
+// This function also performs inter-procedural analysis by recursively
+// examining called functions to detect forwarding chains to noreturn
+// functions. When a function is determined to never return through this
+// analysis, it's automatically marked with analyzer_noreturn attribute
+// for caching and future reference.
static bool isImmediateSinkBlock(const CFGBlock *Blk) {
if (Blk->hasNoReturnElement())
return true;
@@ -6298,10 +6334,43 @@ static bool isImmediateSinkBlock(const CFGBlock *Blk) {
// at least for now, but once we have better support for exceptions,
// we'd need to carefully handle the case when the throw is being
// immediately caught.
- if (llvm::any_of(*Blk, [](const CFGElement &Elm) {
+ if (llvm::any_of(*Blk, [](const CFGElement &Elm) -> bool {
+ if (std::optional<CFGStmt> StmtElm = Elm.getAs<CFGStmt>())
+ return isa<CXXThrowExpr>(StmtElm->getStmt());
+ return false;
+ }))
+ return true;
+
+ auto HasNoReturnCall = [](const CallExpr *CE) {
+ if (!CE)
+ return false;
+
+ static thread_local llvm::SmallPtrSet<const FunctionDecl *, 32> InProgress;
+
+ auto *FD = CE->getDirectCallee();
+
+ if (!FD || InProgress.count(FD))
+ return false;
+
+ InProgress.insert(FD);
+ auto DoCleanup = llvm::make_scope_exit([&]() { InProgress.erase(FD); });
+
+ auto NoReturnFromCFG = [FD]() {
+ if (!FD->getBody())
+ return false;
+
+ auto CalleeCFG =
+ CFG::buildCFG(FD, FD->getBody(), &FD->getASTContext(), {});
+
+ return CalleeCFG && CalleeCFG->getEntry().isInevitablySinking();
+ };
+
+ return FD->isAnalyzerNoReturn() || NoReturnFromCFG();
+ };
+
+ if (llvm::any_of(*Blk, [&](const CFGElement &Elm) {
if (std::optional<CFGStmt> StmtElm = Elm.getAs<CFGStmt>())
- if (isa<CXXThrowExpr>(StmtElm->getStmt()))
- return true;
+ return HasNoReturnCall(dyn_cast<CallExpr>(StmtElm->getStmt()));
return false;
}))
return true;
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 1113bbe7f4d9c..c799ca98e4a0d 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -283,7 +283,7 @@ computeBlockInputState(const CFGBlock &Block, AnalysisContext &AC) {
JoinedStateBuilder Builder(AC, JoinBehavior);
for (const CFGBlock *Pred : Preds) {
// Skip if the `Block` is unreachable or control flow cannot get past it.
- if (!Pred || Pred->hasNoReturnElement())
+ if (!Pred || Pred->isInevitablySinking())
continue;
// Skip if `Pred` was not evaluated yet. This could happen if `Pred` has a
@@ -562,7 +562,7 @@ runTypeErasedDataflowAnalysis(
BlockStates[Block->getBlockID()] = std::move(NewBlockState);
// Do not add unreachable successor blocks to `Worklist`.
- if (Block->hasNoReturnElement())
+ if (Block->isInevitablySinking())
continue;
Worklist.enqueueSuccessors(Block);
diff --git a/clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
index 17c3cb4e9e04c..834bd81c2fa21 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
@@ -50,37 +50,45 @@ void NoReturnFunctionChecker::checkPostCall(const CallEvent &CE,
BuildSinks = getFunctionExtInfo(C->getType()).getNoReturn();
}
- if (!BuildSinks && CE.isGlobalCFunction()) {
- if (const IdentifierInfo *II = CE.getCalleeIdentifier()) {
- // HACK: Some functions are not marked noreturn, and don't return.
- // Here are a few hardwired ones. If this takes too long, we can
- // potentially cache these results.
- BuildSinks
- = llvm::StringSwitch<bool>(StringRef(II->getName()))
- .Case("exit", true)
- .Case("panic", true)
- .Case("error", true)
- .Case("Assert", true)
- // FIXME: This is just a wrapper around throwing an exception.
- // Eventually inter-procedural analysis should handle this easily.
- .Case("ziperr", true)
- .Case("assfail", true)
- .Case("db_error", true)
- .Case("__assert", true)
- .Case("__assert2", true)
- // For the purpose of static analysis, we do not care that
- // this MSVC function will return if the user decides to continue.
- .Case("_wassert", true)
- .Case("__assert_rtn", true)
- .Case("__assert_fail", true)
- .Case("dtrace_assfail", true)
- .Case("yy_fatal_error", true)
- .Case("_XCAssertionFailureHandler", true)
- .Case("_DTAssertionFailureHandler", true)
- .Case("_TSAssertionFailureHandler", true)
- .Default(false);
- }
- }
+ if (!BuildSinks && CE.isGlobalCFunction()) {
+ if (const IdentifierInfo *II = CE.getCalleeIdentifier()) {
+ // HACK: Some functions are not marked noreturn, and don't return.
+ // Here are a few hardwired ones. If this takes too long, we can
+ // potentially cache these results.
+ //
+ // (!) In case of function list update, please also update
+ // CFGBuilder::VisitCallExpr (CFG.cpp)
+ BuildSinks =
+ llvm::StringSwitch<bool>(StringRef(II->getName()))
+ .Case("exit", true)
+ .Case("abort", true)
+ .Case("panic", true)
+ .Case("error", true)
+ .Case("Assert", true)
+ // FIXME: This is just a wrapper around throwing an exception.
+ // Eventually inter-procedural analysis should handle this
+ // easily.
+ .Case("ziperr", true)
+ .Case("assfail", true)
+ .Case("db_error", true)
+ .Case("__assert", true)
+ .Case("__assert2", true)
+ // For the purpose of static analysis, we do not care that
+ // this MSVC function will return if the user decides to
+ // continue.
+ .Case("_wassert", true)
+ .Case("__assert_rtn", true)
+ .Case("__assert_fail", true)
+ .Case("dtrace_assfail", true)
+ .Case("yy_fatal_error", true)
+ .Case("_XCAssertionFailureHandler", true)
+ .Case("_DTAssertionFailureHandler", true)
+ .Case("_TSAssertionFailureHandler", true)
+ .Case("__builtin_trap", true)
+ .Case("__builtin_unreachable", true)
+ .Default(false);
+ }
+ }
if (BuildSinks)
C.generateSink(C.getState(), C.getPredecessor());
diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index 9fb7bebdbe41e..9150b3e155533 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -49,6 +49,7 @@ using namespace ast_matchers;
using llvm::IsStringMapEntry;
using ::testing::DescribeMatcher;
using ::testing::IsEmpty;
+using ::testing::Not;
using ::testing::NotNull;
using ::testing::Test;
using ::testing::UnorderedElementsAre;
@@ -693,6 +694,86 @@ TEST_F(NoreturnDestructorTest, ConditionalOperatorNestedBranchReturns) {
// FIXME: Called functions at point `p` should contain only "foo".
}
+class AnalyzerNoreturnTest : public Test {
+protected:
+ template <typename Matcher>
+ void runDataflow(llvm::StringRef Code, Matcher Expectations) {
+ tooling::FileContentMappings FilesContents;
+ FilesContents.push_back(
+ std::make_pair<std::string, std::string>("noreturn_test_defs.h", R"(
+ void assertionHandler() __attribute__((analyzer_noreturn));
+
+ void assertionTrampoline() {
+ assertionHandler();
+ }
+
+ void trap() {}
+ )"));
+
+ ASSERT_THAT_ERROR(
+ test::checkDataflow<FunctionCallAnalysis>(
+ AnalysisInputs<FunctionCallAnalysis>(
+ Code, ast_matchers::hasName("target"),
+ [](ASTContext &C, Environment &) {
+ return FunctionCallAnalysis(C);
+ })
+ .withASTBuildArgs({"-fsyntax-only", "-std=c++17"})
+ .withASTBuildVirtualMappedFiles(std::move(FilesContents)),
+ /*VerifyResults=*/
+ [&Expectations](
+ const llvm::StringMap<
+ DataflowAnalysisState<FunctionCallLattice>> &Results,
+ const AnalysisOutputs &) {
+ EXPECT_THAT(Results, Expectations);
+ }),
+ llvm::Succeeded());
+ }
+};
+
+TEST_F(AnalyzerNoreturnTest, Breathing) {
+ std::string Code = R"(
+ #include "noreturn_test_defs.h"
+
+ void target() {
+ trap();
+ // [[p]]
+ }
+ )";
+ runDataflow(Code, UnorderedElementsAre(IsStringMapEntry(
+ "p", HoldsFunctionCallLattice(HasCalledFunctions(
+ UnorderedElementsAre("trap"))))));
+}
+
+TEST_F(AnalyzerNoreturnTest, DirectNoReturnCall) {
+ std::string Code = R"(
+ #include "noreturn_test_defs.h"
+
+ void target() {
+ assertionHandler();
+ trap();
+ // [[p]]
+ }
+ )";
+ runDataflow(Code, Not(UnorderedElementsAre(IsStringMapEntry(
+ "p", HoldsFunctionCallLattice(HasCalledFunctions(
+ UnorderedElementsAre("trap")))))));
+}
+
+TEST_F(AnalyzerNoreturnTest, IndirectNoReturnCall) {
+ std::string Code = R"(
+ #include "noreturn_test_defs.h"
+
+ void target() {
+ assertionTrampoline();
+ trap();
+ // [[p]]
+ }
+ )";
+ runDataflow(Code, Not(UnorderedElementsAre(IsStringMapEntry(
+ "p", HoldsFunctionCallLattice(HasCalledFunctions(
+ UnorderedElementsAre("trap")))))));
+}
+
// Models an analysis that uses flow conditions.
class SpecialBoolAnalysis final
: public DataflowAnalysis<SpecialBoolAnalysis, NoopLattice> {
``````````
</details>
https://github.com/llvm/llvm-project/pull/146355
More information about the cfe-commits
mailing list