llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Florian Mayer (fmayer) <details> <summary>Changes</summary> --- Full diff: https://github.com/llvm/llvm-project/pull/170950.diff 2 Files Affected: - (modified) clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp (+24) - (modified) clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp (+149) ``````````diff diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp index 1b68d704239e8..c917c8e8c11ba 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp @@ -1037,6 +1037,26 @@ transferAssertionResultOperatorBoolCall(const CXXMemberCallExpr *Expr, State.Env.setValue(*Expr, Res); } +static void transferDerefCall(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + auto *StatusOrLoc = State.Env.get<RecordStorageLocation>(*Expr->getArg(0)); + + if (StatusOrLoc && State.Env.getStorageLocation(*Expr) == nullptr) + State.Env.setStorageLocation(*Expr, + StatusOrLoc->getSyntheticField("value")); +} + +static void transferArrowCall(const CallExpr *Expr, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + auto *StatusOrLoc = State.Env.get<RecordStorageLocation>(*Expr->getArg(0)); + if (!StatusOrLoc) + return; + State.Env.setValue(*Expr, State.Env.create<PointerValue>( + StatusOrLoc->getSyntheticField("value"))); +} + static RecordStorageLocation * getSmartPtrLikeStorageLocation(const Expr &E, const Environment &Env) { if (!E.isPRValue()) @@ -1123,6 +1143,10 @@ buildTransferMatchSwitch(ASTContext &Ctx, transferValueAssignmentCall) .CaseOfCFGStmt<CXXConstructExpr>(isStatusOrValueConstructor(), transferValueConstructor) + .CaseOfCFGStmt<CallExpr>(isStatusOrOperatorCallWithName("->"), + transferArrowCall) + .CaseOfCFGStmt<CallExpr>(isStatusOrOperatorCallWithName("*"), + transferDerefCall) .CaseOfCFGStmt<CallExpr>(isAsStatusCallWithStatus(), transferAsStatusCallWithStatus) .CaseOfCFGStmt<CallExpr>(isAsStatusCallWithStatusOr(), diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp index 48e61abf09f19..cd7353c62f537 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp @@ -3691,6 +3691,155 @@ TEST_P(UncheckedStatusOrAccessModelTest, UniquePtrReset) { )cc"); } +TEST_P(UncheckedStatusOrAccessModelTest, NestedStatusOrInStatusOrStruct) { + // Non-standard assignment with a nested StatusOr. + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Inner { + absl::StatusOr<std::string> sor; + }; + + struct Outer { + absl::StatusOr<Inner> inner; + }; + + void target() { + Outer foo = Make<Outer>(); + foo.inner->sor = "a"; // [[unsafe]] + } + )cc"); + + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr<std::string> sor; + }; + + void target(const absl::StatusOr<Foo>& foo) { + if (foo.ok() && foo->sor.ok()) foo->sor.value(); + } + )cc"); + + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr<std::string> sor; + }; + + void target(const absl::StatusOr<Foo>& foo) { + if (foo.ok() && (*foo).sor.ok()) (*foo).sor.value(); + } + )cc"); + + // With assignment. + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr<std::string> sor; + }; + + void target(absl::StatusOr<Foo>& foo) { + if (foo.ok() && foo->sor.ok()) { + foo->sor = Make<absl::StatusOr<std::string>>(); + foo->sor.value(); // [[unsafe]] + } + } + )cc"); + + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr<std::string> sor; + }; + + void target(absl::StatusOr<Foo>& foo) { + if (foo.ok() && foo->sor.ok()) { + auto n = Make<absl::StatusOr<std::string>>(); + if (n.ok()) foo->sor = n; + foo->sor.value(); + } + } + )cc"); + + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr<std::string> sor; + }; + + void target(absl::StatusOr<Foo>& foo) { + if (foo.ok() && foo->sor.ok()) { + auto n = Make<absl::StatusOr<std::string>>(); + if (n.ok()) foo->sor = std::move(n); + foo->sor.value(); + } + } + )cc"); + + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr<std::string> sor; + }; + + void target(absl::StatusOr<Foo>& foo) { + if (foo.ok() && foo->sor.ok()) *foo->sor; + } + )cc"); + + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr<std::string> sor; + }; + + void target(absl::StatusOr<Foo>& foo) { + if (!foo.ok()) return; + if (!foo->sor.ok()) + foo->sor.value(); // [[unsafe]] + else + foo->sor.value(); + } + )cc"); + + ExpectDiagnosticsFor( + R"cc( +#include "unchecked_statusor_access_test_defs.h" + + struct Foo { + absl::StatusOr<std::string> sor; + }; + + void target(absl::StatusOr<Foo>& foo, bool b) { + if (!foo.ok()) return; + if (b) { + if (!foo->sor.ok()) return; + foo->sor.value(); + } else { + if (!foo->sor.ok()) return; + foo->sor.value(); + } + foo->sor.value(); + } + )cc"); +} + } // namespace std::string `````````` </details> https://github.com/llvm/llvm-project/pull/170950 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
