[clang][dataflow][WIP] Prototype for pointer value widening (nullable `getPointeeLoc()` flavor)

[mboehme]

Summary

DO NOT REVIEW

This is a WIP for discussion only.

clang/include/clang/Analysis/FlowSensitive/Value.h

public:
   static bool classof(const Value *Val) {
     return Val->getKind() == Kind::Integer;
   }
 };
 
 /// Models a symbolic pointer. Specifically, any value of type `T*`.
 class PointerValue final : public Value {
 public:
-  explicit PointerValue(StorageLocation &PointeeLoc)
+  explicit PointerValue(StorageLocation *PointeeLoc)
       : Value(Kind::Pointer), PointeeLoc(PointeeLoc) {}
 
   static bool classof(const Value *Val) {
     return Val->getKind() == Kind::Pointer;
   }
 
-  StorageLocation &getPointeeLoc() const { return PointeeLoc; }
+  StorageLocation *getPointeeLoc() const { return PointeeLoc; }
 
 private:
-  StorageLocation &PointeeLoc;
+  StorageLocation *PointeeLoc;
 };
 
 /// Models a value of `struct` or `class` type.
 /// In C++, prvalues of class type serve only a limited purpose: They can only
 /// be used to initialize a result object. It is not possible to access member
 /// variables or call member functions on a prvalue of class type.
 /// Correspondingly, `RecordValue` also serves only two limited purposes:
 /// - It conveys a prvalue of class type from the place where the object is

clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp

 
 PointerValue &
 DataflowAnalysisContext::getOrCreateNullPointerValue(QualType PointeeType) {
   auto CanonicalPointeeType =
       PointeeType.isNull() ? PointeeType : PointeeType.getCanonicalType();
   auto Res = NullPointerVals.try_emplace(CanonicalPointeeType, nullptr);
   if (Res.second) {
     auto &PointeeLoc = createStorageLocation(CanonicalPointeeType);
-    Res.first->second = &arena().create<PointerValue>(PointeeLoc);
+    Res.first->second = &arena().create<PointerValue>(&PointeeLoc);
   }
   return *Res.first->second;
 }
 
 void DataflowAnalysisContext::addFlowConditionConstraint(
     Atom Token, const Formula &Constraint) {
   auto Res = FlowConditionConstraints.try_emplace(Token, &Constraint);
   if (!Res.second) {

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

if (isa<BoolValue>(&Prev)) {
     assert(isa<BoolValue>(Current));
     // Widen to Top, because we know they are different values. If previous was
     // already Top, re-use that to (implicitly) indicate that no change occured.
     if (isa<TopBoolValue>(Prev))
       return Prev;
     return CurrentEnv.makeTopBoolValue();
   }
 
+  if (auto *PrevPtr = dyn_cast<PointerValue>(&Prev)) {
+    auto &CurPtr = cast<PointerValue>(Current);
+
+    if (PrevPtr->getPointeeLoc() != CurPtr.getPointeeLoc())
+      // TODO: Preserve properties
+      return CurrentEnv.create<PointerValue>(nullptr);
+  }
+
   // FIXME: Add other built-in model widening.
 
   // Custom-model widening.
   if (auto *W = Model.widen(Type, Prev, PrevEnv, Current, CurrentEnv))
     return *W;
 
   // Default of widening is a no-op: leave the current value unchanged.
   return Current;
Value *Environment::createValueUnlessSelfReferential(
   }
 
   if (Type->isPointerType()) {
     CreatedValuesCount++;
     QualType PointeeType = Type->getPointeeType();
     StorageLocation &PointeeLoc =
         createLocAndMaybeValue(PointeeType, Visited, Depth, CreatedValuesCount);
 
-    return &arena().create<PointerValue>(PointeeLoc);
+    return &arena().create<PointerValue>(&PointeeLoc);
   }
 
   if (Type->isRecordType()) {
     CreatedValuesCount++;
     llvm::DenseMap<const ValueDecl *, StorageLocation *> FieldLocs;
     for (const FieldDecl *Field : DACtx->getModeledFields(Type)) {
       assert(Field != nullptr);
 

 
 RecordStorageLocation *getImplicitObjectLocation(const CXXMemberCallExpr &MCE,
                                                  const Environment &Env) {
   Expr *ImplicitObject = MCE.getImplicitObjectArgument();
   if (ImplicitObject == nullptr)
     return nullptr;
   if (ImplicitObject->getType()->isPointerType()) {
     if (auto *Val = cast_or_null<PointerValue>(Env.getValue(*ImplicitObject)))
-      return &cast<RecordStorageLocation>(Val->getPointeeLoc());
+      return cast_or_null<RecordStorageLocation>(Val->getPointeeLoc());
     return nullptr;
   }
   return cast_or_null<RecordStorageLocation>(
       Env.getStorageLocation(*ImplicitObject));
 }
 
 RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
                                              const Environment &Env) {
   Expr *Base = ME.getBase();
   if (Base == nullptr)
     return nullptr;
   if (ME.isArrow()) {
     if (auto *Val = cast_or_null<PointerValue>(Env.getValue(*Base)))
-      return &cast<RecordStorageLocation>(Val->getPointeeLoc());
+      return cast_or_null<RecordStorageLocation>(Val->getPointeeLoc());
     return nullptr;
   }
   return cast_or_null<RecordStorageLocation>(Env.getStorageLocation(*Base));
 }
 
 std::vector<FieldDecl *> getFieldsForInitListExpr(const RecordDecl *RD) {
   // Unnamed bitfields are only used for padding and do not appear in
   // `InitListExpr`'s inits. However, those fields do appear in `RecordDecl`'s

clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp

void dump(Value &V) {
 
     switch (V.getKind()) {
     case Value::Kind::Integer:
     case Value::Kind::TopBool:
     case Value::Kind::AtomicBool:
     case Value::Kind::FormulaBool:
       break;
     case Value::Kind::Pointer:
-      JOS.attributeObject(
-          "pointee", [&] { dump(cast<PointerValue>(V).getPointeeLoc()); });
+      if (StorageLocation *PointeeLoc = cast<PointerValue>(V).getPointeeLoc())
+        JOS.attributeObject("pointee", [&] { dump(*PointeeLoc); });
       break;
     case Value::Kind::Record:
       for (const auto &Child : cast<RecordValue>(V).getLoc().children())
         JOS.attributeObject("f:" + Child.first->getNameAsString(), [&] {
           if (Child.second)
             if (Value *Val = Env.getValue(*Child.second))
               dump(*Val);
         });

clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp

StorageLocation *maybeInitializeOptionalValueMember(QualType Q,
                                                     Environment &Env) {
   // The "value" property represents a synthetic field. As such, it needs
   // `StorageLocation`, like normal fields (and other variables). So, we model
   // it with a `PointerValue`, since that includes a storage location.  Once
   // the property is set, it will be shared by all environments that access the
   // `Value` representing the optional (here, `OptionalVal`).
   if (auto *ValueProp = OptionalVal.getProperty("value")) {
     auto *ValuePtr = clang::cast<PointerValue>(ValueProp);
-    auto &ValueLoc = ValuePtr->getPointeeLoc();
-    if (Env.getValue(ValueLoc) != nullptr)
-      return &ValueLoc;
+    auto *ValueLoc = ValuePtr->getPointeeLoc();
+    if (ValueLoc != nullptr) {
+      if (Env.getValue(*ValueLoc) != nullptr)
+        return ValueLoc;
 
       // The property was previously set, but the value has been lost. This can
       // happen in various situations, for example:
-    // - Because of an environment merge (where the two environments mapped the
+      // - Because of an environment merge (where the two environments mapped
+      // the
       //   property to different values, which resulted in them both being
       //   discarded).
       // - When two blocks in the CFG, with neither a dominator of the other,
       //   visit the same optional value. (FIXME: This is something we can and
       //   should fix -- see also the lengthy FIXME below.)
       // - Or even when a block is revisited during testing to collect
       //   per-statement state.
       // FIXME: This situation means that the optional contents are not shared
       // between branches and the like. Practically, this lack of sharing
       // reduces the precision of the model when the contents are relevant to
       // the check, like another optional or a boolean that influences control
       // flow.
-    if (ValueLoc.getType()->isRecordType()) {
-      refreshRecordValue(cast<RecordStorageLocation>(ValueLoc), Env);
-      return &ValueLoc;
+      if (ValueLoc->getType()->isRecordType()) {
+        refreshStructValue(cast<RecordStorageLocation>(*ValueLoc), Env);
+        return ValueLoc;
       } else {
-      auto *ValueVal = Env.createValue(ValueLoc.getType());
+        auto *ValueVal = Env.createValue(ValueLoc->getType());
         if (ValueVal == nullptr)
           return nullptr;
-      Env.setValue(ValueLoc, *ValueVal);
-      return &ValueLoc;
+        Env.setValue(*ValueLoc, *ValueVal);
+        return ValueLoc;
+      }
     }
   }
 
   auto Ty = Q.getNonReferenceType();
   auto &ValueLoc = Env.createObject(Ty);
-  auto &ValuePtr = Env.create<PointerValue>(ValueLoc);
+  auto &ValuePtr = Env.create<PointerValue>(&ValueLoc);
   // FIXME:
   // The change we make to the `value` property below may become visible to
   // other blocks that aren't successors of the current block and therefore
   // don't see the change we made above mapping `ValueLoc` to `ValueVal`. For
   // example:
   //
   //   void target(optional<int> oo, bool b) {
   //     // `oo` is associated with a `RecordValue` here, which we will call
auto *HasValueVal =
       cast_or_null<BoolValue>(OptionalVal.getProperty("has_value"));
   return HasValueVal != nullptr &&
          Env.flowConditionImplies(HasValueVal->formula());
 }
 
 Value *getValueBehindPossiblePointer(const Expr &E, const Environment &Env) {
   Value *Val = Env.getValue(E);
   if (auto *PointerVal = dyn_cast_or_null<PointerValue>(Val))
-    return Env.getValue(PointerVal->getPointeeLoc());
+    if (PointerVal->getPointeeLoc() != nullptr)
+      return Env.getValue(*PointerVal->getPointeeLoc());
   return Val;
 }
 
 void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
                         LatticeTransferState &State) {
   if (auto *OptionalVal =
           getValueBehindPossiblePointer(*ObjectExpr, State.Env)) {
     if (State.Env.getStorageLocation(*UnwrapExpr) == nullptr)
       if (auto *Loc = maybeInitializeOptionalValueMember(
               UnwrapExpr->getType(), *OptionalVal, State.Env))
         State.Env.setStorageLocation(*UnwrapExpr, *Loc);
   }
 }
 
 void transferArrowOpCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
                          LatticeTransferState &State) {
   if (auto *OptionalVal =
           getValueBehindPossiblePointer(*ObjectExpr, State.Env)) {
     if (auto *Loc = maybeInitializeOptionalValueMember(
             UnwrapExpr->getType()->getPointeeType(), *OptionalVal, State.Env)) {
-      State.Env.setValue(*UnwrapExpr, State.Env.create<PointerValue>(*Loc));
+      State.Env.setValue(*UnwrapExpr, State.Env.create<PointerValue>(Loc));
     }
   }
 }
 
 void transferMakeOptionalCall(const CallExpr *E,
                               const MatchFinder::MatchResult &,
                               LatticeTransferState &State) {
   createOptionalValue(State.Env.getResultObjectLocation(*E),

clang/lib/Analysis/FlowSensitive/Transfer.cpp

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);
       if (PointeeLoc == nullptr)
         break;
 
-      Env.setValue(*S, Env.create<PointerValue>(*PointeeLoc));
+      Env.setValue(*S, Env.create<PointerValue>(PointeeLoc));
       break;
     }
     case CK_BuiltinFnToFnPtr:
       // Despite its name, the result type of `BuiltinFnToFnPtr` is a function,
       // not a function pointer. In addition, builtin functions can only be
       // called directly; it is not legal to take their address. We therefore
       // don't need to create a value or storage location for them.
       break;
     default:
       break;
     }
   }
 
   void VisitUnaryOperator(const UnaryOperator *S) {
     const Expr *SubExpr = S->getSubExpr();
     assert(SubExpr != nullptr);
 
     switch (S->getOpcode()) {
     case UO_Deref: {
       const auto *SubExprVal =
           cast_or_null<PointerValue>(Env.getValue(*SubExpr));
       if (SubExprVal == nullptr)
         break;
 
-      Env.setStorageLocation(*S, SubExprVal->getPointeeLoc());
+      if (SubExprVal->getPointeeLoc() != nullptr)
+        Env.setStorageLocation(*S, *SubExprVal->getPointeeLoc());
       break;
     }
     case UO_AddrOf: {
       // FIXME: Model pointers to members.
       if (S->getType()->isMemberPointerType())
         break;
 
       if (StorageLocation *PointeeLoc = Env.getStorageLocation(*SubExpr))
-        Env.setValue(*S, Env.create<PointerValue>(*PointeeLoc));
+        Env.setValue(*S, Env.create<PointerValue>(PointeeLoc));
       break;
     }
     case UO_LNot: {
       auto *SubExprVal = dyn_cast_or_null<BoolValue>(Env.getValue(*SubExpr));
       if (SubExprVal == nullptr)
         break;
 
       Env.setValue(*S, Env.makeNot(*SubExprVal));
       break;
     }
     default:
       break;
     }
   }
 
   void VisitCXXThisExpr(const CXXThisExpr *S) {
     auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
     if (ThisPointeeLoc == nullptr)
       // Unions are not supported yet, and will not have a location for the
       // `this` expression's pointee.
       return;
 
-    Env.setValue(*S, Env.create<PointerValue>(*ThisPointeeLoc));
+    Env.setValue(*S, Env.create<PointerValue>(ThisPointeeLoc));
   }
 
   void VisitCXXNewExpr(const CXXNewExpr *S) {
     if (Value *Val = Env.createValue(S->getType()))
       Env.setValue(*S, *Val);
   }
 
   void VisitCXXDeleteExpr(const CXXDeleteExpr *S) {

clang/lib/Analysis/FlowSensitive/Value.cpp

 
 namespace clang {
 namespace dataflow {
 
 static bool areEquivalentIndirectionValues(const Value &Val1,
                                            const Value &Val2) {
   if (auto *IndVal1 = dyn_cast<PointerValue>(&Val1)) {
     auto *IndVal2 = cast<PointerValue>(&Val2);
-    return &IndVal1->getPointeeLoc() == &IndVal2->getPointeeLoc();
+    return IndVal1->getPointeeLoc() == IndVal2->getPointeeLoc();
   }
   return false;
 }
 
 bool areEquivalentValues(const Value &Val1, const Value &Val2) {
   return &Val1 == &Val2 || (Val1.getKind() == Val2.getKind() &&
                             (isa<TopBoolValue>(&Val1) ||
                              areEquivalentIndirectionValues(Val1, Val2)));
 }
 
 raw_ostream &operator<<(raw_ostream &OS, const Value &Val) {
   switch (Val.getKind()) {
   case Value::Kind::Pointer: {
     const auto *PV = dyn_cast<PointerValue>(&Val);
-    return OS << "Pointer(" << &PV->getPointeeLoc() << ")";
+    return OS << "Pointer(" << PV->getPointeeLoc() << ")";
   }
   // FIXME: support remaining cases.
   default:
     return OS << debugString(Val.getKind());
   }
 }
 
 } // namespace dataflow
 } // namespace clang

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

runDataflow(
         const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
 
         const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
         ASSERT_THAT(FooDecl, NotNull());
         auto *FooValue = dyn_cast_or_null<PointerValue>(Env.getValue(*FooDecl));
         ASSERT_THAT(FooValue, NotNull());
 
         EXPECT_TRUE(isa<RecordStorageLocation>(FooValue->getPointeeLoc()));
-        auto *FooPointeeValue = Env.getValue(FooValue->getPointeeLoc());
+        auto *FooPointeeValue = Env.getValue(*FooValue->getPointeeLoc());
         ASSERT_THAT(FooPointeeValue, NotNull());
         EXPECT_TRUE(isa<RecordValue>(FooPointeeValue));
       });
 }
 
 // As a memory optimization, we prevent modeling fields nested below a certain
 // level (currently, depth 3). This test verifies this lack of modeling. We also
 // include a regression test for the case that the unmodeled field is a
runDataflow(Code, [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>>
     const auto &FooReferentLoc =
         *cast<RecordStorageLocation>(BarLoc.getChild(*FooRefDecl));
     EXPECT_THAT(Env.getValue(FooReferentLoc), NotNull());
     EXPECT_THAT(getFieldValue(&FooReferentLoc, *BarDecl, Env), IsNull());
 
     const auto &FooPtrVal =
         *cast<PointerValue>(getFieldValue(&BarLoc, *FooPtrDecl, Env));
     const auto &FooPtrPointeeLoc =
-        cast<RecordStorageLocation>(FooPtrVal.getPointeeLoc());
+        *cast<RecordStorageLocation>(FooPtrVal.getPointeeLoc());
     EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), NotNull());
     EXPECT_THAT(getFieldValue(&FooPtrPointeeLoc, *BarDecl, Env), IsNull());
 
     EXPECT_THAT(getFieldValue(&BarLoc, *BazRefDecl, Env), NotNull());
 
     const auto &BazPtrVal =
         *cast<PointerValue>(getFieldValue(&BarLoc, *BazPtrDecl, Env));
-    const StorageLocation &BazPtrPointeeLoc = BazPtrVal.getPointeeLoc();
-    EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
+    const StorageLocation *BazPtrPointeeLoc = BazPtrVal.getPointeeLoc();
+    ASSERT_THAT(BazPtrPointeeLoc, NotNull());
+    EXPECT_THAT(Env.getValue(*BazPtrPointeeLoc), NotNull());
   });
 }
 
 TEST(TransferTest, PointerVarDecl) {
   std::string Code = R"(
     struct A {};
 
     A *getA();
runDataflow(
 
         const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
         ASSERT_THAT(FooDecl, NotNull());
 
         const StorageLocation *FooLoc = Env.getStorageLocation(*FooDecl);
         ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc));
 
         const PointerValue *FooVal = cast<PointerValue>(Env.getValue(*FooLoc));
