Skip to content

Commit

Permalink
[clang][dataflow] Use Strict accessors where we weren't using them …
Browse files Browse the repository at this point in the history 
…yet.

This eliminates all uses of the deprecated accessors.

Reviewed By: ymandel, xazax.hun

Differential Revision: https://reviews.llvm.org/D156672
  • Loading branch information
@martinboehme
martinboehme committed Jul 31, 2023
1 parent e0b3f45 commit f76f667
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 78 deletions.
26 changes: 10 additions & 16 deletions clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,8 @@ Environment Environment::pushCall(const CallExpr *Call) const {
if (const auto *MethodCall = dyn_cast<CXXMemberCallExpr>(Call)) {
if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) {
if (!isa<CXXThisExpr>(Arg))
Env.ThisPointeeLoc = cast<AggregateStorageLocation>(
getStorageLocation(*Arg, SkipPast::Reference));
Env.ThisPointeeLoc =
cast<AggregateStorageLocation>(getStorageLocationStrict(*Arg));
// Otherwise (when the argument is `this`), retain the current
// environment's `ThisPointeeLoc`.
}
Expand Down Expand Up @@ -832,9 +832,8 @@ StorageLocation &Environment::createObjectInternal(const VarDecl *D,
// can happen that we can't see the initializer, so `InitExpr` may still
// be null.
if (InitExpr) {
if (auto *InitExprLoc =
getStorageLocation(*InitExpr, SkipPast::Reference))
return *InitExprLoc;
if (auto *InitExprLoc = getStorageLocationStrict(*InitExpr))
return *InitExprLoc;
}

// Even though we have an initializer, we might not get an
Expand Down Expand Up @@ -913,32 +912,27 @@ getImplicitObjectLocation(const CXXMemberCallExpr &MCE,
Expr *ImplicitObject = MCE.getImplicitObjectArgument();
if (ImplicitObject == nullptr)
return nullptr;
StorageLocation *Loc =
Env.getStorageLocation(*ImplicitObject, SkipPast::Reference);
if (Loc == nullptr)
return nullptr;
if (ImplicitObject->getType()->isPointerType()) {
if (auto *Val = cast_or_null<PointerValue>(Env.getValue(*Loc)))
if (auto *Val = cast_or_null<PointerValue>(Env.getValue(*ImplicitObject)))
return &cast<AggregateStorageLocation>(Val->getPointeeLoc());
return nullptr;
}
return cast<AggregateStorageLocation>(Loc);
return cast_or_null<AggregateStorageLocation>(
Env.getStorageLocationStrict(*ImplicitObject));
}

AggregateStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
const Environment &Env) {
Expr *Base = ME.getBase();
if (Base == nullptr)
return nullptr;
StorageLocation *Loc = Env.getStorageLocation(*Base, SkipPast::Reference);
if (Loc == nullptr)
return nullptr;
if (ME.isArrow()) {
if (auto *Val = cast_or_null<PointerValue>(Env.getValue(*Loc)))
if (auto *Val = cast_or_null<PointerValue>(Env.getValue(*Base)))
return &cast<AggregateStorageLocation>(Val->getPointeeLoc());
return nullptr;
}
return cast<AggregateStorageLocation>(Loc);
return cast_or_null<AggregateStorageLocation>(
Env.getStorageLocationStrict(*Base));
}

std::vector<FieldDecl *> getFieldsForInitListExpr(const RecordDecl *RD) {
Expand Down
15 changes: 11 additions & 4 deletions clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,17 @@ class HTMLLogger : public Logger {
if (ElementIndex > 0) {
auto S =
Iters.back().first->Elements[ElementIndex - 1].getAs<CFGStmt>();
if (const Expr *E = S ? llvm::dyn_cast<Expr>(S->getStmt()) : nullptr)
if (auto *Loc = State.Env.getStorageLocation(*E, SkipPast::None))
JOS->attributeObject(
"value", [&] { ModelDumper(*JOS, State.Env).dump(*Loc); });
if (const Expr *E = S ? llvm::dyn_cast<Expr>(S->getStmt()) : nullptr) {
if (E->isPRValue()) {
if (auto *V = State.Env.getValue(*E))
JOS->attributeObject(
"value", [&] { ModelDumper(*JOS, State.Env).dump(*V); });
} else {
if (auto *Loc = State.Env.getStorageLocationStrict(*E))
JOS->attributeObject(
"value", [&] { ModelDumper(*JOS, State.Env).dump(*Loc); });
}
}
}
if (!ContextLogs.empty()) {
JOS->attribute("logs", ContextLogs);
Expand Down
23 changes: 12 additions & 11 deletions clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,8 @@ const Formula &forceBoolValue(Environment &Env, const Expr &Expr) {
if (Value != nullptr)
return Value->formula();

auto &Loc = Env.createStorageLocation(Expr);
Value = &Env.makeAtomicBoolValue();
Env.setValue(Loc, *Value);
Env.setStorageLocation(Expr, Loc);
Env.setValueStrict(Expr, *Value);
return Value->formula();
}

Expand Down Expand Up @@ -439,10 +437,10 @@ void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
LatticeTransferState &State) {
if (auto *OptionalVal =
getValueBehindPossiblePointer(*ObjectExpr, State.Env)) {
if (State.Env.getStorageLocation(*UnwrapExpr, SkipPast::None) == nullptr)
if (State.Env.getStorageLocationStrict(*UnwrapExpr) == nullptr)
if (auto *Loc = maybeInitializeOptionalValueMember(
UnwrapExpr->getType(), *OptionalVal, State.Env))
State.Env.setStorageLocation(*UnwrapExpr, *Loc);
State.Env.setStorageLocationStrict(*UnwrapExpr, *Loc);
}
}

Expand Down Expand Up @@ -471,9 +469,7 @@ void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr,
if (auto *HasValueVal = getHasValue(
State.Env, getValueBehindPossiblePointer(
*CallExpr->getImplicitObjectArgument(), State.Env))) {
auto &CallExprLoc = State.Env.createStorageLocation(*CallExpr);
State.Env.setValue(CallExprLoc, *HasValueVal);
State.Env.setStorageLocation(*CallExpr, CallExprLoc);
State.Env.setValueStrict(*CallExpr, *HasValueVal);
}
}

Expand Down Expand Up @@ -534,15 +530,20 @@ void transferValueOrNotEqX(const Expr *ComparisonExpr,
void transferCallReturningOptional(const CallExpr *E,
const MatchFinder::MatchResult &Result,
LatticeTransferState &State) {
if (State.Env.getStorageLocation(*E, SkipPast::None) != nullptr)
if (State.Env.getValue(*E) != nullptr)
return;

AggregateStorageLocation *Loc = nullptr;
if (E->isPRValue()) {
Loc = &State.Env.getResultObjectLocation(*E);
} else {
Loc = &cast<AggregateStorageLocation>(State.Env.createStorageLocation(*E));
State.Env.setStorageLocationStrict(*E, *Loc);
Loc = cast_or_null<AggregateStorageLocation>(
State.Env.getStorageLocationStrict(*E));
if (Loc == nullptr) {
Loc =
&cast<AggregateStorageLocation>(State.Env.createStorageLocation(*E));
State.Env.setStorageLocationStrict(*E, *Loc);
}
}

createOptionalValue(*Loc, State.Env.makeAtomicBoolValue(), State.Env);
Expand Down
74 changes: 27 additions & 47 deletions clang/lib/Analysis/FlowSensitive/Transfer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,30 +141,26 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
Env.setValue(*LHSLoc, *RHSVal);

// Assign a storage location for the whole expression.
Env.setStorageLocation(*S, *LHSLoc);
Env.setStorageLocationStrict(*S, *LHSLoc);
break;
}
case BO_LAnd:
case BO_LOr: {
auto &Loc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, Loc);

BoolValue &LHSVal = getLogicOperatorSubExprValue(*LHS);
BoolValue &RHSVal = getLogicOperatorSubExprValue(*RHS);

if (S->getOpcode() == BO_LAnd)
Env.setValue(Loc, Env.makeAnd(LHSVal, RHSVal));
Env.setValueStrict(*S, Env.makeAnd(LHSVal, RHSVal));
else
Env.setValue(Loc, Env.makeOr(LHSVal, RHSVal));
Env.setValueStrict(*S, Env.makeOr(LHSVal, RHSVal));
break;
}
case BO_NE:
case BO_EQ: {
auto &LHSEqRHSValue = evaluateBooleanEquality(*LHS, *RHS, Env);
auto &Loc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, Loc);
Env.setValue(Loc, S->getOpcode() == BO_EQ ? LHSEqRHSValue
: Env.makeNot(LHSEqRHSValue));
Env.setValueStrict(*S, S->getOpcode() == BO_EQ
? LHSEqRHSValue
: Env.makeNot(LHSEqRHSValue));
break;
}
case BO_Comma: {
Expand Down Expand Up @@ -238,7 +234,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
VisitDeclRefExpr(DE);
VisitMemberExpr(ME);

if (auto *Loc = Env.getStorageLocation(*ME, SkipPast::Reference))
if (auto *Loc = Env.getStorageLocationStrict(*ME))
Env.setStorageLocation(*B, *Loc);
} else if (auto *VD = B->getHoldingVar()) {
// Holding vars are used to back the `BindingDecl`s of tuple-like
Expand Down Expand Up @@ -283,9 +279,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
if (SubExprVal == nullptr)
break;

auto &ExprLoc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, ExprLoc);
Env.setValue(ExprLoc, *SubExprVal);
Env.setValueStrict(*S, *SubExprVal);
break;
}

