diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -255,12 +255,22 @@ /// Creates a symbolic value for an `optional` value using `HasValueVal` as the /// symbolic value of its "has_value" property. -StructValue &createOptionalValue(Environment &Env, BoolValue &HasValueVal) { +StructValue &createOptionalValue(BoolValue &HasValueVal, Environment &Env) { auto &OptionalVal = Env.create(); setHasValue(OptionalVal, HasValueVal); return OptionalVal; } +/// Creates a symbolic value for an `optional` value at an existing storage +/// location. Uses `HasValueVal` as the symbolic value of the "has_value" +/// property. +StructValue &createOptionalValue(AggregateStorageLocation &Loc, + BoolValue &HasValueVal, Environment &Env) { + StructValue &OptionalVal = createOptionalValue(HasValueVal, Env); + Env.setValue(Loc, OptionalVal); + return OptionalVal; +} + /// Returns the symbolic value that represents the "has_value" property of the /// optional value `OptionalVal`. Returns null if `OptionalVal` is null. BoolValue *getHasValue(Environment &Env, Value *OptionalVal) { @@ -422,15 +432,6 @@ return HasValueVal != nullptr && Env.flowConditionImplies(*HasValueVal); } -StorageLocation *maybeSkipPointer(StorageLocation *Loc, - const Environment &Env) { - if (Loc == nullptr) - return nullptr; - if (auto *Val = dyn_cast_or_null(Env.getValue(*Loc))) - return &Val->getPointeeLoc(); - return Loc; -} - Value *getValueBehindPossiblePointer(const Expr &E, const Environment &Env) { Value *Val = Env.getValue(E, SkipPast::Reference); if (auto *PointerVal = dyn_cast_or_null(Val)) @@ -467,7 +468,7 @@ auto &Loc = State.Env.createStorageLocation(*E); State.Env.setStorageLocation(*E, Loc); State.Env.setValue( - Loc, createOptionalValue(State.Env, State.Env.getBoolLiteralValue(true))); + Loc, createOptionalValue(State.Env.getBoolLiteralValue(true), State.Env)); } void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr, @@ -544,15 +545,12 @@ auto &Loc = State.Env.createStorageLocation(*E); State.Env.setStorageLocation(*E, Loc); State.Env.setValue( - Loc, createOptionalValue(State.Env, State.Env.makeAtomicBoolValue())); + Loc, createOptionalValue(State.Env.makeAtomicBoolValue(), State.Env)); } -void assignOptionalValue(const Expr &E, Environment &Env, - BoolValue &HasValueVal) { - if (auto *OptionalLoc = maybeSkipPointer( - Env.getStorageLocation(E, SkipPast::Reference), Env)) { - Env.setValue(*OptionalLoc, createOptionalValue(Env, HasValueVal)); - } +void constructOptionalValue(const Expr &E, Environment &Env, + BoolValue &HasValueVal) { + Env.setValueStrict(E, createOptionalValue(HasValueVal, Env)); } /// Returns a symbolic value for the "has_value" property of an `optional` @@ -590,25 +588,23 @@ LatticeTransferState &State) { assert(E->getNumArgs() > 0); - assignOptionalValue(*E, State.Env, - valueOrConversionHasValue(*E->getConstructor(), - *E->getArg(0), MatchRes, - State)); + constructOptionalValue(*E, State.Env, + valueOrConversionHasValue(*E->getConstructor(), + *E->getArg(0), MatchRes, + State)); } void transferAssignment(const CXXOperatorCallExpr *E, BoolValue &HasValueVal, LatticeTransferState &State) { assert(E->getNumArgs() > 0); - auto *OptionalLoc = - State.Env.getStorageLocation(*E->getArg(0), SkipPast::Reference); - if (OptionalLoc == nullptr) - return; - - State.Env.setValue(*OptionalLoc, createOptionalValue(State.Env, HasValueVal)); + if (auto *Loc = cast( + State.Env.getStorageLocationStrict(*E->getArg(0)))) { + createOptionalValue(*Loc, HasValueVal, State.Env); - // Assign a storage location for the whole expression. - State.Env.setStorageLocation(*E, *OptionalLoc); + // Assign a storage location for the whole expression. + State.Env.setStorageLocationStrict(*E, *Loc); + } } void transferValueOrConversionAssignment( @@ -627,19 +623,19 @@ transferAssignment(E, State.Env.getBoolLiteralValue(false), State); } -void transferSwap(StorageLocation *Loc1, StorageLocation *Loc2, - Environment &Env) { +void transferSwap(AggregateStorageLocation *Loc1, + AggregateStorageLocation *Loc2, Environment &Env) { // We account for cases where one or both of the optionals are not modeled, // either lacking associated storage locations, or lacking values associated // to such storage locations. if (Loc1 == nullptr) { if (Loc2 != nullptr) - Env.setValue(*Loc2, createOptionalValue(Env, Env.makeAtomicBoolValue())); + createOptionalValue(*Loc2, Env.makeAtomicBoolValue(), Env); return; } if (Loc2 == nullptr) { - Env.setValue(*Loc1, createOptionalValue(Env, Env.makeAtomicBoolValue())); + createOptionalValue(*Loc1, Env.makeAtomicBoolValue(), Env); return; } @@ -649,36 +645,35 @@ // allows for local reasoning about the value. To avoid the above, we would // need *lazy* value allocation. // FIXME: allocate values lazily, instead of just creating a fresh value. - auto *Val1 = Env.getValue(*Loc1); - if (Val1 == nullptr) - Val1 = &createOptionalValue(Env, Env.makeAtomicBoolValue()); + BoolValue *BoolVal1 = getHasValue(Env, Env.getValue(*Loc1)); + if (BoolVal1 == nullptr) + BoolVal1 = &Env.makeAtomicBoolValue(); - auto *Val2 = Env.getValue(*Loc2); - if (Val2 == nullptr) - Val2 = &createOptionalValue(Env, Env.makeAtomicBoolValue()); + BoolValue *BoolVal2 = getHasValue(Env, Env.getValue(*Loc2)); + if (BoolVal2 == nullptr) + BoolVal2 = &Env.makeAtomicBoolValue(); - Env.setValue(*Loc1, *Val2); - Env.setValue(*Loc2, *Val1); + createOptionalValue(*Loc1, *BoolVal2, Env); + createOptionalValue(*Loc2, *BoolVal1, Env); } void transferSwapCall(const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { assert(E->getNumArgs() == 1); - transferSwap(maybeSkipPointer( - State.Env.getStorageLocation(*E->getImplicitObjectArgument(), - SkipPast::Reference), - State.Env), - State.Env.getStorageLocation(*E->getArg(0), SkipPast::Reference), - State.Env); + auto *OtherLoc = cast_or_null( + State.Env.getStorageLocationStrict(*E->getArg(0))); + transferSwap(getImplicitObjectLocation(*E, State.Env), OtherLoc, State.Env); } void transferStdSwapCall(const CallExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { assert(E->getNumArgs() == 2); - transferSwap(State.Env.getStorageLocation(*E->getArg(0), SkipPast::Reference), - State.Env.getStorageLocation(*E->getArg(1), SkipPast::Reference), - State.Env); + auto *Arg0Loc = cast_or_null( + State.Env.getStorageLocationStrict(*E->getArg(0))); + auto *Arg1Loc = cast_or_null( + State.Env.getStorageLocationStrict(*E->getArg(1))); + transferSwap(Arg0Loc, Arg1Loc, State.Env); } void transferStdForwardCall(const CallExpr *E, const MatchFinder::MatchResult &, @@ -697,7 +692,7 @@ Value *ValArg = State.Env.getValue(*LocArg); if (ValArg == nullptr) - ValArg = &createOptionalValue(State.Env, State.Env.makeAtomicBoolValue()); + ValArg = &createOptionalValue(State.Env.makeAtomicBoolValue(), State.Env); // Create a new storage location LocRet = &State.Env.createStorageLocation(*E); @@ -798,24 +793,24 @@ isOptionalInPlaceConstructor(), [](const CXXConstructExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { - assignOptionalValue(*E, State.Env, - State.Env.getBoolLiteralValue(true)); + constructOptionalValue(*E, State.Env, + State.Env.getBoolLiteralValue(true)); }) // nullopt_t::nullopt_t .CaseOfCFGStmt( isNulloptConstructor(), [](const CXXConstructExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { - assignOptionalValue(*E, State.Env, - State.Env.getBoolLiteralValue(false)); + constructOptionalValue(*E, State.Env, + State.Env.getBoolLiteralValue(false)); }) // optional::optional(nullopt_t) .CaseOfCFGStmt( isOptionalNulloptConstructor(), [](const CXXConstructExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { - assignOptionalValue(*E, State.Env, - State.Env.getBoolLiteralValue(false)); + constructOptionalValue(*E, State.Env, + State.Env.getBoolLiteralValue(false)); }) // optional::optional (value/conversion) .CaseOfCFGStmt(isOptionalValueOrConversionConstructor(), @@ -867,8 +862,11 @@ isOptionalMemberCallWithName("emplace"), [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { - assignOptionalValue(*E->getImplicitObjectArgument(), State.Env, - State.Env.getBoolLiteralValue(true)); + if (AggregateStorageLocation *Loc = + getImplicitObjectLocation(*E, State.Env)) { + createOptionalValue(*Loc, State.Env.getBoolLiteralValue(true), + State.Env); + } }) // optional::reset @@ -876,8 +874,11 @@ isOptionalMemberCallWithName("reset"), [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { - assignOptionalValue(*E->getImplicitObjectArgument(), State.Env, - State.Env.getBoolLiteralValue(false)); + if (AggregateStorageLocation *Loc = + getImplicitObjectLocation(*E, State.Env)) { + createOptionalValue(*Loc, State.Env.getBoolLiteralValue(false), + State.Env); + } }) // optional::swap @@ -1043,7 +1044,7 @@ if (isa(CurrentHasVal)) return &Current; } - return &createOptionalValue(CurrentEnv, CurrentEnv.makeTopBoolValue()); + return &createOptionalValue(CurrentEnv.makeTopBoolValue(), CurrentEnv); case ComparisonResult::Unknown: return nullptr; }