[clang] dea08c2 - Fix RegionStore assertion failure after #127602 (#129224)

[via cfe-commits]

Author: Balázs Benics
Date: 2025-02-28T15:48:31+01:00
New Revision: dea08c2b67f38dba707003374f41b2277ab564d4

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

LOG: Fix RegionStore assertion failure after #127602 (#129224)

Basically, we may leave the loop because if exhaust the fields, array
elements or other subobjects to initialize.
In that case, the Bindings may be in an exhausted state, thus no further
addBinding calls are allowed.

Let's harden the code by sprinkling some early exists in the recursive
dispatcher functions.
And to actually fix the issue, I added a check guarding the single
unguarded addBinding right after a loop I mentioned.

Fixes #129211

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Core/RegionStore.cpp
    clang/test/Analysis/region-store.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 620fc117c6789..550a276c66c71 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2570,6 +2570,9 @@ RegionStoreManager::bind(LimitedRegionBindingsConstRef B, Loc L, SVal V) {
 LimitedRegionBindingsRef
 RegionStoreManager::setImplicitDefaultValue(LimitedRegionBindingsConstRef B,
                                             const MemRegion *R, QualType T) {
+  if (B.hasExhaustedBindingLimit())
+    return B;
+
   SVal V;
 
   if (Loc::isLocType(T))
@@ -2596,6 +2599,8 @@ RegionStoreManager::setImplicitDefaultValue(LimitedRegionBindingsConstRef B,
 std::optional<LimitedRegionBindingsRef> RegionStoreManager::tryBindSmallArray(
     LimitedRegionBindingsConstRef B, const TypedValueRegion *R,
     const ArrayType *AT, nonloc::LazyCompoundVal LCV) {
+  if (B.hasExhaustedBindingLimit())
+    return B.withValuesEscaped(LCV);
 
   auto CAT = dyn_cast<ConstantArrayType>(AT);
 
@@ -2632,6 +2637,8 @@ RegionStoreManager::bindArray(LimitedRegionBindingsConstRef B,
                               const TypedValueRegion *R, SVal Init) {
   llvm::TimeTraceScope TimeScope("RegionStoreManager::bindArray",
                                  [R]() { return R->getDescriptiveName(); });
+  if (B.hasExhaustedBindingLimit())
+    return B.withValuesEscaped(Init);
 
   const ArrayType *AT =cast<ArrayType>(Ctx.getCanonicalType(R->getValueType()));
   QualType ElementTy = AT->getElementType();
@@ -2698,6 +2705,9 @@ RegionStoreManager::bindVector(LimitedRegionBindingsConstRef B,
                                const TypedValueRegion *R, SVal V) {
   llvm::TimeTraceScope TimeScope("RegionStoreManager::bindVector",
                                  [R]() { return R->getDescriptiveName(); });
+  if (B.hasExhaustedBindingLimit())
+    return B.withValuesEscaped(V);
+
   QualType T = R->getValueType();
   const VectorType *VT = T->castAs<VectorType>(); // Use castAs for typedefs.
 
@@ -2722,6 +2732,9 @@ RegionStoreManager::bindVector(LimitedRegionBindingsConstRef B,
     if (VI == VE)
       break;
 
+    if (NewB.hasExhaustedBindingLimit())
+      return NewB.withValuesEscaped(VI, VE);
+
     NonLoc Idx = svalBuilder.makeArrayIndex(index);
     const ElementRegion *ER = MRMgr.getElementRegion(ElemType, Idx, R, Ctx);
 
@@ -2758,6 +2771,9 @@ RegionStoreManager::getUniqueDefaultBinding(nonloc::LazyCompoundVal LCV) const {
 std::optional<LimitedRegionBindingsRef> RegionStoreManager::tryBindSmallStruct(
     LimitedRegionBindingsConstRef B, const TypedValueRegion *R,
     const RecordDecl *RD, nonloc::LazyCompoundVal LCV) {
+  if (B.hasExhaustedBindingLimit())
+    return B.withValuesEscaped(LCV);
+
   // If we try to copy a Conjured value representing the value of the whole
   // struct, don't try to element-wise copy each field.
   // That would unnecessarily bind Derived symbols slicing off the subregion for
@@ -2822,6 +2838,9 @@ RegionStoreManager::bindStruct(LimitedRegionBindingsConstRef B,
                                const TypedValueRegion *R, SVal V) {
   llvm::TimeTraceScope TimeScope("RegionStoreManager::bindStruct",
                                  [R]() { return R->getDescriptiveName(); });
+  if (B.hasExhaustedBindingLimit())
+    return B.withValuesEscaped(V);
+
   QualType T = R->getValueType();
   assert(T->isStructureOrClassType());
 
@@ -2931,6 +2950,9 @@ RegionStoreManager::bindStruct(LimitedRegionBindingsConstRef B,
     ++VI;
   }
 
+  if (NewB.hasExhaustedBindingLimit())
+    return NewB.withValuesEscaped(VI, VE);
+
   // There may be fewer values in the initialize list than the fields of struct.
   if (FI != FE) {
     NewB = NewB.addBinding(R, BindingKey::Default,
@@ -2945,6 +2967,9 @@ RegionStoreManager::bindAggregate(LimitedRegionBindingsConstRef B,
                                   const TypedRegion *R, SVal Val) {
   llvm::TimeTraceScope TimeScope("RegionStoreManager::bindAggregate",
                                  [R]() { return R->getDescriptiveName(); });
+  if (B.hasExhaustedBindingLimit())
+    return B.withValuesEscaped(Val);
+
   // Remove the old bindings, using 'R' as the root of all regions
   // we will invalidate. Then add the new binding.
   return removeSubRegionBindings(B, R).addBinding(R, BindingKey::Default, Val);

diff  --git a/clang/test/Analysis/region-store.cpp b/clang/test/Analysis/region-store.cpp
index 9e80a2e688575..cb3313cbbb313 100644
--- a/clang/test/Analysis/region-store.cpp
+++ b/clang/test/Analysis/region-store.cpp
@@ -386,3 +386,32 @@ void tooManyFnArgumentsWhenInlining() {
     10,11,12,13,14,15,16,17,18,19,
   });
 }
+
+void gh129211_assertion() {
+  struct Clazz {
+    int b;
+    int : 0;
+  };
+
+  Clazz d[][5][5] = {
+    {
+      {{}, {}, {}, {}, {}},
+      {{}, {}, {}, {}, {}},
+      {{}, {}, {}, {}, {}},
+      {{}, {}, {}, {}, {}},
+      {{}, {}, {}, {}, {}}
+    },
+    {
+      {{}, {}, {}, {}, {}},
+      {{}, {}, {}, {}, {}},
+      {{}, {}, {}, {}, {}},
+      {{}, {}, {}, {}, {}},
+      {{}, {}, {}, {}, {}},
+    },
+    {
+      {{}, {}, {}, {}, {}},
+      {{}, {}, {}, {}, {}},
+      {{}, {}, {}, {}},
+    }
+  }; // no-crash
+}