Expand All @@ -308,28 +302,21 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
break;
}
case CK_NullToPointer: {
auto &Loc = Env.createStorageLocation(S->getType());
Env.setStorageLocation(*S, Loc);

auto &NullPointerVal =
Env.getOrCreateNullPointerValue(S->getType()->getPointeeType());
Env.setValue(Loc, NullPointerVal);
Env.setValueStrict(*S, NullPointerVal);
break;
}
case CK_NullToMemberPointer:
// FIXME: Implement pointers to members. For now, don't associate a value
// with this expression.
break;
case CK_FunctionToPointerDecay: {
StorageLocation *PointeeLoc =
Env.getStorageLocation(*SubExpr, SkipPast::Reference);
StorageLocation *PointeeLoc = Env.getStorageLocationStrict(*SubExpr);
if (PointeeLoc == nullptr)
break;

auto &PointerLoc = Env.createStorageLocation(*S);
auto &PointerVal = Env.create<PointerValue>(*PointeeLoc);
Env.setStorageLocation(*S, PointerLoc);
Env.setValue(PointerLoc, PointerVal);
Env.setValueStrict(*S, Env.create<PointerValue>(*PointeeLoc));
break;
}
case CK_BuiltinFnToFnPtr:
Expand Down Expand Up @@ -371,9 +358,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
if (SubExprVal == nullptr)
break;

