Skip to content

Commit

Permalink
[clang][dataflow] Various refactorings to UncheckedOptionalAccessModel.
Browse files Browse the repository at this point in the history 
These are intended to ease an upcoming change that will eliminate the duplication between `AggregateStorageLocation` and `StructValue` (see https://discourse.llvm.org/t/70086 for details), but many of the changes also have value in their own right.

Depends On D154586

Reviewed By: ymandel, gribozavr2

Differential Revision: https://reviews.llvm.org/D154597
  • Loading branch information
@martinboehme
martinboehme committed Jul 10, 2023
1 parent e8a1560 commit f653d14
Showing 1 changed file with 64 additions and 63 deletions.
127 changes: 64 additions & 63 deletions clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,12 +255,22 @@ void setHasValue(Value &OptionalVal, BoolValue &HasValueVal) {

/// 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<StructValue>();
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) {
Expand Down Expand Up @@ -422,15 +432,6 @@ bool isNonEmptyOptional(const Value &OptionalVal, const Environment &Env) {
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<PointerValue>(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<PointerValue>(Val))
Expand Down Expand Up @@ -467,7 +468,7 @@ void transferMakeOptionalCall(const CallExpr *E,
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,
Expand Down Expand Up @@ -544,15 +545,12 @@ void transferCallReturningOptional(const CallExpr *E,
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<T>`
Expand Down Expand Up @@ -590,25 +588,23 @@ void transferValueOrConversionConstructor(
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<AggregateStorageLocation>(
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(
Expand All @@ -627,19 +623,19 @@ void transferNulloptAssignment(const CXXOperatorCallExpr *E,
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;
}

Expand All @@ -649,36 +645,35 @@ void transferSwap(StorageLocation *Loc1, StorageLocation *Loc2,
// 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<AggregateStorageLocation>(
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<AggregateStorageLocation>(
State.Env.getStorageLocationStrict(*E->getArg(0)));
auto *Arg1Loc = cast_or_null<AggregateStorageLocation>(
State.Env.getStorageLocationStrict(*E->getArg(1)));
transferSwap(Arg0Loc, Arg1Loc, State.Env);
}

void transferStdForwardCall(const CallExpr *E, const MatchFinder::MatchResult &,
Expand All @@ -697,7 +692,7 @@ void transferStdForwardCall(const CallExpr *E, const MatchFinder::MatchResult &,

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);
Expand Down Expand Up @@ -798,24 +793,24 @@ auto buildTransferMatchSwitch() {
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<CXXConstructExpr>(
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<CXXConstructExpr>(
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<CXXConstructExpr>(isOptionalValueOrConversionConstructor(),
Expand Down Expand Up @@ -867,17 +862,23 @@ auto buildTransferMatchSwitch() {
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
.CaseOfCFGStmt<CXXMemberCallExpr>(
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
Expand Down Expand Up @@ -1043,7 +1044,7 @@ Value *UncheckedOptionalAccessModel::widen(QualType Type, Value &Prev,
if (isa<TopBoolValue>(CurrentHasVal))
return &Current;
}
return &createOptionalValue(CurrentEnv, CurrentEnv.makeTopBoolValue());
return &createOptionalValue(CurrentEnv.makeTopBoolValue(), CurrentEnv);
case ComparisonResult::Unknown:
return nullptr;
}
Expand Down

0 comments on commit f653d14

Please sign in to comment.