From 434bd3ed97a851753cad71e7f2739daf25b77f0f Mon Sep 17 00:00:00 2001
From: Googler <no-reply@google.com>
Date: Thu, 12 Mar 2026 19:11:47 -0700
Subject: [PATCH] Unpack any Top pointer null state prior to comparison in
 modeling Abseil's CHECK_NE

CHECK_NE is implemented with
`const char* s = absl::...::Check_NEImpl(GetReferenceableValue(lhs), GetReferenceableValue(rhs))` as a component.

Since `GetReferenceableValue(lhs)` only passes along a reference to `lhs`,
we never see the LValueToRValue conversion where we normally do unpacking
for raw pointers. The value is "used" within the other `Check_NEImpl`
helper function, but the use is not visible in the current function.
Fix by modeling as if the LValueToRValue happened in this function.

PiperOrigin-RevId: 882897538
---
 nullability/test/check_macros.cc | 23 +++++++++++++++++++++++
 nullability/value_transferer.cc  | 12 +++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/nullability/test/check_macros.cc b/nullability/test/check_macros.cc
index 707071a7a..b106ea2dd 100644
--- a/nullability/test/check_macros.cc
+++ b/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.
diff --git a/nullability/value_transferer.cc b/nullability/value_transferer.cc
index fcbabae5d..bdf451590 100644
--- a/nullability/value_transferer.cc
+++ b/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