Skip to content

[clang]: Support analyzer_noreturn attribute in CFG #150952

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions clang/include/clang/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


/// True if the function was a definition but its body was skipped.
bool hasSkippedBody() const { return FunctionDeclBits.HasSkippedBody; }
void setHasSkippedBody(bool Skipped = true) {
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Analysis/CFG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down