Skip to content

Commit dea08c2

Browse files
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
1 parent db973ce commit dea08c2

File tree

2 files changed

+54
-0
lines changed

2 files changed

+54
-0
lines changed

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2570,6 +2570,9 @@ RegionStoreManager::bind(LimitedRegionBindingsConstRef B, Loc L, SVal V) {
25702570
LimitedRegionBindingsRef
25712571
RegionStoreManager::setImplicitDefaultValue(LimitedRegionBindingsConstRef B,
25722572
const MemRegion *R, QualType T) {
2573+
if (B.hasExhaustedBindingLimit())
2574+
return B;
2575+
25732576
SVal V;
25742577

25752578
if (Loc::isLocType(T))
@@ -2596,6 +2599,8 @@ RegionStoreManager::setImplicitDefaultValue(LimitedRegionBindingsConstRef B,
25962599
std::optional<LimitedRegionBindingsRef> RegionStoreManager::tryBindSmallArray(
25972600
LimitedRegionBindingsConstRef B, const TypedValueRegion *R,
25982601
const ArrayType *AT, nonloc::LazyCompoundVal LCV) {
2602+
if (B.hasExhaustedBindingLimit())
2603+
return B.withValuesEscaped(LCV);
25992604

26002605
auto CAT = dyn_cast<ConstantArrayType>(AT);
26012606

@@ -2632,6 +2637,8 @@ RegionStoreManager::bindArray(LimitedRegionBindingsConstRef B,
26322637
const TypedValueRegion *R, SVal Init) {
26332638
llvm::TimeTraceScope TimeScope("RegionStoreManager::bindArray",
26342639
[R]() { return R->getDescriptiveName(); });
2640+
if (B.hasExhaustedBindingLimit())
2641+
return B.withValuesEscaped(Init);
26352642

26362643
const ArrayType *AT =cast<ArrayType>(Ctx.getCanonicalType(R->getValueType()));
26372644
QualType ElementTy = AT->getElementType();
@@ -2698,6 +2705,9 @@ RegionStoreManager::bindVector(LimitedRegionBindingsConstRef B,
26982705
const TypedValueRegion *R, SVal V) {
26992706
llvm::TimeTraceScope TimeScope("RegionStoreManager::bindVector",
27002707
[R]() { return R->getDescriptiveName(); });
2708+
if (B.hasExhaustedBindingLimit())
2709+
return B.withValuesEscaped(V);
2710+
27012711
QualType T = R->getValueType();
27022712
const VectorType *VT = T->castAs<VectorType>(); // Use castAs for typedefs.
27032713

@@ -2722,6 +2732,9 @@ RegionStoreManager::bindVector(LimitedRegionBindingsConstRef B,
27222732
if (VI == VE)
27232733
break;
27242734

2735+
if (NewB.hasExhaustedBindingLimit())
2736+
return NewB.withValuesEscaped(VI, VE);
2737+
27252738
NonLoc Idx = svalBuilder.makeArrayIndex(index);
27262739
const ElementRegion *ER = MRMgr.getElementRegion(ElemType, Idx, R, Ctx);
27272740

@@ -2758,6 +2771,9 @@ RegionStoreManager::getUniqueDefaultBinding(nonloc::LazyCompoundVal LCV) const {
27582771
std::optional<LimitedRegionBindingsRef> RegionStoreManager::tryBindSmallStruct(
27592772
LimitedRegionBindingsConstRef B, const TypedValueRegion *R,
27602773
const RecordDecl *RD, nonloc::LazyCompoundVal LCV) {
2774+
if (B.hasExhaustedBindingLimit())
2775+
return B.withValuesEscaped(LCV);
2776+
27612777
// If we try to copy a Conjured value representing the value of the whole
27622778
// struct, don't try to element-wise copy each field.
27632779
// That would unnecessarily bind Derived symbols slicing off the subregion for
@@ -2822,6 +2838,9 @@ RegionStoreManager::bindStruct(LimitedRegionBindingsConstRef B,
28222838
const TypedValueRegion *R, SVal V) {
28232839
llvm::TimeTraceScope TimeScope("RegionStoreManager::bindStruct",
28242840
[R]() { return R->getDescriptiveName(); });
2841+
if (B.hasExhaustedBindingLimit())
2842+
return B.withValuesEscaped(V);
2843+
28252844
QualType T = R->getValueType();
28262845
assert(T->isStructureOrClassType());
28272846

@@ -2931,6 +2950,9 @@ RegionStoreManager::bindStruct(LimitedRegionBindingsConstRef B,
29312950
++VI;
29322951
}
29332952

2953+
if (NewB.hasExhaustedBindingLimit())
2954+
return NewB.withValuesEscaped(VI, VE);
2955+
29342956
// There may be fewer values in the initialize list than the fields of struct.
29352957
if (FI != FE) {
29362958
NewB = NewB.addBinding(R, BindingKey::Default,
@@ -2945,6 +2967,9 @@ RegionStoreManager::bindAggregate(LimitedRegionBindingsConstRef B,
29452967
const TypedRegion *R, SVal Val) {
29462968
llvm::TimeTraceScope TimeScope("RegionStoreManager::bindAggregate",
29472969
[R]() { return R->getDescriptiveName(); });
2970+
if (B.hasExhaustedBindingLimit())
2971+
return B.withValuesEscaped(Val);
2972+
29482973
// Remove the old bindings, using 'R' as the root of all regions
29492974
// we will invalidate. Then add the new binding.
29502975
return removeSubRegionBindings(B, R).addBinding(R, BindingKey::Default, Val);

clang/test/Analysis/region-store.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,3 +386,32 @@ void tooManyFnArgumentsWhenInlining() {
386386
10,11,12,13,14,15,16,17,18,19,
387387
});
388388
}
389+
390+
void gh129211_assertion() {
391+
struct Clazz {
392+
int b;
393+
int : 0;
394+
};
395+
396+
Clazz d[][5][5] = {
397+
{
398+
{{}, {}, {}, {}, {}},
399+
{{}, {}, {}, {}, {}},
400+
{{}, {}, {}, {}, {}},
401+
{{}, {}, {}, {}, {}},
402+
{{}, {}, {}, {}, {}}
403+
},
404+
{
405+
{{}, {}, {}, {}, {}},
406+
{{}, {}, {}, {}, {}},
407+
{{}, {}, {}, {}, {}},
408+
{{}, {}, {}, {}, {}},
409+
{{}, {}, {}, {}, {}},
410+
},
411+
{
412+
{{}, {}, {}, {}, {}},
413+
{{}, {}, {}, {}, {}},
414+
{{}, {}, {}, {}},
415+
}
416+
}; // no-crash
417+
}

0 commit comments

Comments
 (0)