[clang][dataflow] Convert `PointeeLoc` of `PointerValue` from reference to pointer.

[wyt]

Summary

This allows PointeeLoc to be empty in the case of nullptr.

Depends On D127745

[wyt]

Update naming of getPointeeLoc with respect to change in parent

[sgatev]

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

189

Can you please document when this can be null?

clang/lib/Analysis/FlowSensitive/Transfer.cpp

266–267

Can you please add a test for this?

[ymandel]

Overall, this seems to be an important API change, and I'm having trouble seeing the big picture here. I think it would be great if you could expand (in the CL description) on the motivation behind the change and the thinking behind the particular design choices you've made here. For example, you mentioned that this change supports explicit modeling of nullptr, but what is gained by this explicit modeling? Also, regarding the design choice itself: is the intent that native nullptr now represents an interpreted nullptr? If so, have you considered instead introducing an explicit NullptrLocation or the like and if so, what were the tradeoffs in this decision?

clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

332

Please document the conditions under which these functions return nullptr.

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

348

If I understand correctly, the nullptr result from this function is now ambiguous: either there's no entry for that storagelocation, or there is and it is null. But, perhaps I'm misunderstanding -- is this not a concern?

clang/lib/Analysis/FlowSensitive/Transfer.cpp

266–267

How does this work in practice? It seems to be an unsual case that we statically know a pointer is null. So, what is the use of this change and (more importantly) what is the impact on the framework, if any?

clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h

private:
   ///
   /// Requirements:
   ///
   ///  `Type` must not be null.
   Value *createValueUnlessSelfReferential(QualType Type,
                                           llvm::DenseSet<QualType> &Visited,
                                           int Depth, int &CreatedValuesCount);
 
