wyt created this revision.
Herald added subscribers: tschuett, steakhal, xazax.hun.
Herald added a project: All.
wyt requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We distinguish between getting pointee location for `ReferenceValue` and 
`PointerValue` as the former must be non-empty but the latter may be empty in 
the case of a `nullptr`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127745

Files:
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -676,7 +676,7 @@
                     Env.getStorageLocation(*FooDecl, SkipPast::None));
                 const auto *BarVal =
                     cast<PointerValue>(Env.getValue(*BarDecl, SkipPast::None));
-                EXPECT_EQ(&BarVal->getPointeeLoc(), FooLoc);
+                EXPECT_EQ(&BarVal->getPointerPointeeLoc(), FooLoc);
               });
 }
 
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -293,29 +293,29 @@
       // [[p]]
     }
   )";
-  runDataflow(Code,
-              [](llvm::ArrayRef<
-                     std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
-                     Results,
-                 ASTContext &ASTCtx) {
-                ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
-                const Environment &Env = Results[0].second.Env;
+  runDataflow(
+      Code, [](llvm::ArrayRef<
+                   std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+                   Results,
+               ASTContext &ASTCtx) {
+        ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+        const Environment &Env = Results[0].second.Env;
 
-                const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
-                ASSERT_THAT(FooDecl, NotNull());
+        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 StorageLocation *FooLoc =
+            Env.getStorageLocation(*FooDecl, SkipPast::None);
+        ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc));
 
-                const ReferenceValue *FooVal =
-                    cast<ReferenceValue>(Env.getValue(*FooLoc));
-                const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
-                EXPECT_TRUE(isa<AggregateStorageLocation>(&FooPointeeLoc));
+        const ReferenceValue *FooVal =
+            cast<ReferenceValue>(Env.getValue(*FooLoc));
+        const StorageLocation &FooPointeeLoc = FooVal->getReferencePointeeLoc();
+        EXPECT_TRUE(isa<AggregateStorageLocation>(&FooPointeeLoc));
 
-                const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
-                EXPECT_TRUE(isa_and_nonnull<StructValue>(FooPointeeVal));
-              });
+        const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
+        EXPECT_TRUE(isa_and_nonnull<StructValue>(FooPointeeVal));
+      });
 }
 
 TEST_F(TransferTest, SelfReferentialReferenceVarDecl) {
@@ -398,31 +398,33 @@
         Env.getStorageLocation(*FooDecl, SkipPast::None));
     const auto *FooVal = cast<ReferenceValue>(Env.getValue(*FooLoc));
     const auto *FooPointeeVal =
-        cast<StructValue>(Env.getValue(FooVal->getPointeeLoc()));
+        cast<StructValue>(Env.getValue(FooVal->getReferencePointeeLoc()));
 
     const auto *BarVal =
         cast<ReferenceValue>(FooPointeeVal->getChild(*BarDecl));
     const auto *BarPointeeVal =
-        cast<StructValue>(Env.getValue(BarVal->getPointeeLoc()));
+        cast<StructValue>(Env.getValue(BarVal->getReferencePointeeLoc()));
 
     const auto *FooRefVal =
         cast<ReferenceValue>(BarPointeeVal->getChild(*FooRefDecl));
-    const StorageLocation &FooRefPointeeLoc = FooRefVal->getPointeeLoc();
+    const StorageLocation &FooRefPointeeLoc =
+        FooRefVal->getReferencePointeeLoc();
     EXPECT_THAT(Env.getValue(FooRefPointeeLoc), IsNull());
 
     const auto *FooPtrVal =
         cast<PointerValue>(BarPointeeVal->getChild(*FooPtrDecl));
-    const StorageLocation &FooPtrPointeeLoc = FooPtrVal->getPointeeLoc();
+    const StorageLocation &FooPtrPointeeLoc = FooPtrVal->getPointerPointeeLoc();
     EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
 
     const auto *BazRefVal =
         cast<ReferenceValue>(BarPointeeVal->getChild(*BazRefDecl));
-    const StorageLocation &BazRefPointeeLoc = BazRefVal->getPointeeLoc();
+    const StorageLocation &BazRefPointeeLoc =
+        BazRefVal->getReferencePointeeLoc();
     EXPECT_THAT(Env.getValue(BazRefPointeeLoc), NotNull());
 
     const auto *BazPtrVal =
         cast<PointerValue>(BarPointeeVal->getChild(*BazPtrDecl));
