https://github.com/fmayer created https://github.com/llvm/llvm-project/pull/169749
None >From 17d207ac278b8efcc25874988575cb5e9c3ee7d6 Mon Sep 17 00:00:00 2001 From: Florian Mayer <[email protected]> Date: Wed, 26 Nov 2025 16:09:04 -0800 Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?= =?UTF-8?q?l=20version?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.7 --- .../Models/UncheckedStatusOrAccessModel.cpp | 102 ++++++++++++++++++ ...ncheckedStatusOrAccessModelTestFixture.cpp | 84 +++++++++++++++ 2 files changed, 186 insertions(+) diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp index b42bfa3821c2e..0a1adde7bfe9d 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp @@ -211,6 +211,31 @@ static auto isStatusConstructor() { using namespace ::clang::ast_matchers; // NOLINT: Too many names return cxxConstructExpr(hasType(statusType())); } +static auto isLoggingGetReferenceableValueCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return callExpr(callee( + functionDecl(hasName("::absl::log_internal::GetReferenceableValue")))); +} + +static auto isLoggingCheckEqImpl() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return callExpr( + callee(functionDecl(hasName("::absl::log_internal::Check_EQImpl")))); +} + +static auto isAsStatusCallWithStatus() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return callExpr( + callee(functionDecl(hasName("::absl::log_internal::AsStatus"))), + hasArgument(0, hasType(statusClass()))); +} + +static auto isAsStatusCallWithStatusOr() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return callExpr( + callee(functionDecl(hasName("::absl::log_internal::AsStatus"))), + hasArgument(0, hasType(statusOrType()))); +} static auto buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) { @@ -602,6 +627,67 @@ static void transferStatusConstructor(const CXXConstructExpr *Expr, if (State.Env.getValue(locForOk(StatusLoc)) == nullptr) initializeStatus(StatusLoc, State.Env); } +static void +transferLoggingGetReferenceableValueCall(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assert(Expr->getNumArgs() == 1); + if (Expr->getArg(0)->isPRValue()) + return; + auto *ArgLoc = State.Env.getStorageLocation(*Expr->getArg(0)); + if (ArgLoc == nullptr) + return; + + State.Env.setStorageLocation(*Expr, *ArgLoc); +} + +static void transferLoggingCheckEqImpl(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assert(Expr->getNumArgs() > 2); + + auto *EqVal = evaluateEquality(Expr->getArg(0), Expr->getArg(1), State.Env); + if (EqVal == nullptr) + return; + + // TODO(sgatev): Model pointer nullability more accurately instead of + // assigning BoolValue as the value of an expression of pointer type. + State.Env.setValue(*Expr, State.Env.makeNot(*EqVal)); +} + +static void transferAsStatusCallWithStatus(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assert(Expr->getNumArgs() == 1); + + auto *ArgLoc = State.Env.get<RecordStorageLocation>(*Expr->getArg(0)); + if (ArgLoc == nullptr) + return; + + if (State.Env.getValue(locForOk(*ArgLoc)) == nullptr) + initializeStatus(*ArgLoc, State.Env); + + auto &ExprVal = State.Env.create<PointerValue>(*ArgLoc); + State.Env.setValue(*Expr, ExprVal); +} + +static void transferAsStatusCallWithStatusOr(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assert(Expr->getNumArgs() == 1); + + auto *ArgLoc = State.Env.get<RecordStorageLocation>(*Expr->getArg(0)); + if (ArgLoc == nullptr) + return; + + RecordStorageLocation &StatusLoc = locForStatus(*ArgLoc); + + if (State.Env.getValue(locForOk(StatusLoc)) == nullptr) + initializeStatusOr(*ArgLoc, State.Env); + + auto &ExprVal = State.Env.create<PointerValue>(StatusLoc); + State.Env.setValue(*Expr, ExprVal); +} CFGMatchSwitch<LatticeTransferState> buildTransferMatchSwitch(ASTContext &Ctx, @@ -652,6 +738,14 @@ buildTransferMatchSwitch(ASTContext &Ctx, transferValueAssignmentCall) .CaseOfCFGStmt<CXXConstructExpr>(isStatusOrValueConstructor(), transferValueConstructor) + .CaseOfCFGStmt<CallExpr>(isAsStatusCallWithStatus(), + transferAsStatusCallWithStatus) + .CaseOfCFGStmt<CallExpr>(isAsStatusCallWithStatusOr(), + transferAsStatusCallWithStatusOr) + .CaseOfCFGStmt<CallExpr>(isLoggingGetReferenceableValueCall(), + transferLoggingGetReferenceableValueCall) + .CaseOfCFGStmt<CallExpr>(isLoggingCheckEqImpl(), + transferLoggingCheckEqImpl) // N.B. These need to come after all other CXXConstructExpr. // These are there to make sure that every Status and StatusOr object // have their ok boolean initialized when constructed. If we were to @@ -662,6 +756,14 @@ buildTransferMatchSwitch(ASTContext &Ctx, transferStatusOrConstructor) .CaseOfCFGStmt<CXXConstructExpr>(isStatusConstructor(), transferStatusConstructor) + .CaseOfCFGStmt<ImplicitCastExpr>( + implicitCastExpr(hasCastKind(CK_PointerToBoolean)), + [](const ImplicitCastExpr *Expr, const MatchFinder::MatchResult &, + LatticeTransferState &State) { + if (auto *SubExprVal = dyn_cast_or_null<BoolValue>( + State.Env.getValue(*Expr->getSubExpr()))) + State.Env.setValue(*Expr, *SubExprVal); + }) .Build(); } diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp index 5635ff4e01d36..13cde05df0a3f 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp @@ -1725,6 +1725,90 @@ TEST_P(UncheckedStatusOrAccessModelTest, QcheckMacro) { )cc"); } +TEST_P(UncheckedStatusOrAccessModelTest, CheckOkMacro) { + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + CHECK_OK(sor.status()); + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + CHECK_OK(sor); + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUS s = Make<STATUS>(); + CHECK_OK(s); + } + )cc"); +} + +TEST_P(UncheckedStatusOrAccessModelTest, QcheckOkMacro) { + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + QCHECK_OK(sor.status()); + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + QCHECK_OK(sor); + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUS s = Make<STATUS>(); + QCHECK_OK(s); + } + )cc"); +} + +TEST_P(UncheckedStatusOrAccessModelTest, CheckEqMacro) { + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + CHECK_EQ(sor.status(), absl::OkStatus()); + sor.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + CHECK_EQ(Make<STATUS>(), absl::OkStatus()); + CHECK_EQ(absl::OkStatus(), Make<STATUS>()); + } + )cc"); +} + +TEST_P(UncheckedStatusOrAccessModelTest, QcheckEqMacro) { + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target(STATUSOR_INT sor) { + QCHECK_EQ(sor.status(), absl::OkStatus()); + sor.value(); + } + )cc"); +} + TEST_P(UncheckedStatusOrAccessModelTest, CheckNeMacro) { ExpectDiagnosticsFor(R"cc( #include "unchecked_statusor_access_test_defs.h" _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