-        const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
-        EXPECT_TRUE(isa<RecordStorageLocation>(&FooPointeeLoc));
+        const auto &FooPointeeLoc =
+            *cast<RecordStorageLocation>(FooVal->getPointeeLoc());
 
         const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
         EXPECT_TRUE(isa_and_nonnull<RecordValue>(FooPointeeVal));
       });
 }
 
 TEST(TransferTest, SelfReferentialPointerVarDecl) {
   std::string Code = R"(
runDataflow(
         ASSERT_THAT(FooRefDecl, NotNull());
         ASSERT_THAT(FooPtrDecl, NotNull());
         ASSERT_THAT(BazRefDecl, NotNull());
         ASSERT_THAT(BazPtrDecl, NotNull());
 
         const auto &FooLoc =
             *cast<ScalarStorageLocation>(Env.getStorageLocation(*FooDecl));
         const auto &FooVal = *cast<PointerValue>(Env.getValue(FooLoc));
+        ASSERT_THAT(FooVal.getPointeeLoc(), NotNull());
         const auto &FooPointeeVal =
-            *cast<RecordValue>(Env.getValue(FooVal.getPointeeLoc()));
+            *cast<RecordValue>(Env.getValue(*FooVal.getPointeeLoc()));
 
         const auto &BarVal =
             *cast<PointerValue>(getFieldValue(&FooPointeeVal, *BarDecl, Env));
+        ASSERT_THAT(BarVal.getPointeeLoc(), NotNull());
         const auto &BarPointeeVal =
-            *cast<RecordValue>(Env.getValue(BarVal.getPointeeLoc()));
+            *cast<RecordValue>(Env.getValue(*BarVal.getPointeeLoc()));
 
         EXPECT_THAT(getFieldValue(&BarPointeeVal, *FooRefDecl, Env), NotNull());
 
         const auto &FooPtrVal = *cast<PointerValue>(
             getFieldValue(&BarPointeeVal, *FooPtrDecl, Env));
+        ASSERT_THAT(FooPtrVal.getPointeeLoc(), NotNull());
         const auto &FooPtrPointeeLoc =
-            cast<RecordStorageLocation>(FooPtrVal.getPointeeLoc());
+            cast<RecordStorageLocation>(*FooPtrVal.getPointeeLoc());
         EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
 
         EXPECT_THAT(getFieldValue(&BarPointeeVal, *BazRefDecl, Env), NotNull());
 
         const auto &BazPtrVal = *cast<PointerValue>(
             getFieldValue(&BarPointeeVal, *BazPtrDecl, Env));
-        const StorageLocation &BazPtrPointeeLoc = BazPtrVal.getPointeeLoc();
-        EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
+        const StorageLocation *BazPtrPointeeLoc = BazPtrVal.getPointeeLoc();
+        ASSERT_THAT(BazPtrPointeeLoc, NotNull());
+        EXPECT_THAT(Env.getValue(*BazPtrPointeeLoc), NotNull());
       });
 }
 
 TEST(TransferTest, DirectlySelfReferentialReference) {
   std::string Code = R"(
     struct target {
       target() {
         (void)0;
runDataflow(
 
         const Value *FooVal = Env.getValue(*FooDecl);
         ASSERT_TRUE(isa_and_nonnull<IntegerValue>(FooVal));
 
         const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
         ASSERT_THAT(BarDecl, NotNull());
 
         const auto *BarVal = cast<PointerValue>(Env.getValue(*BarDecl));
-        EXPECT_EQ(Env.getValue(BarVal->getPointeeLoc()), FooVal);
+        ASSERT_THAT(BarVal->getPointeeLoc(), NotNull());
+        EXPECT_EQ(Env.getValue(*BarVal->getPointeeLoc()), FooVal);
 
         const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
         ASSERT_THAT(BazDecl, NotNull());
 
         EXPECT_EQ(Env.getValue(*BazDecl), FooVal);
       });
 }
 
runDataflow(
 
         const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
         ASSERT_THAT(FooDecl, NotNull());
 
         const StorageLocation *FooLoc = Env.getStorageLocation(*FooDecl);
         ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc));
 
         const PointerValue *FooVal = cast<PointerValue>(Env.getValue(*FooLoc));
