[clang]: Support analyzer_noreturn attribute in CFG
#150952
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang Author: Andrey Karlov (negativ) ChangesProblemCurrently, functions with void assertion_handler() __attribute__((analyzer_noreturn)) {
log(...);
}
void handle_error(const std::optional<int> opt) {
if (!opt) {
fatal_error(); // Static analyzer doesn't know this never returns
}
*opt = 1; // False-positive `unchecked-optional-access` warning as analyzer thinks this is reachable
}Solution
CommentsThis PR incorporates part of the work done in #146355 Full diff: https://github.com/llvm/llvm-project/pull/150952.diff 5 Files Affected:
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..4911157828765 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,17 @@ void nullable_value_after_swap(BloombergLP::bdlb::NullableValue<int> &opt1, Bloo
}
}
+void assertion_handler() __attribute__((analyzer_noreturn));
+
+void function_calling_analyzer_noreturn(const bsl::optional<int>& opt)
+{
+ if (!opt) {
+ assertion_handler();
+ }
+
+ *opt; // no-warning: The previous condition guards this dereference.
+}
+
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 08fe1f881503b..d58920270083a 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2668,6 +2668,10 @@ class FunctionDecl : public DeclaratorDecl,
/// an attribute on its declaration or its type.
bool isNoReturn() const;
+ /// Determines whether this function is known to be 'noreturn' for analyzer,
+ /// through an `analyzer_noreturn` attribute on its declaration.
+ 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 83fcd87aec2f8..3c0b55f3e3b68 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -3596,6 +3596,10 @@ bool FunctionDecl::isNoReturn() const {
return false;
}
+bool FunctionDecl::isAnalyzerNoReturn() const {
+ return 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..60a2d113c08e2 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -2833,7 +2833,8 @@ CFGBlock *CFGBuilder::VisitCallExpr(CallExpr *C, AddStmtChoice asc) {
if (!FD->isVariadic())
findConstructionContextsForArguments(C);
- if (FD->isNoReturn() || C->isBuiltinAssumeFalse(*Context))
+ if (FD->isNoReturn() || FD->isAnalyzerNoReturn() ||
+ C->isBuiltinAssumeFalse(*Context))
NoReturn = true;
if (FD->hasAttr<NoThrowAttr>())
AddEHEdge = false;
diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index 9fb7bebdbe41e..d1dd4ff3ea33e 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -693,6 +693,80 @@ 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 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, IsEmpty());
+}
+
+TEST_F(AnalyzerNoreturnTest, CanonicalDeclCallCheck) {
+ std::string Code = R"(
+ #include "noreturn_test_defs.h"
+
+ extern void assertionHandler();
+
+ void target() {
+ assertionHandler();
+ trap();
+ // [[p]]
+ }
+ )";
+ runDataflow(Code, IsEmpty());
+}
+
// Models an analysis that uses flow conditions.
class SpecialBoolAnalysis final
: public DataflowAnalysis<SpecialBoolAnalysis, NoopLattice> {
|
|
@llvm/pr-subscribers-clang-tools-extra Author: Andrey Karlov (negativ) ChangesProblemCurrently, functions with void assertion_handler() __attribute__((analyzer_noreturn)) {
log(...);
}
void handle_error(const std::optional<int> opt) {
if (!opt) {
fatal_error(); // Static analyzer doesn't know this never returns
}
*opt = 1; // False-positive `unchecked-optional-access` warning as analyzer thinks this is reachable
}Solution
CommentsThis PR incorporates part of the work done in #146355 Full diff: https://github.com/llvm/llvm-project/pull/150952.diff 5 Files Affected:
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..4911157828765 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,17 @@ void nullable_value_after_swap(BloombergLP::bdlb::NullableValue<int> &opt1, Bloo
}
}
+void assertion_handler() __attribute__((analyzer_noreturn));
+
+void function_calling_analyzer_noreturn(const bsl::optional<int>& opt)
+{
+ if (!opt) {
+ assertion_handler();
+ }
+
+ *opt; // no-warning: The previous condition guards this dereference.
+}
+
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 08fe1f881503b..d58920270083a 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2668,6 +2668,10 @@ class FunctionDecl : public DeclaratorDecl,
/// an attribute on its declaration or its type.
bool isNoReturn() const;
+ /// Determines whether this function is known to be 'noreturn' for analyzer,
+ /// through an `analyzer_noreturn` attribute on its declaration.
+ 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 83fcd87aec2f8..3c0b55f3e3b68 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -3596,6 +3596,10 @@ bool FunctionDecl::isNoReturn() const {
return false;
}
+bool FunctionDecl::isAnalyzerNoReturn() const {
+ return 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..60a2d113c08e2 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -2833,7 +2833,8 @@ CFGBlock *CFGBuilder::VisitCallExpr(CallExpr *C, AddStmtChoice asc) {
if (!FD->isVariadic())
findConstructionContextsForArguments(C);
- if (FD->isNoReturn() || C->isBuiltinAssumeFalse(*Context))
+ if (FD->isNoReturn() || FD->isAnalyzerNoReturn() ||
+ C->isBuiltinAssumeFalse(*Context))
NoReturn = true;
if (FD->hasAttr<NoThrowAttr>())
AddEHEdge = false;
diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index 9fb7bebdbe41e..d1dd4ff3ea33e 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -693,6 +693,80 @@ 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 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, IsEmpty());
+}
+
+TEST_F(AnalyzerNoreturnTest, CanonicalDeclCallCheck) {
+ std::string Code = R"(
+ #include "noreturn_test_defs.h"
+
+ extern void assertionHandler();
+
+ void target() {
+ assertionHandler();
+ trap();
+ // [[p]]
+ }
+ )";
+ runDataflow(Code, IsEmpty());
+}
+
// Models an analysis that uses flow conditions.
class SpecialBoolAnalysis final
: public DataflowAnalysis<SpecialBoolAnalysis, NoopLattice> {
|
|
@llvm/pr-subscribers-clang-tidy Author: Andrey Karlov (negativ) ChangesProblemCurrently, functions with void assertion_handler() __attribute__((analyzer_noreturn)) {
log(...);
}
void handle_error(const std::optional<int> opt) {
if (!opt) {
fatal_error(); // Static analyzer doesn't know this never returns
}
*opt = 1; // False-positive `unchecked-optional-access` warning as analyzer thinks this is reachable
}Solution
CommentsThis PR incorporates part of the work done in #146355 Full diff: https://github.com/llvm/llvm-project/pull/150952.diff 5 Files Affected:
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..4911157828765 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,17 @@ void nullable_value_after_swap(BloombergLP::bdlb::NullableValue<int> &opt1, Bloo
}
}
+void assertion_handler() __attribute__((analyzer_noreturn));
+
+void function_calling_analyzer_noreturn(const bsl::optional<int>& opt)
+{
+ if (!opt) {
+ assertion_handler();
+ }
+
+ *opt; // no-warning: The previous condition guards this dereference.
+}
+
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 08fe1f881503b..d58920270083a 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2668,6 +2668,10 @@ class FunctionDecl : public DeclaratorDecl,
/// an attribute on its declaration or its type.
bool isNoReturn() const;
+ /// Determines whether this function is known to be 'noreturn' for analyzer,
+ /// through an `analyzer_noreturn` attribute on its declaration.
+ 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 83fcd87aec2f8..3c0b55f3e3b68 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -3596,6 +3596,10 @@ bool FunctionDecl::isNoReturn() const {
return false;
}
+bool FunctionDecl::isAnalyzerNoReturn() const {
+ return 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..60a2d113c08e2 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -2833,7 +2833,8 @@ CFGBlock *CFGBuilder::VisitCallExpr(CallExpr *C, AddStmtChoice asc) {
if (!FD->isVariadic())
findConstructionContextsForArguments(C);
- if (FD->isNoReturn() || C->isBuiltinAssumeFalse(*Context))
+ if (FD->isNoReturn() || FD->isAnalyzerNoReturn() ||
+ C->isBuiltinAssumeFalse(*Context))
NoReturn = true;
if (FD->hasAttr<NoThrowAttr>())
AddEHEdge = false;
diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index 9fb7bebdbe41e..d1dd4ff3ea33e 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -693,6 +693,80 @@ 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 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, IsEmpty());
+}
+
+TEST_F(AnalyzerNoreturnTest, CanonicalDeclCallCheck) {
+ std::string Code = R"(
+ #include "noreturn_test_defs.h"
+
+ extern void assertionHandler();
+
+ void target() {
+ assertionHandler();
+ trap();
+ // [[p]]
+ }
+ )";
+ runDataflow(Code, IsEmpty());
+}
+
// Models an analysis that uses flow conditions.
class SpecialBoolAnalysis final
: public DataflowAnalysis<SpecialBoolAnalysis, NoopLattice> {
|
Xazax-hun
approved these changes
Jul 29, 2025
| @@ -2668,6 +2668,10 @@ class FunctionDecl : public DeclaratorDecl, | |||
| /// an attribute on its declaration or its type. | |||
| bool isNoReturn() const; | |||
|
|
|||
| /// Determines whether this function is known to be 'noreturn' for analyzer, | |||
| /// through an `analyzer_noreturn` attribute on its declaration. | |||
| bool isAnalyzerNoReturn() const; | |||
There was a problem hiding this comment.
Since this function is a very trivial check, I wonder if we need it or we could just inline its content at the use site. The function above isNoReturn is there because we have more cases to check. I don't have a strong feeling about this, I am fine either way.
There was a problem hiding this comment.
The initial implementation of isAnalyzerNoReturn() was a bit more complex because it also invoked isNoReturn(). Subsequently, I moved away from this approach and decided to simplify the function, but I kept the function as an excellent entry point for potential future enhancements.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Currently, functions with
analyzer_noreturnattribute aren't recognized asno-returnbyCFG:Solution
FunctionDeclclass by adding anisAnalyzerNoReturn()functionCFGBuilder::VisitCallExprto check bothFD->isNoReturn()andFD->isAnalyzerNoReturn()propertiesComments
This PR incorporates part of the work done in #146355