-    const StorageLocation &BazPtrPointeeLoc = BazPtrVal->getPointeeLoc();
+    const StorageLocation &BazPtrPointeeLoc = BazPtrVal->getPointerPointeeLoc();
     EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
   });
 }
@@ -454,7 +456,7 @@
         ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc));
 
         const PointerValue *FooVal = cast<PointerValue>(Env.getValue(*FooLoc));
-        const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
+        const StorageLocation &FooPointeeLoc = FooVal->getPointerPointeeLoc();
         EXPECT_TRUE(isa<AggregateStorageLocation>(&FooPointeeLoc));
 
         const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
@@ -555,31 +557,35 @@
             Env.getStorageLocation(*FooDecl, SkipPast::None));
         const auto *FooVal = cast<PointerValue>(Env.getValue(*FooLoc));
         const auto *FooPointeeVal =
-            cast<StructValue>(Env.getValue(FooVal->getPointeeLoc()));
+            cast<StructValue>(Env.getValue(FooVal->getPointerPointeeLoc()));
 
         const auto *BarVal =
             cast<PointerValue>(FooPointeeVal->getChild(*BarDecl));
         const auto *BarPointeeVal =
-            cast<StructValue>(Env.getValue(BarVal->getPointeeLoc()));
+            cast<StructValue>(Env.getValue(BarVal->getPointerPointeeLoc()));
 
         const auto *FooRefVal =
             cast<ReferenceValue>(BarPointeeVal->getChild(*FooRefDecl));
-        const StorageLocation &FooRefPointeeLoc = FooRefVal->getPointeeLoc();
+        const StorageLocation &FooRefPointeeLoc =
+            FooRefVal->getReferencePointeeLoc();
         EXPECT_THAT(Env.getValue(FooRefPointeeLoc), IsNull());
 
         const auto *FooPtrVal =
             cast<PointerValue>(BarPointeeVal->getChild(*FooPtrDecl));
-        const StorageLocation &FooPtrPointeeLoc = FooPtrVal->getPointeeLoc();
+        const StorageLocation &FooPtrPointeeLoc =
+            FooPtrVal->getPointerPointeeLoc();
         EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
 
         const auto *BazRefVal =
             cast<ReferenceValue>(BarPointeeVal->getChild(*BazRefDecl));
-        const StorageLocation &BazRefPointeeLoc = BazRefVal->getPointeeLoc();
+        const StorageLocation &BazRefPointeeLoc =
+            BazRefVal->getReferencePointeeLoc();
         EXPECT_THAT(Env.getValue(BazRefPointeeLoc), NotNull());
 
         const auto *BazPtrVal =
             cast<PointerValue>(BarPointeeVal->getChild(*BazPtrDecl));
-        const StorageLocation &BazPtrPointeeLoc = BazPtrVal->getPointeeLoc();
+        const StorageLocation &BazPtrPointeeLoc =
+            BazPtrVal->getPointerPointeeLoc();
         EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
       });
 }
@@ -799,7 +805,7 @@
 
                 const auto *BarVal =
                     cast<PointerValue>(Env.getValue(*BarDecl, SkipPast::None));
-                EXPECT_EQ(Env.getValue(BarVal->getPointeeLoc()), FooVal);
+                EXPECT_EQ(Env.getValue(BarVal->getPointerPointeeLoc()), FooVal);
 
                 const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
                 ASSERT_THAT(BazDecl, NotNull());
@@ -952,31 +958,31 @@
       // [[p]]
     }
   )";
-  runDataflow(Code,
-              [](llvm::ArrayRef<
-                     std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
-                     Results,
-                 ASTContext &ASTCtx) {
-                ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
-                const Environment &Env = Results[0].second.Env;
+  runDataflow(
+      Code, [](llvm::ArrayRef<
+                   std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+                   Results,
+               ASTContext &ASTCtx) {
+        ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+        const Environment &Env = Results[0].second.Env;
 
-                const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
-                ASSERT_THAT(FooDecl, NotNull());
+        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 StorageLocation *FooLoc =
+            Env.getStorageLocation(*FooDecl, SkipPast::None);
+        ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc));
 
-                const ReferenceValue *FooVal =
-                    dyn_cast<ReferenceValue>(Env.getValue(*FooLoc));
-                ASSERT_THAT(FooVal, NotNull());
+        const ReferenceValue *FooVal =
+            dyn_cast<ReferenceValue>(Env.getValue(*FooLoc));
+        ASSERT_THAT(FooVal, NotNull());
 
-                const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
-                EXPECT_TRUE(isa<AggregateStorageLocation>(&FooPointeeLoc));
+        const StorageLocation &FooPointeeLoc = FooVal->getReferencePointeeLoc();
+        EXPECT_TRUE(isa<AggregateStorageLocation>(&FooPointeeLoc));
 
-                const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
-                EXPECT_TRUE(isa_and_nonnull<StructValue>(FooPointeeVal));
-              });
+        const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
+        EXPECT_TRUE(isa_and_nonnull<StructValue>(FooPointeeVal));
+      });
 }
 
 TEST_F(TransferTest, PointerParamDecl) {
@@ -1004,7 +1010,7 @@
         ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(FooLoc));
 
         const PointerValue *FooVal = cast<PointerValue>(Env.getValue(*FooLoc));