-        const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
-        EXPECT_TRUE(isa<RecordStorageLocation>(&FooPointeeLoc));
+        const StorageLocation *FooPointeeLoc = FooVal->getPointeeLoc();
+        EXPECT_TRUE(isa<RecordStorageLocation>(FooPointeeLoc));
 
-        const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
+        const Value *FooPointeeVal = Env.getValue(*FooPointeeLoc);
         EXPECT_TRUE(isa_and_nonnull<RecordValue>(FooPointeeVal));
       });
 }
 
 TEST(TransferTest, StructMember) {
   std::string Code = R"(
     struct A {
       int Bar;
runDataflow(
         const auto *BazVal = cast<PointerValue>(Env.getValue(*BazDecl));
         const auto *NullVal = cast<PointerValue>(Env.getValue(*NullDecl));
 
         EXPECT_EQ(FooXVal, FooYVal);
         EXPECT_NE(FooXVal, BarVal);
         EXPECT_NE(FooXVal, BazVal);
         EXPECT_NE(BarVal, BazVal);
 
-        const StorageLocation &FooPointeeLoc = FooXVal->getPointeeLoc();
+        const StorageLocation *FooPointeeLoc = FooXVal->getPointeeLoc();
         EXPECT_TRUE(isa<ScalarStorageLocation>(FooPointeeLoc));
-        EXPECT_THAT(Env.getValue(FooPointeeLoc), IsNull());
+        EXPECT_THAT(Env.getValue(*FooPointeeLoc), IsNull());
 
-        const StorageLocation &BarPointeeLoc = BarVal->getPointeeLoc();
+        const StorageLocation *BarPointeeLoc = BarVal->getPointeeLoc();
         EXPECT_TRUE(isa<ScalarStorageLocation>(BarPointeeLoc));
-        EXPECT_THAT(Env.getValue(BarPointeeLoc), IsNull());
+        EXPECT_THAT(Env.getValue(*BarPointeeLoc), IsNull());
 
-        const StorageLocation &BazPointeeLoc = BazVal->getPointeeLoc();
+        const StorageLocation *BazPointeeLoc = BazVal->getPointeeLoc();
         EXPECT_TRUE(isa<RecordStorageLocation>(BazPointeeLoc));
-        EXPECT_THAT(Env.getValue(BazPointeeLoc), IsNull());
+        EXPECT_THAT(Env.getValue(*BazPointeeLoc), IsNull());
 
-        const StorageLocation &NullPointeeLoc = NullVal->getPointeeLoc();
+        const StorageLocation *NullPointeeLoc = NullVal->getPointeeLoc();
         EXPECT_TRUE(isa<ScalarStorageLocation>(NullPointeeLoc));
-        EXPECT_THAT(Env.getValue(NullPointeeLoc), IsNull());
+        EXPECT_THAT(Env.getValue(*NullPointeeLoc), IsNull());
       });
 }
 
 TEST(TransferTest, PointerToMemberVariable) {
   std::string Code = R"(
     struct S {
       int i;
     };
runDataflow(
         ASSERT_THAT(FooDecl, NotNull());
 
         const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
         ASSERT_THAT(BarDecl, NotNull());
 
         const auto *FooLoc =
             cast<ScalarStorageLocation>(Env.getStorageLocation(*FooDecl));
         const auto *BarVal = cast<PointerValue>(Env.getValue(*BarDecl));
-        EXPECT_EQ(&BarVal->getPointeeLoc(), FooLoc);
+        EXPECT_EQ(BarVal->getPointeeLoc(), FooLoc);
       });
 }
 
 TEST(TransferTest, AddrOfReference) {
   std::string Code = R"(
     void target(int *Foo) {
       int *Bar = &(*Foo);
       // [[p]]
runDataflow(
         const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
         ASSERT_THAT(FooDecl, NotNull());
 
         const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
         ASSERT_THAT(BarDecl, NotNull());
 
         const auto *FooVal = cast<PointerValue>(Env.getValue(*FooDecl));
         const auto *BarVal = cast<PointerValue>(Env.getValue(*BarDecl));
-        EXPECT_EQ(&BarVal->getPointeeLoc(), &FooVal->getPointeeLoc());
+        EXPECT_EQ(BarVal->getPointeeLoc(), FooVal->getPointeeLoc());
       });
 }
 
 TEST(TransferTest, CannotAnalyzeFunctionTemplate) {
   std::string Code = R"(
     template <typename T>
     void target() {}
   )";
