Skip to content

Commit d63cc4c

Browse files
authored
[analyzer] Unknown array lvalue element in Store (#133381)
Remove the early return for BaseRegions of type ElementRegion. Return meaningful MemRegionVal for these cases as well. Previous discussion: https://discourse.llvm.org/t/lvalueelement-returns-unknownval-for-multi-dimensional-arrays/85476
1 parent fff8f03 commit d63cc4c

File tree

3 files changed

+33
-11
lines changed

3 files changed

+33
-11
lines changed

clang/lib/StaticAnalyzer/Core/Store.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -511,13 +511,9 @@ SVal StoreManager::getLValueElement(QualType elementType, NonLoc Offset,
511511
// Only allow non-integer offsets if the base region has no offset itself.
512512
// FIXME: This is a somewhat arbitrary restriction. We should be using
513513
// SValBuilder here to add the two offsets without checking their types.
514-
if (!isa<nonloc::ConcreteInt>(Offset)) {
515-
if (isa<ElementRegion>(BaseRegion->StripCasts()))
516-
return UnknownVal();
517-
514+
if (!isa<nonloc::ConcreteInt>(Offset))
518515
return loc::MemRegionVal(MRMgr.getElementRegion(
519516
elementType, Offset, cast<SubRegion>(ElemR->getSuperRegion()), Ctx));
520-
}
521517

522518
const llvm::APSInt& OffI = Offset.castAs<nonloc::ConcreteInt>().getValue();
523519
assert(BaseIdxI.isSigned());

clang/test/Analysis/ArrayBound/assumption-reporting.c

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,9 @@ int assumingBothPointerToMiddle(int arg) {
3939
// will speak about the "byte offset" measured from the beginning of the TenElements.
4040
int *p = TenElements + 2;
4141
int a = p[arg];
42-
// FIXME: The following note does not appear:
43-
// {{Assuming byte offset is non-negative and less than 40, the extent of 'TenElements'}}
44-
// It seems that the analyzer "gives up" modeling this pointer arithmetics
45-
// and says that `p[arg]` is just an UnknownVal (instead of calculating that
46-
// it's equivalent to `TenElements[2+arg]`).
42+
// expected-note@-1 {{Assuming byte offset is non-negative and less than 40, the extent of 'TenElements'}}
4743

4844
int b = TenElements[arg]; // This is normal access, and only the lower bound is new.
49-
// expected-note@-1 {{Assuming index is non-negative}}
5045
int c = TenElements[arg + 10];
5146
// expected-warning@-1 {{Out of bound access to memory after the end of 'TenElements'}}
5247
// expected-note@-2 {{Access of 'TenElements' at an overflowing index, while it holds only 10 'int' elements}}

clang/test/Analysis/lvalue_elements.c

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// RUN: %clang_analyze_cc1 -std=c11 -analyzer-checker=debug.ExprInspection -verify %s
2+
3+
void clang_analyzer_dump(int*);
4+
5+
const int const_index = 1;
6+
extern int unknown_index;
7+
extern int array[3];
8+
extern int matrix[3][3];
9+
10+
int main(){
11+
12+
// expected-warning@+1 {{&Element{array,1 S64b,int}}}
13+
clang_analyzer_dump(&array[const_index]);
14+
15+
// expected-warning@+1 {{&Element{array,reg_$1<int unknown_index>,int}}}
16+
clang_analyzer_dump(&array[unknown_index]);
17+
18+
// expected-warning@+1 {{&Element{Element{matrix,1 S64b,int[3]},1 S64b,int}}}
19+
clang_analyzer_dump(&matrix[const_index][const_index]);
20+
21+
// expected-warning@+1 {{&Element{Element{matrix,reg_$1<int unknown_index>,int[3]},1 S64b,int}}}
22+
clang_analyzer_dump(&matrix[unknown_index][const_index]);
23+
24+
// expected-warning@+1 {{&Element{Element{matrix,1 S64b,int[3]},reg_$1<int unknown_index>,int}}}
25+
clang_analyzer_dump(&matrix[const_index][unknown_index]);
26+
27+
// expected-warning@+1 {{&Element{Element{matrix,reg_$1<int unknown_index>,int[3]},reg_$1<int unknown_index>,int}}}
28+
clang_analyzer_dump(&matrix[unknown_index][unknown_index]);
29+
30+
return 0;
31+
}

0 commit comments

Comments
 (0)