-  StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const;
-  const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const;
+  StorageLocation *skip(StorageLocation &Loc, SkipPast SP) const;
+  const StorageLocation *skip(const StorageLocation &Loc, SkipPast SP) const;
 
   // `DACtx` is not null and not owned by this object.
   DataflowAnalysisContext *DACtx;
 
   // Maps from program declarations and statements to storage locations that are
   // assigned to them. Unlike the maps in `DataflowAnalysisContext`, these
   // include only storage locations that are in scope for a particular basic
   // block.

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

 
 private:
   StorageLocation &ReferentLoc;
 };
 
 /// 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, with a flat map of fields to
 /// child storage locations, containing all accessible members of base struct
 /// and class types.
 class StructValue final : public Value {
 public:
   StructValue() : StructValue(llvm::DenseMap<const ValueDecl *, Value *>()) {}

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

 
 static bool areEquivalentIndirectionValues(Value *Val1, Value *Val2) {
   if (auto *IndVal1 = dyn_cast<ReferenceValue>(Val1)) {
     auto *IndVal2 = cast<ReferenceValue>(Val2);
     return &IndVal1->getReferentLoc() == &IndVal2->getReferentLoc();
   }
   if (auto *IndVal1 = dyn_cast<PointerValue>(Val1)) {
     auto *IndVal2 = cast<PointerValue>(Val2);
-    return &IndVal1->getPointeeLoc() == &IndVal2->getPointeeLoc();
+    return IndVal1->getPointeeLoc() == IndVal2->getPointeeLoc();
   }
   return false;
 }
 
 /// Returns true if and only if `Val1` is equivalent to `Val2`.
 static bool equivalentValues(QualType Type, Value *Val1,
                              const Environment &Env1, Value *Val2,
                              const Environment &Env2,
StorageLocation &Environment::createStorageLocation(const Expr &E) {
   return Loc;
 }
 
 void Environment::setStorageLocation(const ValueDecl &D, StorageLocation &Loc) {
   assert(DeclToLoc.find(&D) == DeclToLoc.end());
   DeclToLoc[&D] = &Loc;
 }
 
 StorageLocation *Environment::getStorageLocation(const ValueDecl &D,
                                                  SkipPast SP) const {
   auto It = DeclToLoc.find(&D);
-  return It == DeclToLoc.end() ? nullptr : &skip(*It->second, SP);
+  return It == DeclToLoc.end() ? nullptr : skip(*It->second, SP);
 }
 
 void Environment::setStorageLocation(const Expr &E, StorageLocation &Loc) {
   const Expr &CanonE = ignoreCFGOmittedNodes(E);
   assert(ExprToLoc.find(&CanonE) == ExprToLoc.end());
   ExprToLoc[&CanonE] = &Loc;
 }
 
 StorageLocation *Environment::getStorageLocation(const Expr &E,
                                                  SkipPast SP) const {
   // FIXME: Add a test with parens.
   auto It = ExprToLoc.find(&ignoreCFGOmittedNodes(E));
-  return It == ExprToLoc.end() ? nullptr : &skip(*It->second, SP);
+  return It == ExprToLoc.end() ? nullptr : skip(*It->second, SP);
 }
 
 StorageLocation *Environment::getThisPointeeStorageLocation() const {
   return DACtx->getThisPointeeStorageLocation();
 }
 
 void Environment::setValue(const StorageLocation &Loc, Value &Val) {
   LocToVal[&Loc] = &Val;
if (!Visited.contains(PointeeType.getCanonicalType())) {
       Value *PointeeVal = createValueUnlessSelfReferential(
           PointeeType, Visited, Depth, CreatedValuesCount);
       Visited.erase(PointeeType.getCanonicalType());
 
       if (PointeeVal != nullptr)
         setValue(PointeeLoc, *PointeeVal);
     }
 
-    return &takeOwnership(std::make_unique<PointerValue>(PointeeLoc));
+    return &takeOwnership(std::make_unique<PointerValue>(&PointeeLoc));
   }
 
   if (Type->isStructureOrClassType()) {
     CreatedValuesCount++;
     // FIXME: Initialize only fields that are accessed in the context that is
     // being analyzed.
     llvm::DenseMap<const ValueDecl *, Value *> FieldValues;
     for (const FieldDecl *Field : getObjectFields(Type)) {
if (Type->isStructureOrClassType()) {
 
     return &takeOwnership(
         std::make_unique<StructValue>(std::move(FieldValues)));
   }
 
   return nullptr;
 }
 
-StorageLocation &Environment::skip(StorageLocation &Loc, SkipPast SP) const {
+StorageLocation *Environment::skip(StorageLocation &Loc, SkipPast SP) const {
   switch (SP) {
   case SkipPast::None:
-    return Loc;
+    return &Loc;
   case SkipPast::Reference:
     // References cannot be chained so we only need to skip past one level of
     // indirection.
     if (auto *Val = dyn_cast_or_null<ReferenceValue>(getValue(Loc)))
-      return Val->getReferentLoc();
-    return Loc;
+      return &Val->getReferentLoc();
+    return &Loc;
   case SkipPast::ReferenceThenPointer:
-    StorageLocation &LocPastRef = skip(Loc, SkipPast::Reference);
-    if (auto *Val = dyn_cast_or_null<PointerValue>(getValue(LocPastRef)))
+    StorageLocation *LocPastRef = skip(Loc, SkipPast::Reference);
+    assert(LocPastRef != nullptr);
+    if (auto *Val = dyn_cast_or_null<PointerValue>(getValue(*LocPastRef)))
       return Val->getPointeeLoc();
     return LocPastRef;
   }
   llvm_unreachable("bad SkipPast kind");
 }
 
-const StorageLocation &Environment::skip(const StorageLocation &Loc,
+const StorageLocation *Environment::skip(const StorageLocation &Loc,
                                          SkipPast SP) const {
   return skip(*const_cast<StorageLocation *>(&Loc), SP);
 }
 
 void Environment::addToFlowCondition(BoolValue &Val) {
   DACtx->addFlowConditionConstraint(*FlowConditionToken, Val);
 }
 
 bool Environment::flowConditionImplies(BoolValue &Val) const {
   return DACtx->flowConditionImplies(*FlowConditionToken, Val);
 }
 
 } // namespace dataflow
 } // namespace clang

clang/lib/Analysis/FlowSensitive/Transfer.cpp

void VisitUnaryOperator(const UnaryOperator *S) {
     switch (S->getOpcode()) {
     case UO_Deref: {
       // Skip past a reference to handle dereference of a dependent pointer.
       const auto *SubExprVal = cast_or_null<PointerValue>(
           Env.getValue(*SubExpr, SkipPast::Reference));
       if (SubExprVal == nullptr)
         break;
 
+      // If PointeeLoc is null, then we are dereferencing a nullptr, skip
+      // creating a value for the dereference
+      if (auto *PointeeLoc = SubExprVal->getPointeeLoc()) {
         auto &Loc = Env.createStorageLocation(*S);
         Env.setStorageLocation(*S, Loc);
-      Env.setValue(Loc, Env.takeOwnership(std::make_unique<ReferenceValue>(
-                            SubExprVal->getPointeeLoc())));
+        Env.setValue(Loc, Env.takeOwnership(
+                              std::make_unique<ReferenceValue>(*PointeeLoc)));
+      }
       break;
     }
     case UO_AddrOf: {
       // Do not form a pointer to a reference. If `SubExpr` is assigned a
       // `ReferenceValue` then form a value that points to the location of its
       // pointee.
       StorageLocation *PointeeLoc =
           Env.getStorageLocation(*SubExpr, SkipPast::Reference);
       if (PointeeLoc == nullptr)
         break;
 
       auto &PointerLoc = Env.createStorageLocation(*S);
       auto &PointerVal =
-          Env.takeOwnership(std::make_unique<PointerValue>(*PointeeLoc));
+          Env.takeOwnership(std::make_unique<PointerValue>(PointeeLoc));
       Env.setStorageLocation(*S, PointerLoc);
       Env.setValue(PointerLoc, PointerVal);
       break;
     }
     case UO_LNot: {
       auto *SubExprVal =
           dyn_cast_or_null<BoolValue>(Env.getValue(*SubExpr, SkipPast::None));
       if (SubExprVal == nullptr)
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;
 
     auto &Loc = Env.createStorageLocation(*S);
     Env.setStorageLocation(*S, Loc);
-    Env.setValue(Loc, Env.takeOwnership(
-                          std::make_unique<PointerValue>(*ThisPointeeLoc)));
+    Env.setValue(
+        Loc, Env.takeOwnership(std::make_unique<PointerValue>(ThisPointeeLoc)));
   }
 
   void VisitMemberExpr(const MemberExpr *S) {
     ValueDecl *Member = S->getMemberDecl();
     assert(Member != nullptr);
 
     // FIXME: Consider assigning pointer values to function member expressions.
     if (Member->isFunctionOrFunctionTemplate())

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

runDataflow(Code, [](llvm::ArrayRef<std::pair<
 
     const auto *FooRefVal =
         cast<ReferenceValue>(BarReferentVal->getChild(*FooRefDecl));
     const StorageLocation &FooReferentLoc = FooRefVal->getReferentLoc();
     EXPECT_THAT(Env.getValue(FooReferentLoc), IsNull());
 
     const auto *FooPtrVal =
         cast<PointerValue>(BarReferentVal->getChild(*FooPtrDecl));
-    const StorageLocation &FooPtrPointeeLoc = FooPtrVal->getPointeeLoc();
-    EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
+    const StorageLocation *FooPtrPointeeLoc = FooPtrVal->getPointeeLoc();
+    ASSERT_THAT(FooPtrPointeeLoc, NotNull());
+    EXPECT_THAT(Env.getValue(*FooPtrPointeeLoc), IsNull());
 
     const auto *BazRefVal =
         cast<ReferenceValue>(BarReferentVal->getChild(*BazRefDecl));
     const StorageLocation &BazReferentLoc = BazRefVal->getReferentLoc();
     EXPECT_THAT(Env.getValue(BazReferentLoc), NotNull());
 
     const auto *BazPtrVal =
         cast<PointerValue>(BarReferentVal->getChild(*BazPtrDecl));
-    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_F(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, SkipPast::None);
         ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc));
 
         const PointerValue *FooVal = cast<PointerValue>(Env.getValue(*FooLoc));
-        const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
-        EXPECT_TRUE(isa<AggregateStorageLocation>(&FooPointeeLoc));
+        const StorageLocation *FooPointeeLoc = FooVal->getPointeeLoc();
+        EXPECT_TRUE(isa_and_nonnull<AggregateStorageLocation>(FooPointeeLoc));
 
-        const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
+        const Value *FooPointeeVal = Env.getValue(*FooPointeeLoc);
         EXPECT_TRUE(isa_and_nonnull<StructValue>(FooPointeeVal));
       });
 }
 
 TEST_F(TransferTest, SelfReferentialPointerVarDecl) {
   std::string Code = R"(
     struct A;
 
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, SkipPast::None));
         const auto *FooVal = cast<PointerValue>(Env.getValue(*FooLoc));
+        const auto *FooPointeeLoc = FooVal->getPointeeLoc();
+        ASSERT_THAT(FooPointeeLoc, NotNull());
         const auto *FooPointeeVal =
-            cast<StructValue>(Env.getValue(FooVal->getPointeeLoc()));
+            cast<StructValue>(Env.getValue(*FooPointeeLoc));
 
         const auto *BarVal =
             cast<PointerValue>(FooPointeeVal->getChild(*BarDecl));
+        const auto *BarPointeeLoc = BarVal->getPointeeLoc();
+        ASSERT_THAT(BarPointeeLoc, NotNull());
         const auto *BarPointeeVal =
-            cast<StructValue>(Env.getValue(BarVal->getPointeeLoc()));
+            cast<StructValue>(Env.getValue(*BarPointeeLoc));
 
         const auto *FooRefVal =
             cast<ReferenceValue>(BarPointeeVal->getChild(*FooRefDecl));
         const StorageLocation &FooReferentLoc = FooRefVal->getReferentLoc();
         EXPECT_THAT(Env.getValue(FooReferentLoc), IsNull());
 
         const auto *FooPtrVal =
             cast<PointerValue>(BarPointeeVal->getChild(*FooPtrDecl));
-        const StorageLocation &FooPtrPointeeLoc = FooPtrVal->getPointeeLoc();
-        EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
+        const StorageLocation *FooPtrPointeeLoc = FooPtrVal->getPointeeLoc();
+        ASSERT_THAT(FooPtrPointeeLoc, NotNull());
+        EXPECT_THAT(Env.getValue(*FooPtrPointeeLoc), IsNull());
 
         const auto *BazRefVal =
             cast<ReferenceValue>(BarPointeeVal->getChild(*BazRefDecl));
         const StorageLocation &BazReferentLoc = BazRefVal->getReferentLoc();
         EXPECT_THAT(Env.getValue(BazReferentLoc), NotNull());
 
         const auto *BazPtrVal =
             cast<PointerValue>(BarPointeeVal->getChild(*BazPtrDecl));
-        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_F(TransferTest, MultipleVarsDecl) {
   std::string Code = R"(
     void target() {
       int Foo, Bar;
       (void)0;
runDataflow(Code,
                 const Value *FooVal = Env.getValue(*FooDecl, SkipPast::None);
                 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, SkipPast::None));
-                EXPECT_EQ(Env.getValue(BarVal->getPointeeLoc()), FooVal);
+                const auto *BarPointeeLoc = BarVal->getPointeeLoc();
+                ASSERT_THAT(BarPointeeLoc, NotNull());
+                EXPECT_EQ(Env.getValue(*BarPointeeLoc), FooVal);
 
                 const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
                 ASSERT_THAT(BazDecl, NotNull());
 
                 EXPECT_EQ(Env.getValue(*BazDecl, SkipPast::None), FooVal);
               });
 }
 
