Author: Florian Mayer Date: 2025-10-27T09:27:19-07:00 New Revision: 430d0edb521c33e6bf6e38cd1b7a49b173ef18e7
URL: https://github.com/llvm/llvm-project/commit/430d0edb521c33e6bf6e38cd1b7a49b173ef18e7 DIFF: https://github.com/llvm/llvm-project/commit/430d0edb521c33e6bf6e38cd1b7a49b173ef18e7.diff LOG: [FlowSensitive] [StatusOr] [8/N] Support value ctor and assignment Reviewers: jvoung, Xazax-hun Reviewed By: jvoung Pull Request: https://github.com/llvm/llvm-project/pull/163894 Added: Modified: clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp index 90551c22e0734..c6a680d8cf252 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp @@ -177,6 +177,31 @@ static auto isPointerComparisonOperatorCall(std::string operator_name) { pointee(anyOf(statusOrType(), statusType()))))))); } +// The nullPointerConstant in the two matchers below is to support +// absl::StatusOr<void*> X = nullptr. +// nullptr does not match the bound type. +// TODO: be less restrictive around convertible types in general. +static auto isStatusOrValueAssignmentCall() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxOperatorCallExpr( + hasOverloadedOperatorName("="), + callee(cxxMethodDecl(ofClass(statusOrClass()))), + hasArgument(1, anyOf(hasType(hasUnqualifiedDesugaredType( + type(equalsBoundNode("T")))), + nullPointerConstant()))); +} + +static auto isStatusOrValueConstructor() { + using namespace ::clang::ast_matchers; // NOLINT: Too many names + return cxxConstructExpr( + hasType(statusOrType()), + hasArgument(0, + anyOf(hasType(hasCanonicalType(type(equalsBoundNode("T")))), + nullPointerConstant(), + hasType(namedDecl(hasAnyName("absl::in_place_t", + "std::in_place_t")))))); +} + static auto buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) { return CFGMatchSwitchBuilder<const Environment, @@ -528,6 +553,27 @@ static void transferEmplaceCall(const CXXMemberCallExpr *Expr, State.Env.assume(OkVal.formula()); } +static void transferValueAssignmentCall(const CXXOperatorCallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + assert(Expr->getNumArgs() > 1); + + auto *StatusOrLoc = State.Env.get<RecordStorageLocation>(*Expr->getArg(0)); + if (StatusOrLoc == nullptr) + return; + + auto &OkVal = initializeStatusOr(*StatusOrLoc, State.Env); + State.Env.assume(OkVal.formula()); +} + +static void transferValueConstructor(const CXXConstructExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + auto &OkVal = + initializeStatusOr(State.Env.getResultObjectLocation(*Expr), State.Env); + State.Env.assume(OkVal.formula()); +} + CFGMatchSwitch<LatticeTransferState> buildTransferMatchSwitch(ASTContext &Ctx, CFGMatchSwitchBuilder<LatticeTransferState> Builder) { @@ -573,6 +619,10 @@ buildTransferMatchSwitch(ASTContext &Ctx, .CaseOfCFGStmt<CallExpr>(isNotOkStatusCall(), transferNotOkStatusCall) .CaseOfCFGStmt<CXXMemberCallExpr>(isStatusOrMemberCallWithName("emplace"), transferEmplaceCall) + .CaseOfCFGStmt<CXXOperatorCallExpr>(isStatusOrValueAssignmentCall(), + transferValueAssignmentCall) + .CaseOfCFGStmt<CXXConstructExpr>(isStatusOrValueConstructor(), + transferValueConstructor) .Build(); } diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp index 425beb939a42a..452062587ce72 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp @@ -2975,6 +2975,178 @@ TEST_P(UncheckedStatusOrAccessModelTest, Emplace) { )cc"); } +TEST_P(UncheckedStatusOrAccessModelTest, ValueConstruction) { + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_BOOL result = false; + result.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_INT result = 21; + result.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_INT result = Make<STATUSOR_INT>(); + result.value(); // [[unsafe]] + } + )cc"); + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_BOOL result = false; + if (result.ok()) + result.value(); + else + result.value(); + } + )cc"); + + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_BOOL result(false); + result.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_INT result(21); + result.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_INT result(Make<STATUSOR_INT>()); + result.value(); // [[unsafe]] + } + )cc"); + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_BOOL result(false); + if (result.ok()) + result.value(); + else + result.value(); + } + )cc"); +} + +TEST_P(UncheckedStatusOrAccessModelTest, ValueAssignment) { + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_BOOL result; + result = false; + result.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_INT result; + result = 21; + result.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_INT result; + result = Make<STATUSOR_INT>(); + result.value(); // [[unsafe]] + } + )cc"); + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_BOOL result; + result = false; + if (result.ok()) + result.value(); + else + result.value(); + } + )cc"); +} + +TEST_P(UncheckedStatusOrAccessModelTest, NestedStatusOr) { + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + absl::StatusOr<STATUSOR_INT> result; + result = Make<STATUSOR_INT>(); + result.value(); + } + )cc"); + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + absl::StatusOr<STATUSOR_INT> result = Make<STATUSOR_INT>(); + result.value(); + } + )cc"); +} + +TEST_P(UncheckedStatusOrAccessModelTest, PtrConstruct) { + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_VOIDPTR sor = nullptr; + *sor; + } + )cc"); + + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_VOIDPTR sor(nullptr); + *sor; + } + )cc"); +} + +TEST_P(UncheckedStatusOrAccessModelTest, InPlaceConstruct) { + ExpectDiagnosticsFor(R"cc( +#include "unchecked_statusor_access_test_defs.h" + + void target() { + STATUSOR_VOIDPTR absl_sor(absl::in_place, {nullptr}); + *absl_sor; + STATUSOR_VOIDPTR std_sor(std::in_place, {nullptr}); + *std_sor; + } + )cc"); +} + } // namespace std::string _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