-        const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
+        const StorageLocation &FooPointeeLoc = FooVal->getPointerPointeeLoc();
         EXPECT_TRUE(isa<AggregateStorageLocation>(&FooPointeeLoc));
 
         const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
@@ -1379,7 +1385,7 @@
         const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
         const auto *BarVal = cast<ReferenceValue>(FooVal->getChild(*BarDecl));
         const auto *BarPointeeVal =
-            cast<IntegerValue>(Env.getValue(BarVal->getPointeeLoc()));
+            cast<IntegerValue>(Env.getValue(BarVal->getReferencePointeeLoc()));
 
         const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
         ASSERT_THAT(BazDecl, NotNull());
@@ -1752,7 +1758,8 @@
 
         const auto *QuxVal =
             cast<ReferenceValue>(Env.getValue(*QuxDecl, SkipPast::None));
-        EXPECT_EQ(&QuxVal->getPointeeLoc(), &FooVal->getPointeeLoc());
+        EXPECT_EQ(&QuxVal->getReferencePointeeLoc(),
+                  &FooVal->getReferencePointeeLoc());
       });
 }
 
@@ -2240,7 +2247,7 @@
                     Env.getStorageLocation(*FooDecl, SkipPast::None));
                 const auto *BarVal =
                     cast<PointerValue>(Env.getValue(*BarDecl, SkipPast::None));
-                EXPECT_EQ(&BarVal->getPointeeLoc(), FooLoc);
+                EXPECT_EQ(&BarVal->getPointerPointeeLoc(), FooLoc);
               });
 }
 
@@ -2269,7 +2276,8 @@
                     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->getPointerPointeeLoc(),
+                          &FooVal->getPointerPointeeLoc());
               });
 }
 
@@ -2299,7 +2307,8 @@
             cast<PointerValue>(Env.getValue(*FooDecl, SkipPast::None));
         const auto *BarVal =
             cast<ReferenceValue>(Env.getValue(*BarDecl, SkipPast::None));
-        EXPECT_EQ(&BarVal->getPointeeLoc(), &FooVal->getPointeeLoc());
+        EXPECT_EQ(&BarVal->getReferencePointeeLoc(),
+                  &FooVal->getPointerPointeeLoc());
       });
 }
 
@@ -2353,31 +2362,31 @@
       /*[[p]]*/
     }
   )";