runDataflow(
         const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
         ASSERT_THAT(FooDecl, NotNull());
 
         const StorageLocation *FooLoc =
             Env.getStorageLocation(*FooDecl, SkipPast::None);
         ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc));
 
         const PointerValue *FooVal = cast<PointerValue>(Env.getValue(*FooLoc));
-        const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
-        EXPECT_TRUE(isa<AggregateStorageLocation>(&FooPointeeLoc));
+        const StorageLocation *FooPointeeLoc = FooVal->getPointeeLoc();
+        EXPECT_TRUE(isa_and_nonnull<AggregateStorageLocation>(FooPointeeLoc));
 
-        const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
+        const Value *FooPointeeVal = Env.getValue(*FooPointeeLoc);
         EXPECT_TRUE(isa_and_nonnull<StructValue>(FooPointeeVal));
       });
 }
 
 TEST_F(TransferTest, StructMember) {
   std::string Code = R"(
     struct A {
       int Bar;
runDataflow(Code,
 
                 const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
                 ASSERT_THAT(BarDecl, NotNull());
 
                 const auto *FooLoc = cast<ScalarStorageLocation>(
                     Env.getStorageLocation(*FooDecl, SkipPast::None));
                 const auto *BarVal =
                     cast<PointerValue>(Env.getValue(*BarDecl, SkipPast::None));
-                EXPECT_EQ(&BarVal->getPointeeLoc(), FooLoc);
+                EXPECT_EQ(BarVal->getPointeeLoc(), FooLoc);
               });
 }
 
 TEST_F(TransferTest, AddrOfReference) {
   std::string Code = R"(
     void target(int *Foo) {
       int *Bar = &(*Foo);
       // [[p]]
runDataflow(Code,
 
                 const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
                 ASSERT_THAT(BarDecl, NotNull());
 
                 const auto *FooVal =
                     cast<PointerValue>(Env.getValue(*FooDecl, SkipPast::None));
                 const auto *BarVal =
                     cast<PointerValue>(Env.getValue(*BarDecl, SkipPast::None));
-                EXPECT_EQ(&BarVal->getPointeeLoc(), &FooVal->getPointeeLoc());
+                EXPECT_EQ(BarVal->getPointeeLoc(), FooVal->getPointeeLoc());
               });
 }
 
 TEST_F(TransferTest, DerefDependentPtr) {
   std::string Code = R"(
     template <typename T>
     void target(T *Foo) {
       T &Bar = *Foo;
runDataflow(
 
         const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
         ASSERT_THAT(BarDecl, NotNull());
 
         const auto *FooVal =
             cast<PointerValue>(Env.getValue(*FooDecl, SkipPast::None));
         const auto *BarVal =
             cast<ReferenceValue>(Env.getValue(*BarDecl, SkipPast::None));
-        EXPECT_EQ(&BarVal->getReferentLoc(), &FooVal->getPointeeLoc());
+        EXPECT_EQ(&BarVal->getReferentLoc(), FooVal->getPointeeLoc());
       });
 }
 
 TEST_F(TransferTest, VarDeclInitAssignConditionalOperator) {
   std::string Code = R"(
     struct A {};
 
     void target(A Foo, A Bar, bool Cond) {
runDataflow(Code,
                 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, SkipPast::None));
+                const auto *FooPointeeLoc = FooVal->getPointeeLoc();
+                ASSERT_THAT(FooPointeeLoc, NotNull());
                 const auto *FooPointeeVal =
-                    cast<IntegerValue>(Env.getValue(FooVal->getPointeeLoc()));
+                    cast<IntegerValue>(Env.getValue(*FooPointeeLoc));
 
                 const auto *BarVal = dyn_cast_or_null<IntegerValue>(
                     Env.getValue(*BarDecl, SkipPast::None));
                 ASSERT_THAT(BarVal, NotNull());
 
                 EXPECT_EQ(BarVal, FooPointeeVal);
               });
 }