runDataflow(
         const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
         ASSERT_THAT(FooDecl, NotNull());
 
         const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
         ASSERT_THAT(BarDecl, NotNull());
 
         const auto *FooVal =
             cast<PointerValue>(EnvAfterLoop.getValue(*FooDecl));
+        ASSERT_THAT(FooVal->getPointeeLoc(), NotNull());
         const auto *FooPointeeVal =
-            cast<IntegerValue>(EnvAfterLoop.getValue(FooVal->getPointeeLoc()));
+            cast<IntegerValue>(EnvAfterLoop.getValue(*FooVal->getPointeeLoc()));
 
         const auto *BarVal = cast<IntegerValue>(EnvInLoop.getValue(*BarDecl));
         EXPECT_EQ(BarVal, FooPointeeVal);
 
         // FIXME: This assertion documents current behavior, but we would prefer
         // declarations to be removed from the environment when their lifetime
         // ends. Once this is the case, change this assertion accordingly.
         ASSERT_THAT(EnvAfterLoop.getValue(*BarDecl), BarVal);
runDataflow(
 
         const ValueDecl *LDecl = findValueDecl(ASTCtx, "l");
         ASSERT_THAT(LDecl, NotNull());
 
         // Inner.
         auto *LVal = dyn_cast<PointerValue>(InnerEnv.getValue(*LDecl));
         ASSERT_THAT(LVal, NotNull());
 
-        EXPECT_EQ(&LVal->getPointeeLoc(),
-                  InnerEnv.getStorageLocation(*ValDecl));
+        EXPECT_EQ(LVal->getPointeeLoc(), InnerEnv.getStorageLocation(*ValDecl));
 
         // Outer.
         LVal = dyn_cast<PointerValue>(OuterEnv.getValue(*LDecl));
         ASSERT_THAT(LVal, NotNull());
 
         // The loop body may not have been executed, so we should not conclude
         // that `l` points to `val`.
-        EXPECT_NE(&LVal->getPointeeLoc(),
-                  OuterEnv.getStorageLocation(*ValDecl));
+        EXPECT_NE(LVal->getPointeeLoc(), OuterEnv.getStorageLocation(*ValDecl));
       });
 }
 
 TEST(TransferTest, LoopDereferencingChangingPointerConverges) {
   std::string Code = R"cc(
     bool some_condition();
 
     void target(int i1, int i2) {
       int *p = &i1;
       while (true) {
         (void)*p;
         if (some_condition())
           p = &i1;
         else
           p = &i2;
       }
     }
   )cc";