-  runDataflow(Code,
-              [](llvm::ArrayRef<
-                     std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
-                     Results,
-                 ASTContext &ASTCtx) {
-                ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
-                const Environment &Env = Results[0].second.Env;
+  runDataflow(
+      Code, [](llvm::ArrayRef<
+                   std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+                   Results,
+               ASTContext &ASTCtx) {
+        ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+        const Environment &Env = Results[0].second.Env;
 
-                const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
-                ASSERT_THAT(FooDecl, NotNull());
+        const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+        ASSERT_THAT(FooDecl, NotNull());
 
-                const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
-                ASSERT_THAT(BarDecl, NotNull());
+        const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+        ASSERT_THAT(BarDecl, NotNull());
 
-                const auto *FooVal =
-                    cast<PointerValue>(Env.getValue(*FooDecl, SkipPast::None));
-                const auto *FooPointeeVal =
-                    cast<IntegerValue>(Env.getValue(FooVal->getPointeeLoc()));
+        const auto *FooVal =
+            cast<PointerValue>(Env.getValue(*FooDecl, SkipPast::None));
+        const auto *FooPointeeVal =
+            cast<IntegerValue>(Env.getValue(FooVal->getPointerPointeeLoc()));
 
-                const auto *BarVal = dyn_cast_or_null<IntegerValue>(
-                    Env.getValue(*BarDecl, SkipPast::None));
-                ASSERT_THAT(BarVal, NotNull());
+        const auto *BarVal = dyn_cast_or_null<IntegerValue>(
+            Env.getValue(*BarDecl, SkipPast::None));
+        ASSERT_THAT(BarVal, NotNull());
 
-                EXPECT_EQ(BarVal, FooPointeeVal);
-              });
+        EXPECT_EQ(BarVal, FooPointeeVal);
+      });
 }
 
 TEST_F(TransferTest, AggregateInitialization) {
@@ -3331,7 +3340,7 @@
             dyn_cast<PointerValue>(InnerEnv.getValue(*LDecl, SkipPast::None));
         ASSERT_THAT(LVal, NotNull());
 
-        EXPECT_EQ(&LVal->getPointeeLoc(),
+        EXPECT_EQ(&LVal->getPointerPointeeLoc(),
                   InnerEnv.getStorageLocation(*ValDecl, SkipPast::Reference));
 
         // Outer.
@@ -3341,7 +3350,7 @@
 
         // 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->getPointerPointeeLoc(),
                   OuterEnv.getStorageLocation(*ValDecl, SkipPast::Reference));
       });
 }
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -266,7 +266,7 @@
       auto &Loc = Env.createStorageLocation(*S);
       Env.setStorageLocation(*S, Loc);
       Env.setValue(Loc, Env.takeOwnership(std::make_unique<ReferenceValue>(
-                            SubExprVal->getPointeeLoc())));
+                            SubExprVal->getPointerPointeeLoc())));
       break;
     }
     case UO_AddrOf: {
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -236,7 +236,7 @@
   // `Value` representing the optional (here, `OptionalVal`).
   if (auto *ValueProp = OptionalVal.getProperty("value")) {
     auto *ValueRef = clang::cast<ReferenceValue>(ValueProp);
-    auto &ValueLoc = ValueRef->getPointeeLoc();
+    auto &ValueLoc = ValueRef->getReferencePointeeLoc();
     if (Env.getValue(ValueLoc) == nullptr) {
       // The property was previously set, but the value has been lost. This can
       // happen, for example, because of an environment merge (where the two
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -53,11 +53,12 @@
 static bool areEquivalentIndirectionValues(Value *Val1, Value *Val2) {
   if (auto *IndVal1 = dyn_cast<ReferenceValue>(Val1)) {
     auto *IndVal2 = cast<ReferenceValue>(Val2);
-    return &IndVal1->getPointeeLoc() == &IndVal2->getPointeeLoc();
+    return &IndVal1->getReferencePointeeLoc() ==
+           &IndVal2->getReferencePointeeLoc();
   }
   if (auto *IndVal1 = dyn_cast<PointerValue>(Val1)) {
     auto *IndVal2 = cast<PointerValue>(Val2);
-    return &IndVal1->getPointeeLoc() == &IndVal2->getPointeeLoc();
+    return &IndVal1->getPointerPointeeLoc() == &IndVal2->getPointerPointeeLoc();
   }
   return false;
 }
@@ -522,12 +523,12 @@
     // 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->getPointeeLoc();
+      return Val->getReferencePointeeLoc();
     return Loc;
   case SkipPast::ReferenceThenPointer:
     StorageLocation &LocPastRef = skip(Loc, SkipPast::Reference);
     if (auto *Val = dyn_cast_or_null<PointerValue>(getValue(LocPastRef)))
-      return Val->getPointeeLoc();
+      return Val->getPointerPointeeLoc();
     return LocPastRef;
   }
   llvm_unreachable("bad SkipPast kind");
Index: clang/include/clang/Analysis/FlowSensitive/Value.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/Value.h
+++ clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -177,7 +177,7 @@
     return Val->getKind() == Kind::Reference;
   }
 
-  StorageLocation &getPointeeLoc() const { return PointeeLoc; }
+  StorageLocation &getReferencePointeeLoc() const { return PointeeLoc; }
 
 private:
   StorageLocation &PointeeLoc;
@@ -193,7 +193,7 @@
     return Val->getKind() == Kind::Pointer;
   }
 
-  StorageLocation &getPointeeLoc() const { return PointeeLoc; }
+  StorageLocation &getPointerPointeeLoc() const { return PointeeLoc; }
 
 private:
   StorageLocation &PointeeLoc;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to