[clang] f10d271 - [clang][dataflow] Handle null pointers of type std::nullptr_t

[Eric Li via cfe-commits]

Author: Eric Li
Date: 2022-07-05T13:49:26Z
New Revision: f10d271ae27f35cd56535ff7e832a358732c8fcd

URL: https://github.com/llvm/llvm-project/commit/f10d271ae27f35cd56535ff7e832a358732c8fcd
DIFF: https://github.com/llvm/llvm-project/commit/f10d271ae27f35cd56535ff7e832a358732c8fcd.diff

LOG: [clang][dataflow] Handle null pointers of type std::nullptr_t

Treat `std::nullptr_t` as a regular scalar type to avoid tripping
assertions when analyzing code that uses `std::nullptr_t`.

Differential Revision: https://reviews.llvm.org/D129097

Added: 
    

Modified: 
    clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
    clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
    clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index c1100d8474aa4..d87b9cc37b996 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -155,6 +155,7 @@ class DataflowAnalysisContext {
 
   /// Returns a pointer value that represents a null pointer. Calls with
   /// `PointeeType` that are canonically equivalent will return the same result.
+  /// A null `PointeeType` can be used for the pointee of `std::nullptr_t`.
   PointerValue &getOrCreateNullPointerValue(QualType PointeeType);
 
   /// Returns a symbolic boolean value that models a boolean literal equal to
@@ -251,6 +252,17 @@ class DataflowAnalysisContext {
   bool equivalentBoolValues(BoolValue &Val1, BoolValue &Val2);
 
 private:
+  struct NullableQualTypeDenseMapInfo : private llvm::DenseMapInfo<QualType> {
+    static QualType getEmptyKey() {
+      // Allow a NULL `QualType` by using a 
diff erent value as the empty key.
+      return QualType::getFromOpaquePtr(reinterpret_cast<Type *>(1));
+    }
+
+    using DenseMapInfo::getHashValue;
+    using DenseMapInfo::getTombstoneKey;
+    using DenseMapInfo::isEqual;
+  };
+
   /// Adds all constraints of the flow condition identified by `Token` and all
   /// of its transitive dependencies to `Constraints`. `VisitedTokens` is used
   /// to track tokens of flow conditions that were already visited by recursive
@@ -311,7 +323,8 @@ class DataflowAnalysisContext {
   // required to initialize the `PointeeLoc` field in `PointerValue`. Consider
   // creating a type-independent `NullPointerValue` without a `PointeeLoc`
   // field.
-  llvm::DenseMap<QualType, PointerValue *> NullPointerVals;
+  llvm::DenseMap<QualType, PointerValue *, NullableQualTypeDenseMapInfo>
+      NullPointerVals;
 
   AtomicBoolValue &TrueVal;
   AtomicBoolValue &FalseVal;

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
index e08fc71c51dc7..cd87e87a6acab 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -24,8 +24,8 @@ namespace dataflow {
 
 StorageLocation &
 DataflowAnalysisContext::getStableStorageLocation(QualType Type) {
-  assert(!Type.isNull());
-  if (Type->isStructureOrClassType() || Type->isUnionType()) {
+  if (!Type.isNull() &&
+      (Type->isStructureOrClassType() || Type->isUnionType())) {
     // FIXME: Explore options to avoid eager initialization of fields as some of
     // them might not be needed for a particular analysis.
     llvm::DenseMap<const ValueDecl *, StorageLocation *> FieldLocs;
@@ -57,8 +57,8 @@ DataflowAnalysisContext::getStableStorageLocation(const Expr &E) {
 
 PointerValue &
 DataflowAnalysisContext::getOrCreateNullPointerValue(QualType PointeeType) {
-  assert(!PointeeType.isNull());
-  auto CanonicalPointeeType = PointeeType.getCanonicalType();
+  auto CanonicalPointeeType =
+      PointeeType.isNull() ? PointeeType : PointeeType.getCanonicalType();
   auto Res = NullPointerVals.try_emplace(CanonicalPointeeType, nullptr);
   if (Res.second) {
     auto &PointeeLoc = getStableStorageLocation(CanonicalPointeeType);

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 04710b1795ef4..c4a42061edd90 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2213,12 +2213,14 @@ TEST(TransferTest, IntegralToBooleanCastFromBool) {
 
 TEST(TransferTest, NullToPointerCast) {
   std::string Code = R"(
+    using my_nullptr_t = decltype(nullptr);
     struct Baz {};
     void target() {
       int *FooX = nullptr;
       int *FooY = nullptr;
       bool **Bar = nullptr;
       Baz *Baz = nullptr;
+      my_nullptr_t Null = 0;
       // [[p]]
     }
   )";
@@ -2242,6 +2244,9 @@ TEST(TransferTest, NullToPointerCast) {
                 const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
                 ASSERT_THAT(BazDecl, NotNull());
 
+                const ValueDecl *NullDecl = findValueDecl(ASTCtx, "Null");
+                ASSERT_THAT(NullDecl, NotNull());
+
                 const auto *FooXVal =
                     cast<PointerValue>(Env.getValue(*FooXDecl, SkipPast::None));
                 const auto *FooYVal =
@@ -2250,6 +2255,8 @@ TEST(TransferTest, NullToPointerCast) {
                     cast<PointerValue>(Env.getValue(*BarDecl, SkipPast::None));
                 const auto *BazVal =
                     cast<PointerValue>(Env.getValue(*BazDecl, SkipPast::None));
+                const auto *NullVal =
+                    cast<PointerValue>(Env.getValue(*NullDecl, SkipPast::None));
 
                 EXPECT_EQ(FooXVal, FooYVal);
                 EXPECT_NE(FooXVal, BarVal);
@@ -2267,6 +2274,11 @@ TEST(TransferTest, NullToPointerCast) {
                 const StorageLocation &BazPointeeLoc = BazVal->getPointeeLoc();
                 EXPECT_TRUE(isa<AggregateStorageLocation>(BazPointeeLoc));
                 EXPECT_THAT(Env.getValue(BazPointeeLoc), IsNull());
+
+                const StorageLocation &NullPointeeLoc =
+                    NullVal->getPointeeLoc();
+                EXPECT_TRUE(isa<ScalarStorageLocation>(NullPointeeLoc));
+                EXPECT_THAT(Env.getValue(NullPointeeLoc), IsNull());
               });
 }