-  // FIXME: Implement pointer value widening to make analysis converge.
-  ASSERT_THAT_ERROR(
-      checkDataflowWithNoopAnalysis(Code),
-      llvm::FailedWithMessage("maximum number of iterations reached"));
+  ASSERT_THAT_ERROR(checkDataflowWithNoopAnalysis(Code), llvm::Succeeded());
 }
 
 TEST(TransferTest, LoopDereferencingChangingRecordPointerConverges) {
   std::string Code = R"cc(
     struct Lookup {
       int x;
     };
 
     bool some_condition();
 
     void target(Lookup l1, Lookup l2) {
       Lookup *l = &l1;
       while (true) {
         (void)l->x;
         if (some_condition())
           l = &l1;
         else
           l = &l2;
       }
     }
   )cc";
   // FIXME: Implement pointer value widening to make analysis converge.
-  ASSERT_THAT_ERROR(
-      checkDataflowWithNoopAnalysis(Code),
-      llvm::FailedWithMessage("maximum number of iterations reached"));
+  ASSERT_THAT_ERROR(checkDataflowWithNoopAnalysis(Code), llvm::Succeeded());
 }
 
 TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
   std::string Code = R"(
     union Union {
       int A;
       float B;
     };
runDataflow(
       Code,
       [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
          ASTContext &ASTCtx) {
         const Environment &Env =
             getEnvironmentAtAnnotation(Results, "after_new");
 
         auto &P = getValueForDecl<PointerValue>(ASTCtx, Env, "p");
 
-        EXPECT_THAT(Env.getValue(P.getPointeeLoc()), NotNull());
+        ASSERT_THAT(P.getPointeeLoc(), NotNull());
+        EXPECT_THAT(Env.getValue(*P.getPointeeLoc()), NotNull());
       });
 }
 
 TEST(TransferTest, NewExpressions_Structs) {
   std::string Code = R"(
     struct Inner {
       int InnerField;
     };
runDataflow(
         const Environment &Env =
             getEnvironmentAtAnnotation(Results, "after_new");
 
         const ValueDecl *OuterField = findValueDecl(ASTCtx, "OuterField");
         const ValueDecl *InnerField = findValueDecl(ASTCtx, "InnerField");
 
         auto &P = getValueForDecl<PointerValue>(ASTCtx, Env, "p");
 
-        auto &OuterLoc = cast<RecordStorageLocation>(P.getPointeeLoc());
+        auto &OuterLoc = *cast<RecordStorageLocation>(P.getPointeeLoc());
         auto &OuterFieldLoc =
             *cast<RecordStorageLocation>(OuterLoc.getChild(*OuterField));
         auto &InnerFieldLoc = *OuterFieldLoc.getChild(*InnerField);
 
         // Values for the struct and all fields exist after the new.
         EXPECT_THAT(Env.getValue(OuterLoc), NotNull());
         EXPECT_THAT(Env.getValue(OuterFieldLoc), NotNull());
         EXPECT_THAT(Env.getValue(InnerFieldLoc), NotNull());
runDataflow(
       [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
          ASTContext &ASTCtx) {
         const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
 
         auto &NonMemberP1 =
             getValueForDecl<PointerValue>(ASTCtx, Env, "non_member_p1");
         auto &NonMemberP2 =
             getValueForDecl<PointerValue>(ASTCtx, Env, "non_member_p2");
-        EXPECT_EQ(&NonMemberP1.getPointeeLoc(), &NonMemberP2.getPointeeLoc());
+        EXPECT_EQ(NonMemberP1.getPointeeLoc(), NonMemberP2.getPointeeLoc());
 
         auto &MemberP1 =
             getValueForDecl<PointerValue>(ASTCtx, Env, "member_p1");
         auto &MemberP2 =
             getValueForDecl<PointerValue>(ASTCtx, Env, "member_p2");
-        EXPECT_EQ(&MemberP1.getPointeeLoc(), &MemberP2.getPointeeLoc());
+        EXPECT_EQ(MemberP1.getPointeeLoc(), MemberP2.getPointeeLoc());
       });
 }
 
 // Check that a builtin function is not associated with a value. (It's only
 // possible to call builtin functions directly, not take their address.)
 TEST(TransferTest, BuiltinFunctionModeled) {
   std::string Code = R"(
     void target() {

clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

runDataflow(
         ASSERT_THAT(FooDecl, NotNull());
 
         const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
         ASSERT_THAT(BarDecl, NotNull());
 
         const auto *FooLoc =
             cast<ScalarStorageLocation>(Env.getStorageLocation(*FooDecl));
         const auto *BarVal = cast<PointerValue>(Env.getValue(*BarDecl));
-        EXPECT_EQ(&BarVal->getPointeeLoc(), FooLoc);
+        EXPECT_EQ(BarVal->getPointeeLoc(), FooLoc);
       });
 }
 
 TEST_F(WideningTest, DistinctValuesWithSamePropertiesAreEquivalent) {
   std::string Code = R"(
     #include "widening_test_defs.h"
 
     void target(bool Cond) {

clang/unittests/Analysis/FlowSensitive/ValueTest.cpp

 TEST(ValueTest, DifferentIntegerValuesNotEquivalent) {
   IntegerValue V1;
   IntegerValue V2;
   EXPECT_FALSE(areEquivalentValues(V1, V2));
 }
 
 TEST(ValueTest, AliasedPointersEquivalent) {
   auto L = ScalarStorageLocation(QualType());
-  PointerValue V1(L);
-  PointerValue V2(L);
+  PointerValue V1(&L);
+  PointerValue V2(&L);
   EXPECT_TRUE(areEquivalentValues(V1, V2));
   EXPECT_TRUE(areEquivalentValues(V2, V1));
 }
 
 TEST(ValueTest, TopsEquivalent) {
   Arena A;
   TopBoolValue V1(A.makeAtomRef(Atom(0)));
   TopBoolValue V2(A.makeAtomRef(Atom(1)));
TEST(ValueTest, EquivalentValuesWithDifferentPropsEquivalent) {
   V2.setProperty("bar", Prop2);
   EXPECT_TRUE(areEquivalentValues(V1, V2));
   EXPECT_TRUE(areEquivalentValues(V2, V1));
 }
 
 TEST(ValueTest, DifferentKindsNotEquivalent) {
   Arena A;
   auto L = ScalarStorageLocation(QualType());
-  PointerValue V1(L);
+  PointerValue V1(&L);
   TopBoolValue V2(A.makeAtomRef(Atom(0)));
   EXPECT_FALSE(areEquivalentValues(V1, V2));
   EXPECT_FALSE(areEquivalentValues(V2, V1));
 }
 
 TEST(ValueTest, NotAliasedPointersNotEquivalent) {
   auto L1 = ScalarStorageLocation(QualType());
-  PointerValue V1(L1);
+  PointerValue V1(&L1);
   auto L2 = ScalarStorageLocation(QualType());
-  PointerValue V2(L2);
+  PointerValue V2(&L2);
   EXPECT_FALSE(areEquivalentValues(V1, V2));
   EXPECT_FALSE(areEquivalentValues(V2, V1));
 }
 
 } // namespace