Fix multi-level pointer narrowing

Author: cs01

clang/include/clang/Sema/Sema.h

@@ -1184,6 +1184,8 @@ class Sema final : public SemaBase {
   /// strict-nullability: Narrow a variable's type to non-null in the current scope.
   void NarrowVariableToNonNull(const VarDecl *VD);
 
+  void NarrowVariablePointeeToNonNull(const VarDecl *VD);
+
   /// strict-nullability: Get the narrowed type for a variable, if any.
   /// Returns the narrowed type if the variable has been narrowed in the
   /// current scope, otherwise returns an empty QualType.
@@ -1206,6 +1208,9 @@ class Sema final : public SemaBase {
   void CollectAndCheckedVariables(Expr *Cond,
                                   SmallVectorImpl<const VarDecl*> &Vars);
 
+  void CollectAndCheckedDereferences(Expr *Cond,
+                                     SmallVectorImpl<const VarDecl*> &Vars);
+
   /// strict-nullability: Check if a condition expression contains any function calls.
   /// This is used to determine if it's safe to apply narrowing from a condition,
   /// since function calls can invalidate pointers before we apply narrowing.

clang/lib/Parse/ParseStmt.cpp

@@ -1557,7 +1557,13 @@ StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) {
     for (const VarDecl *VD : Actions.AndExprCheckedVars) {
       Actions.NarrowVariableToNonNull(VD);
     }
-    Actions.AndExprCheckedVars.clear();  // Consume the list
+    Actions.AndExprCheckedVars.clear();
+
+    SmallVector<const VarDecl*, 8> DerefCheckedVars;
+    Actions.CollectAndCheckedDereferences(Cond.get().second, DerefCheckedVars);
+    for (const VarDecl *VD : DerefCheckedVars) {
+      Actions.NarrowVariablePointeeToNonNull(VD);
+    }
 
     // Also narrow the traditional null-checked variable (but not if condition has calls)
     if (CheckedVar && !IsNegatedCheck && !ConditionHasCalls) {
@@ -1902,6 +1908,12 @@ StmtResult Parser::ParseWhileStatement(SourceLocation *TrailingElseLoc,
       Actions.NarrowVariableToNonNull(VD);
     }
 
+    SmallVector<const VarDecl*, 8> DerefCheckedVars;
+    Actions.CollectAndCheckedDereferences(Cond.get().second, DerefCheckedVars);
+    for (const VarDecl *VD : DerefCheckedVars) {
+      Actions.NarrowVariablePointeeToNonNull(VD);
+    }
+
     // Handle dereferences in condition: while (*p == 'x')
     SmallVector<const VarDecl*, 4> DereferencedVars;
     Actions.CollectDereferencedVariables(Cond.get().second, DereferencedVars);
@@ -2409,6 +2421,12 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc,
       Actions.NarrowVariableToNonNull(VD);
     }
 
+    SmallVector<const VarDecl*, 8> DerefCheckedVars;
+    Actions.CollectAndCheckedDereferences(SecondPart.get().second, DerefCheckedVars);
+    for (const VarDecl *VD : DerefCheckedVars) {
+      Actions.NarrowVariablePointeeToNonNull(VD);
+    }
+
     // Handle dereferences in condition: for (; *p == 'x'; )
     SmallVector<const VarDecl*, 4> DereferencedVars;
     Actions.CollectDereferencedVariables(SecondPart.get().second, DereferencedVars);

clang/lib/Sema/Sema.cpp

@@ -729,26 +729,71 @@ void Sema::NarrowVariableToNonNull(const VarDecl *VD) {
     return;
 
   QualType OrigType = VD->getType();
-  std::optional<NullabilityKind> Nullability = OrigType->getNullability();
 
-  // Only narrow if the type is nullable
-  if (!Nullability || *Nullability != NullabilityKind::Nullable)
+  QualType CurrentType = GetNarrowedType(VD);
+  if (CurrentType.isNull())
+    CurrentType = OrigType;
+
+  std::optional<NullabilityKind> Nullability = CurrentType->getNullability();
+
+  if (Nullability && *Nullability == NullabilityKind::NonNull)
     return;
 
-  // Strip the existing nullability attribute first
-  QualType BaseType = OrigType;
+  QualType BaseType = CurrentType;
   (void)AttributedType::stripOuterNullability(BaseType);
 
-  // Create a non-null version of the type
   QualType NarrowedType = Context.getAttributedType(
       NullabilityKind::NonNull,
       BaseType,
       BaseType);
 
-  // Store it in the current scope
   NullabilityNarrowingScopes.back()[VD] = NarrowedType;
 }
 
+void Sema::NarrowVariablePointeeToNonNull(const VarDecl *VD) {
+  if (!getLangOpts().StrictNullability)
+    return;
+  if (!VD || NullabilityNarrowingScopes.empty())
+    return;
+
+  QualType CurrentType = GetNarrowedType(VD);
+  if (CurrentType.isNull())
+    CurrentType = VD->getType();
+
+  const PointerType *PtrType = CurrentType->getAs<PointerType>();
+  if (!PtrType)
+    return;
+
+  QualType PointeeType = PtrType->getPointeeType();
+  std::optional<NullabilityKind> PointeeNullability = PointeeType->getNullability();
+
+  if (PointeeNullability && *PointeeNullability == NullabilityKind::NonNull)
+    return;
+
+  QualType PointeeBase = PointeeType;
+  (void)AttributedType::stripOuterNullability(PointeeBase);
+
+  QualType NarrowedPointee = Context.getAttributedType(
+      NullabilityKind::NonNull,
+      PointeeBase,
+      PointeeBase);
+
+  std::optional<NullabilityKind> OuterNullability = CurrentType->getNullability();
+  QualType OuterBase = CurrentType;
+  (void)AttributedType::stripOuterNullability(OuterBase);
+
+  QualType NewPointerType = Context.getPointerType(NarrowedPointee);
+
+  if (OuterNullability) {
+    NewPointerType = Context.getAttributedType(
+        *OuterNullability,
+        NewPointerType,
+        NewPointerType);
+  }
+
+  NullabilityNarrowingScopes.back()[VD] = NewPointerType;
+}
+
 QualType Sema::GetNarrowedType(const VarDecl *VD) const {
   if (!getLangOpts().StrictNullability)
     return QualType();
@@ -826,10 +871,23 @@ const VarDecl* Sema::AnalyzeConditionForNullCheck(Expr *Cond, bool &IsNegated) {
     }
   }
 
+  // Handle: *ptr (dereferenced pointer)
+  if (auto *UO = dyn_cast<UnaryOperator>(E)) {
+    if (UO->getOpcode() == UO_Deref) {
+      Expr *SubExpr = UO->getSubExpr()->IgnoreParenImpCasts();
+      if (auto *DRE = dyn_cast<DeclRefExpr>(SubExpr)) {
+        if (auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
+          if (VD->getType()->isPointerType()) {
+            return VD;
+          }
+        }
+      }
+    }
+  }
+
   // Handle: ptr (implicit boolean conversion)
   if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
     if (auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
-      // Check if this is a pointer type
       if (VD->getType()->isPointerType()) {
         return VD;
       }
@@ -882,23 +940,55 @@ void Sema::CollectAndCheckedVariables(Expr *Cond,
   // Check if this is an AND expression
   if (auto *BO = dyn_cast<BinaryOperator>(E)) {
     if (BO->getOpcode() == BO_LAnd) {
-      // Recursively collect from both sides of the AND
       CollectAndCheckedVariables(BO->getLHS(), Vars);
       CollectAndCheckedVariables(BO->getRHS(), Vars);
       return;
     }
   }
 
-  // Otherwise, check if this is a simple non-negated null check
   bool IsNegated = false;
   const VarDecl *VD = AnalyzeConditionForNullCheck(Cond, IsNegated);
 
-  // For AND patterns like "p && q", we want non-negated checks
   if (VD && !IsNegated) {
     Vars.push_back(VD);
   }
 }
 
+void Sema::CollectAndCheckedDereferences(Expr *Cond,
+                                          SmallVectorImpl<const VarDecl*> &Vars) {
+  if (!Cond)
+    return;
+
+  Expr *E = Cond->IgnoreParenImpCasts();
+
+  if (auto *BO = dyn_cast<BinaryOperator>(E)) {
+    if (BO->getOpcode() == BO_LAnd) {
+      CollectAndCheckedDereferences(BO->getLHS(), Vars);
+      CollectAndCheckedDereferences(BO->getRHS(), Vars);
+      return;
+    }
+  }
+
+  if (auto *UO = dyn_cast<UnaryOperator>(E)) {
+    if (UO->getOpcode() == UO_LNot) {
+      E = UO->getSubExpr()->IgnoreParenImpCasts();
+    }
+  }
+
+  if (auto *UO = dyn_cast<UnaryOperator>(E)) {
+    if (UO->getOpcode() == UO_Deref) {
+      Expr *SubExpr = UO->getSubExpr()->IgnoreParenImpCasts();
+      if (auto *DRE = dyn_cast<DeclRefExpr>(SubExpr)) {
+        if (auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
+          if (VD->getType()->isPointerType()) {
+            Vars.push_back(VD);
+          }
+        }
+      }
+    }
+  }
+}
+
 // strict-nullability: Check if an expression contains any function calls that could have side effects
 // This is used to determine if it's safe to apply narrowing from a condition
 static bool ContainsSideEffectingCall(Expr *E) {

clang/lib/Sema/SemaExpr.cpp

@@ -14900,35 +14900,41 @@ static QualType CheckIndirectionOperand(Sema &S, Expr *Op, ExprValueKind &VK,
       if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
         QualType NarrowedType = S.GetNarrowedType(VD);
         if (!NarrowedType.isNull()) {
-          // Use the narrowed type for nullability checking
           CheckType = NarrowedType;
         }
       }
     }
-    // strict-nullability: Also check if this is an increment/decrement of a narrowed variable
-    // For *p++, Op is the UnaryOperator(++), and its sub-expression is the variable
     else if (const auto *UO = dyn_cast<UnaryOperator>(Op->IgnoreParenImpCasts())) {
-      if (UO->getOpcode() == UO_PostInc || UO->getOpcode() == UO_PreInc ||
+      if (UO->getOpcode() == UO_Deref) {
+        Expr *DerefSubExpr = UO->getSubExpr()->IgnoreParenImpCasts();
+        if (const auto *DRE = dyn_cast<DeclRefExpr>(DerefSubExpr)) {
+          if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
+            QualType NarrowedType = S.GetNarrowedType(VD);
+            if (!NarrowedType.isNull()) {
+              if (const PointerType *PT = NarrowedType->getAs<PointerType>()) {
+                CheckType = PT->getPointeeType();
+              }
+            }
+          }
+        }
+      }
+      else if (UO->getOpcode() == UO_PostInc || UO->getOpcode() == UO_PreInc ||
           UO->getOpcode() == UO_PostDec || UO->getOpcode() == UO_PreDec) {
-        // Get the variable being incremented/decremented
         if (const auto *DRE = dyn_cast<DeclRefExpr>(UO->getSubExpr()->IgnoreParenImpCasts())) {
           if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
             QualType NarrowedType = S.GetNarrowedType(VD);
             if (!NarrowedType.isNull()) {
-              // The result of p++ is the old value of p, which had the narrowed type
               CheckType = NarrowedType;
             }
           }
         }
       }
     }
 
-    if (auto Nullability = CheckType->getNullability()) {
-      if (*Nullability == NullabilityKind::Nullable) {
-        // strict-nullability: Warn about dereferencing nullable pointers.
-        // Dereferencing does NOT perform a null-check - it will crash if null!
-        S.Diag(OpLoc, diag::warn_strict_nullability_dereference) << OpTy;
-      }
+    auto Nullability = CheckType->getNullability();
+    if (!Nullability || *Nullability == NullabilityKind::Nullable ||
+        *Nullability == NullabilityKind::Unspecified) {
+      S.Diag(OpLoc, diag::warn_strict_nullability_dereference) << OpTy;
     }
   }
 

clang/test/Sema/strict-nullability.c

@@ -404,14 +404,6 @@ void test_chained_deref(int** pp) {
         **pp = 42;  // OK - both levels narrowed
     }
 }
-
-// Test: Triple pointers
-void test_triple_pointers(int*** ppp) {
-    if (ppp && *ppp && **ppp) {
-        ***ppp = 42;  // OK - all three levels narrowed
-    }
-}
-
 // Test: Declared nonnull multi-level
 void test_nonnull_outer_ptr(int* * _Nonnull pp) {
     *pp = 0;  // OK - pp is nonnull by declaration
@@ -627,3 +619,31 @@ void test_volatile_with_check(void) {
     }
 }
 
+void test_double_deref_no_check(int** pp) {
+    **pp = 42; // expected-warning{{dereferencing nullable pointer of type 'int **'}}
+               // expected-warning@-1{{dereferencing nullable pointer of type 'int *'}}
+}
+
+void test_double_deref_both_checked(int** pp) {
+    if (pp && *pp) {
+        **pp = 42;
+    }
+}
+
+void test_double_deref_only_outer_checked(int** pp) {
+    if (pp) {
+        **pp = 42; // expected-warning{{dereferencing nullable pointer of type 'int *'}}
+    }
+}
+
+void test_double_deref_nonnull_inner(int* _Nonnull * pp) {
+    if (pp) {
+        **pp = 42;
+    }
+}
+
+void test_double_deref_nonnull(int* _Nonnull *_Nonnull pp) {
+    **pp = 42;
+}
+
+

nullsafe-playground/examples/multi-level.c

@@ -1,10 +1,19 @@
-// Multi-level pointers require careful handling
-void deref_twice(int** pp) {
-    **pp = 42;  // warning - *pp might be null!
+void deref_twice_unsafe(int**  pp) {
+    **pp = 42;  // Error
 }
 
-void safe_deref(int** pp) {
+void deref_twice_safe(int**  pp) {
     if (pp && *pp) {
         **pp = 42;  // OK
     }
+}
+
+void deref_twice_partial(int**  pp) {
+    if (pp) {
+        **pp = 42;  // Error
+    }
+}
+
+void example_nonnull_inner(int * _Nonnull * _Nonnull pp) {
+    **pp = 42;  // OK
 }

nullsafe-playground/examples/nonnull-annotation.c

@@ -1,9 +1,24 @@
-// _Nonnull annotations guarantee non-null
-void process(_Nonnull int* data) {
-    *data = 42;  // OK - data is guaranteed non-null
+void process(int* _Nonnull data) {
+    *data = 42;
 }
 
-void example(int* _Nullable x, int* _Nonnull y) {
-    process(x);  // warning - passing nullable to nonnull
-    process(y);  // OK
+void example(int* _Nullable  x, int* _Nonnull y) {
+    process(x);
+    process(y);
+}
+
+void deref_twice_unsafe(int* _Nullable * _Nullable pp) {
+    **pp = 42;
+}
+
+void deref_twice_safe(int* _Nullable * _Nullable pp) {
+    if (pp && *pp) {
+        **pp = 42;
+    }
+}
+
+void deref_twice_partial(int* _Nullable * _Nullable pp) {
+    if (pp) {
+        **pp = 42;
+    }
 }