Unpack any Top pointer null state prior to comparison in modeling Abseil's CHECK_NE

nullability/test/check_macros.cc

@@ -65,6 +65,29 @@ TEST void checkNERightSmartPointer(std::unique_ptr<int> P) {
   nonnull(P);
 }
 
+// Test where a loop can result in Top pointer null states.
+// Make sure we can still do a null check after the loop, despite the Top
+// null state.
+TEST bool checkNEAfterLoop(int* _Nullable P, bool B) {
+  int X = 17;
+  for (int I = 0; I < 10; ++I) {
+    if (B) P = &X;
+  }
+  CHECK_NE(P, nullptr);
+  nonnull(P);
+  return {};
+}
+
+TEST bool checkNEAfterLoopSmartPointer(_Nullable std::unique_ptr<int> P,
+                                       bool B) {
+  for (int I = 0; I < 10; ++I) {
+    if (B) P = std::make_unique<int>(17);
+  }
+  CHECK_NE(P, nullptr);
+  nonnull(P);
+  return {};
+}
+
 TEST void utilCheckNEImplModelEqualAndNull() {
   int *P = nullptr;
   // `P` is definitely equal to `nullptr`, so result is nonnull.

nullability/value_transferer.cc

@@ -917,8 +917,18 @@ static void modelGetReferenceableValue(const CallExpr& CE, Environment& Env) {
   if (!CE.isGLValue()) return;
   assert(CE.getNumArgs() == 1);
   assert(CE.getArg(0) != nullptr);
-  if (StorageLocation* Loc = Env.getStorageLocation(*CE.getArg(0)))
+  if (StorageLocation* Loc = Env.getStorageLocation(*CE.getArg(0))) {
     Env.setStorageLocation(CE, *Loc);
+    // Normally, we do unpackPointerValue during an LValueToRValue conversion
+    // for raw pointers (smart pointers are already unpacked during
+    // ensureSmartPointerInitialized). Since the result of GetReferenceableValue
+    // is passed to a helper function (`CheckNE_Impl`), the LValueToRValue
+    // cast is in the separate function and we won't see it in this function.
+    // Model as if the LValueToRValue happens here and unpack.
+    if (isSupportedRawPointerType(CE.getArg(0)->getType())) {
+      unpackPointerValue(*Loc, Env);
+    }
+  }
 }
 
 // Models the Abseil-logging `CheckNE_Impl` function. Essentially, associates