llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Andrey Karlov (negativ)
<details>
<summary>Changes</summary>
## Problem
Currently, functions with `analyzer_noreturn` attribute aren't recognized as
`no-return` by `CFG`:
```cpp
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
1. Extend the `FunctionDecl` class by adding an `isAnalyzerNoReturn()` function
2. Update `CFGBuilder::VisitCallExpr` to check both `FD->isNoReturn()` and
`FD->isAnalyzerNoReturn()` properties
## Comments
This PR incorporates part of the work done in
https://github.com/llvm/llvm-project/pull/146355
---
Full diff: https://github.com/llvm/llvm-project/pull/150952.diff
5 Files Affected:
- (modified)
clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
(+11)
- (modified) clang/include/clang/AST/Decl.h (+4)
- (modified) clang/lib/AST/Decl.cpp (+4)
- (modified) clang/lib/Analysis/CFG.cpp (+2-1)
- (modified)
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp (+74)
``````````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..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> {
``````````
</details>
https://github.com/llvm/llvm-project/pull/150952
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
