diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -336,8 +336,8 @@ if (const auto *MethodCall = dyn_cast(Call)) { if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) { if (!isa(Arg)) - Env.ThisPointeeLoc = cast( - getStorageLocation(*Arg, SkipPast::Reference)); + Env.ThisPointeeLoc = + cast(getStorageLocationStrict(*Arg)); // Otherwise (when the argument is `this`), retain the current // environment's `ThisPointeeLoc`. } @@ -832,9 +832,8 @@ // 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 @@ -913,16 +912,13 @@ 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(Env.getValue(*Loc))) + if (auto *Val = cast_or_null(Env.getValue(*ImplicitObject))) return &cast(Val->getPointeeLoc()); return nullptr; } - return cast(Loc); + return cast_or_null( + Env.getStorageLocationStrict(*ImplicitObject)); } AggregateStorageLocation *getBaseObjectLocation(const MemberExpr &ME, @@ -930,15 +926,13 @@ 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(Env.getValue(*Loc))) + if (auto *Val = cast_or_null(Env.getValue(*Base))) return &cast(Val->getPointeeLoc()); return nullptr; } - return cast(Loc); + return cast_or_null( + Env.getStorageLocationStrict(*Base)); } std::vector getFieldsForInitListExpr(const RecordDecl *RD) { diff --git a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp --- a/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp +++ b/clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp @@ -254,10 +254,17 @@ if (ElementIndex > 0) { auto S = Iters.back().first->Elements[ElementIndex - 1].getAs(); - if (const Expr *E = S ? llvm::dyn_cast(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(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); 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 @@ -242,10 +242,8 @@ 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(); } @@ -439,10 +437,10 @@ 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); } } @@ -471,9 +469,7 @@ 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); } } @@ -534,15 +530,20 @@ 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(State.Env.createStorageLocation(*E)); - State.Env.setStorageLocationStrict(*E, *Loc); + Loc = cast_or_null( + State.Env.getStorageLocationStrict(*E)); + if (!Loc) { + Loc = + &cast(State.Env.createStorageLocation(*E)); + State.Env.setStorageLocationStrict(*E, *Loc); + } } createOptionalValue(*Loc, State.Env.makeAtomicBoolValue(), State.Env); diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -141,30 +141,26 @@ 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: { @@ -238,7 +234,7 @@ 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 @@ -283,9 +279,7 @@ if (SubExprVal == nullptr) break; - auto &ExprLoc = Env.createStorageLocation(*S); - Env.setStorageLocation(*S, ExprLoc); - Env.setValue(ExprLoc, *SubExprVal); + Env.setValueStrict(*S, *SubExprVal); break; } @@ -308,12 +302,9 @@ 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: @@ -321,15 +312,11 @@ // 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(*PointeeLoc); - Env.setStorageLocation(*S, PointerLoc); - Env.setValue(PointerLoc, PointerVal); + Env.setValueStrict(*S, Env.create(*PointeeLoc)); break; } case CK_BuiltinFnToFnPtr: @@ -371,9 +358,7 @@ 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: @@ -388,16 +373,12 @@ // `this` expression's pointee. return; - auto &Loc = Env.createStorageLocation(*S); - Env.setStorageLocation(*S, Loc); - Env.setValue(Loc, Env.create(*ThisPointeeLoc)); + Env.setValueStrict(*S, Env.create(*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) { @@ -450,7 +431,7 @@ if (VarDeclLoc == nullptr) return; - Env.setStorageLocation(*S, *VarDeclLoc); + Env.setStorageLocationStrict(*S, *VarDeclLoc); return; } } @@ -484,16 +465,17 @@ assert(Arg != nullptr); auto *ArgLoc = cast_or_null( - 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(Env.getValue(*Arg))) { + if (Value *Val = Env.getValue(*ArgLoc)) + Env.setValueStrict(*S, *Val); + } else { auto &Val = *cast(Env.createValue(S->getType())); Env.setValueStrict(*S, Val); - copyRecord(ArgVal->getAggregateLoc(), Val.getAggregateLoc(), Env); + copyRecord(*ArgLoc, Val.getAggregateLoc(), Env); } return; } @@ -565,22 +547,20 @@ 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); } @@ -595,13 +575,13 @@ return; if (StructValue *StructVal = dyn_cast(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) {