runDataflow(
         const ValueDecl *LDecl = findValueDecl(ASTCtx, "l");
         ASSERT_THAT(LDecl, NotNull());
 
         // Inner.
         auto *LVal =
             dyn_cast<PointerValue>(InnerEnv.getValue(*LDecl, SkipPast::None));
         ASSERT_THAT(LVal, NotNull());
 
-        EXPECT_EQ(&LVal->getPointeeLoc(),
+        EXPECT_EQ(LVal->getPointeeLoc(),
                   InnerEnv.getStorageLocation(*ValDecl, SkipPast::Reference));
 
         // Outer.
         LVal =
             dyn_cast<PointerValue>(OuterEnv.getValue(*LDecl, SkipPast::None));
         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(),
+        EXPECT_NE(LVal->getPointeeLoc(),
                   OuterEnv.getStorageLocation(*ValDecl, SkipPast::Reference));
       });
 }
 
 TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
   std::string Code = R"(
     union Union {
       int A;

clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

runDataflow(Code,
 
                 const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
                 ASSERT_THAT(BarDecl, NotNull());
 
                 const auto *FooLoc = cast<ScalarStorageLocation>(
                     Env.getStorageLocation(*FooDecl, SkipPast::None));
                 const auto *BarVal =
                     cast<PointerValue>(Env.getValue(*BarDecl, SkipPast::None));
-                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) {