auto &ExprLoc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, ExprLoc);
Env.setValue(ExprLoc, Env.makeNot(*SubExprVal));
Env.setValueStrict(*S, Env.makeNot(*SubExprVal));
break;
}
default:
Expand All @@ -388,16 +373,12 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
// `this` expression's pointee.
return;

auto &Loc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, Loc);
Env.setValue(Loc, Env.create<PointerValue>(*ThisPointeeLoc));
Env.setValueStrict(*S, Env.create<PointerValue>(*ThisPointeeLoc));
}

void VisitCXXNewExpr(const CXXNewExpr *S) {
auto &Loc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, Loc);
if (Value *Val = Env.createValue(S->getType()))
Env.setValue(Loc, *Val);
Env.setValueStrict(*S, *Val);
}

void VisitCXXDeleteExpr(const CXXDeleteExpr *S) {
Expand Down Expand Up @@ -450,7 +431,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
if (VarDeclLoc == nullptr)
return;

Env.setStorageLocation(*S, *VarDeclLoc);
Env.setStorageLocationStrict(*S, *VarDeclLoc);
return;
}
}
Expand Down Expand Up @@ -484,16 +465,17 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
assert(Arg != nullptr);

auto *ArgLoc = cast_or_null<AggregateStorageLocation>(
Env.getStorageLocation(*Arg, SkipPast::Reference));
Env.getStorageLocationStrict(*Arg));
if (ArgLoc == nullptr)
return;

if (S->isElidable()) {
Env.setStorageLocation(*S, *ArgLoc);
} else if (auto *ArgVal = cast_or_null<StructValue>(Env.getValue(*Arg))) {
if (Value *Val = Env.getValue(*ArgLoc))
Env.setValueStrict(*S, *Val);
} else {
auto &Val = *cast<StructValue>(Env.createValue(S->getType()));
Env.setValueStrict(*S, Val);
copyRecord(ArgVal->getAggregateLoc(), Val.getAggregateLoc(), Env);
copyRecord(*ArgLoc, Val.getAggregateLoc(), Env);
}
return;
}
Expand Down Expand Up @@ -565,22 +547,20 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
const Expr *Arg = S->getArg(0);
assert(Arg != nullptr);

auto *ArgLoc = Env.getStorageLocation(*Arg, SkipPast::None);
auto *ArgLoc = Env.getStorageLocationStrict(*Arg);
if (ArgLoc == nullptr)
return;

Env.setStorageLocation(*S, *ArgLoc);
Env.setStorageLocationStrict(*S, *ArgLoc);
} else if (S->getDirectCallee() != nullptr &&
S->getDirectCallee()->getBuiltinID() ==
Builtin::BI__builtin_expect) {
assert(S->getNumArgs() > 0);
assert(S->getArg(0) != nullptr);
// `__builtin_expect` returns by-value, so strip away any potential
// references in the argument.
auto *ArgLoc = Env.getStorageLocation(*S->getArg(0), SkipPast::Reference);
if (ArgLoc == nullptr)
auto *ArgVal = Env.getValue(*S->getArg(0));
if (ArgVal == nullptr)
return;
Env.setStorageLocation(*S, *ArgLoc);
Env.setValueStrict(*S, *ArgVal);
} else if (const FunctionDecl *F = S->getDirectCallee()) {
transferInlineCall(S, F);
}
Expand All @@ -595,13 +575,13 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
return;

if (StructValue *StructVal = dyn_cast<StructValue>(SubExprVal)) {
Env.setStorageLocation(*S, StructVal->getAggregateLoc());
Env.setStorageLocationStrict(*S, StructVal->getAggregateLoc());
return;
}

StorageLocation &Loc = Env.createStorageLocation(*S);
Env.setValue(Loc, *SubExprVal);
Env.setStorageLocation(*S, Loc);
Env.setStorageLocationStrict(*S, Loc);
}

void VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *S) {
Expand Down

0 comments on commit f76f667

Please sign in to comment.