Skip to content

Commit 1255906

Browse files
danix800danix800
committed
[analyzer] Fix a few size-type inconsistency relating to DynamicExtent
Size-type inconsistency (signedness) causes confusion and even bugs. For example when signed compared to unsigned the result might not be expected. Summary of this commit: Related APIs changes: 1. getDynamicExtent() returns signed version of extent; 2. Add getDynamicElementCountWithOffset() for offset version of element count; 3. getElementExtent() could be 0, add defensive checking for getDynamicElementCount(), if element is of zero-length, try ConstantArrayType::getSize() as element count; Related checker changes: 1. ArrayBoundCheckerV2: add testcase for signed <-> unsigned comparison from type-inconsistency results by getDynamicExtent() 2. ExprInspection: use more general API to report more results Fixes #64920 Reviewed By: donat.nagy, steakhal Differential Revision: https://reviews.llvm.org/D158499
1 parent 4b9c2cf commit 1255906

File tree

6 files changed

+169
-52
lines changed

6 files changed

+169
-52
lines changed

‎clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ ProgramStateRef setDynamicExtent(ProgramStateRef State, const MemRegion *MR,
5353
/// (bufptr) // extent is unknown
5454
SVal getDynamicExtentWithOffset(ProgramStateRef State, SVal BufV);
5555

56+
/// \returns The stored element count of the region represented by a symbolic
57+
/// value \p BufV.
58+
DefinedOrUnknownSVal getDynamicElementCountWithOffset(ProgramStateRef State,
59+
SVal BufV, QualType Ty);
60+
5661
} // namespace ento
5762
} // namespace clang
5863

‎clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -325,25 +325,25 @@ void ExprInspectionChecker::analyzerDump(const CallExpr *CE,
325325

326326
void ExprInspectionChecker::analyzerGetExtent(const CallExpr *CE,
327327
CheckerContext &C) const {
328-
const MemRegion *MR = getArgRegion(CE, C);
329-
if (!MR)
328+
const Expr *Arg = getArgExpr(CE, C);
329+
if (!Arg)
330330
return;
331331

332332
ProgramStateRef State = C.getState();
333-
DefinedOrUnknownSVal Size = getDynamicExtent(State, MR, C.getSValBuilder());
333+
SVal Size = getDynamicExtentWithOffset(State, C.getSVal(Arg));
334334

335335
State = State->BindExpr(CE, C.getLocationContext(), Size);
336336
C.addTransition(State);
337337
}
338338

339339
void ExprInspectionChecker::analyzerDumpExtent(const CallExpr *CE,
340340
CheckerContext &C) const {
341-
const MemRegion *MR = getArgRegion(CE, C);
342-
if (!MR)
341+
const Expr *Arg = getArgExpr(CE, C);
342+
if (!Arg)
343343
return;
344344

345-
DefinedOrUnknownSVal Size =
346-
getDynamicExtent(C.getState(), MR, C.getSValBuilder());
345+
ProgramStateRef State = C.getState();
346+
SVal Size = getDynamicExtentWithOffset(State, C.getSVal(Arg));
347347
printAndReport(C, Size);
348348
}
349349

@@ -362,8 +362,8 @@ void ExprInspectionChecker::analyzerDumpElementCount(const CallExpr *CE,
362362

363363
assert(!ElementTy->isPointerType());
364364

365-
DefinedOrUnknownSVal ElementCount =
366-
getDynamicElementCount(C.getState(), MR, C.getSValBuilder(), ElementTy);
365+
DefinedOrUnknownSVal ElementCount = getDynamicElementCountWithOffset(
366+
C.getState(), C.getSVal(getArgExpr(CE, C)), ElementTy);
367367
printAndReport(C, ElementCount);
368368
}
369369

‎clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp

Lines changed: 56 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ DefinedOrUnknownSVal getDynamicExtent(ProgramStateRef State,
3030
MR = MR->StripCasts();
3131

3232
if (const DefinedOrUnknownSVal *Size = State->get<DynamicExtentMap>(MR))
33-
return *Size;
33+
if (auto SSize =
34+
SVB.convertToArrayIndex(*Size).getAs<DefinedOrUnknownSVal>())
35+
return *SSize;
3436

3537
return MR->getMemRegionManager().getStaticSize(MR, SVB);
3638
}
@@ -40,24 +42,49 @@ DefinedOrUnknownSVal getElementExtent(QualType Ty, SValBuilder &SVB) {
4042
SVB.getArrayIndexType());
4143
}
4244

45+
static DefinedOrUnknownSVal getConstantArrayElementCount(SValBuilder &SVB,
46+
const MemRegion *MR) {
47+
MR = MR->StripCasts();
48+
49+
const auto *TVR = MR->getAs<TypedValueRegion>();
50+
if (!TVR)
51+
return UnknownVal();
52+
53+
if (const ConstantArrayType *CAT =
54+
SVB.getContext().getAsConstantArrayType(TVR->getValueType()))
55+
return SVB.makeIntVal(CAT->getSize(), /* isUnsigned = */ false);
56+
57+
return UnknownVal();
58+
}
59+
60+
static DefinedOrUnknownSVal
61+
getDynamicElementCount(ProgramStateRef State, SVal Size,
62+
DefinedOrUnknownSVal ElementSize) {
63+
SValBuilder &SVB = State->getStateManager().getSValBuilder();
64+
65+
auto ElementCount =
66+
SVB.evalBinOp(State, BO_Div, Size, ElementSize, SVB.getArrayIndexType())
67+
.getAs<DefinedOrUnknownSVal>();
68+
return ElementCount.value_or(UnknownVal());
69+
}
70+
4371
DefinedOrUnknownSVal getDynamicElementCount(ProgramStateRef State,
4472
const MemRegion *MR,
4573
SValBuilder &SVB,
4674
QualType ElementTy) {
4775
assert(MR != nullptr && "Not-null region expected");
4876
MR = MR->StripCasts();
4977

50-
DefinedOrUnknownSVal Size = getDynamicExtent(State, MR, SVB);
51-
SVal ElementSize = getElementExtent(ElementTy, SVB);
78+
DefinedOrUnknownSVal ElementSize = getElementExtent(ElementTy, SVB);
79+
if (ElementSize.isZeroConstant())
80+
return getConstantArrayElementCount(SVB, MR);
5281

53-
SVal ElementCount =
54-
SVB.evalBinOp(State, BO_Div, Size, ElementSize, SVB.getArrayIndexType());
55-
56-
return ElementCount.castAs<DefinedOrUnknownSVal>();
82+
return getDynamicElementCount(State, getDynamicExtent(State, MR, SVB),
83+
ElementSize);
5784
}
5885

5986
SVal getDynamicExtentWithOffset(ProgramStateRef State, SVal BufV) {
60-
SValBuilder &SvalBuilder = State->getStateManager().getSValBuilder();
87+
SValBuilder &SVB = State->getStateManager().getSValBuilder();
6188
const MemRegion *MRegion = BufV.getAsRegion();
6289
if (!MRegion)
6390
return UnknownVal();
@@ -68,15 +95,28 @@ SVal getDynamicExtentWithOffset(ProgramStateRef State, SVal BufV) {
6895
if (!BaseRegion)
6996
return UnknownVal();
7097

71-
NonLoc OffsetInBytes = SvalBuilder.makeArrayIndex(
72-
Offset.getOffset() /
73-
MRegion->getMemRegionManager().getContext().getCharWidth());
74-
DefinedOrUnknownSVal ExtentInBytes =
75-
getDynamicExtent(State, BaseRegion, SvalBuilder);
98+
NonLoc OffsetInChars =
99+
SVB.makeArrayIndex(Offset.getOffset() / SVB.getContext().getCharWidth());
100+
DefinedOrUnknownSVal ExtentInBytes = getDynamicExtent(State, BaseRegion, SVB);
101+
102+
return SVB.evalBinOp(State, BinaryOperator::Opcode::BO_Sub, ExtentInBytes,
103+
OffsetInChars, SVB.getArrayIndexType());
104+
}
105+
106+
DefinedOrUnknownSVal getDynamicElementCountWithOffset(ProgramStateRef State,
107+
SVal BufV,
108+
QualType ElementTy) {
109+
const MemRegion *MR = BufV.getAsRegion();
110+
if (!MR)
111+
return UnknownVal();
112+
113+
SValBuilder &SVB = State->getStateManager().getSValBuilder();
114+
DefinedOrUnknownSVal ElementSize = getElementExtent(ElementTy, SVB);
115+
if (ElementSize.isZeroConstant())
116+
return getConstantArrayElementCount(SVB, MR);
76117

77-
return SvalBuilder.evalBinOp(State, BinaryOperator::Opcode::BO_Sub,
78-
ExtentInBytes, OffsetInBytes,
79-
SvalBuilder.getArrayIndexType());
118+
return getDynamicElementCount(State, getDynamicExtentWithOffset(State, BufV),
119+
ElementSize);
80120
}
81121

82122
ProgramStateRef setDynamicExtent(ProgramStateRef State, const MemRegion *MR,

‎clang/test/Analysis/array-bound-v2-constraint-check.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.security.ArrayBoundV2,debug.ExprInspection \
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,alpha.security.ArrayBoundV2,debug.ExprInspection \
22
// RUN: -analyzer-config eagerly-assume=false -verify %s
33

44
void clang_analyzer_eval(int);
55
void clang_analyzer_printState(void);
66

7-
typedef unsigned long long size_t;
7+
typedef typeof(sizeof(int)) size_t;
88
const char a[] = "abcd"; // extent: 5 bytes
99

1010
void symbolic_size_t_and_int0(size_t len) {
@@ -83,6 +83,18 @@ void symbolic_longlong_and_int0(long long len) {
8383
clang_analyzer_eval(len <= 2); // expected-warning {{UNKNOWN}}
8484
}
8585

86+
void *malloc(size_t);
87+
void free(void *);
88+
void symbolic_longlong_and_int0_dynamic_extent(long long len) {
89+
char *b = malloc(5);
90+
(void)b[len + 1]; // no-warning
91+
// len: [-1,3]
92+
clang_analyzer_eval(-1 <= len && len <= 3); // expected-warning {{TRUE}}
93+
clang_analyzer_eval(0 <= len); // expected-warning {{UNKNOWN}}
94+
clang_analyzer_eval(len <= 2); // expected-warning {{UNKNOWN}}
95+
free(b);
96+
}
97+
8698
void symbolic_longlong_and_int1(long long len) {
8799
(void)a[len]; // no-warning
88100
// len: [0,4]

‎clang/test/Analysis/flexible-array-members.c

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,20 +44,52 @@ void test_incomplete_array_fam(void) {
4444
clang_analyzer_dump(clang_analyzer_getExtent(&fam));
4545
clang_analyzer_dump(clang_analyzer_getExtent(fam.data));
4646
// expected-warning@-2 {{4 S64b}}
47-
// expected-warning@-2 {{Unknown}}
47+
// expected-warning@-2 {{0 S64b}}
4848

4949
FAM *p = (FAM *)alloca(sizeof(FAM));
5050
clang_analyzer_dump(clang_analyzer_getExtent(p));
5151
clang_analyzer_dump(clang_analyzer_getExtent(p->data));
52-
// expected-warning@-2 {{4 U64b}}
53-
// expected-warning@-2 {{Unknown}}
52+
// expected-warning@-2 {{4 S64b}}
53+
// expected-warning@-2 {{0 S64b}}
5454

5555
FAM *q = (FAM *)malloc(sizeof(FAM));
5656
clang_analyzer_dump(clang_analyzer_getExtent(q));
5757
clang_analyzer_dump(clang_analyzer_getExtent(q->data));
58-
// expected-warning@-2 {{4 U64b}}
59-
// expected-warning@-2 {{Unknown}}
58+
// expected-warning@-2 {{4 S64b}}
59+
// expected-warning@-2 {{0 S64b}}
60+
free(q);
61+
62+
q = (FAM *)malloc(sizeof(FAM) + sizeof(int) * 2);
63+
clang_analyzer_dump(clang_analyzer_getExtent(q));
64+
clang_analyzer_dump(clang_analyzer_getExtent(q->data));
65+
// expected-warning@-2 {{12 S64b}}
66+
// expected-warning@-2 {{8 S64b}}
6067
free(q);
68+
69+
typedef struct __attribute__((packed)) {
70+
char c;
71+
int data[];
72+
} PackedFAM;
73+
74+
PackedFAM *t = (PackedFAM *)malloc(sizeof(PackedFAM) + sizeof(int) * 2);
75+
clang_analyzer_dump(clang_analyzer_getExtent(t));
76+
clang_analyzer_dump(clang_analyzer_getExtent(t->data));
77+
// expected-warning@-2 {{9 S64b}}
78+
// expected-warning@-2 {{8 S64b}}
79+
free(t);
80+
}
81+
82+
void test_too_small_base(void) {
83+
typedef struct FAM {
84+
long c;
85+
int data[];
86+
} FAM;
87+
short s = 0;
88+
FAM *p = (FAM *) &s;
89+
clang_analyzer_dump(clang_analyzer_getExtent(p));
90+
clang_analyzer_dump(clang_analyzer_getExtent(p->data));
91+
// expected-warning@-2 {{2 S64b}}
92+
// expected-warning@-2 {{-6 S64b}}
6193
}
6294

6395
void test_zero_length_array_fam(void) {
@@ -70,19 +102,19 @@ void test_zero_length_array_fam(void) {
70102
clang_analyzer_dump(clang_analyzer_getExtent(&fam));
71103
clang_analyzer_dump(clang_analyzer_getExtent(fam.data));
72104
// expected-warning@-2 {{4 S64b}}
73-
// expected-warning@-2 {{Unknown}}
105+
// expected-warning@-2 {{0 S64b}}
74106

75107
FAM *p = (FAM *)alloca(sizeof(FAM));
76108
clang_analyzer_dump(clang_analyzer_getExtent(p));
77109
clang_analyzer_dump(clang_analyzer_getExtent(p->data));
78-
// expected-warning@-2 {{4 U64b}}
79-
// expected-warning@-2 {{Unknown}}
110+
// expected-warning@-2 {{4 S64b}}
111+
// expected-warning@-2 {{0 S64b}}
80112

81113
FAM *q = (FAM *)malloc(sizeof(FAM));
82114
clang_analyzer_dump(clang_analyzer_getExtent(q));
83115
clang_analyzer_dump(clang_analyzer_getExtent(q->data));
84-
// expected-warning@-2 {{4 U64b}}
85-
// expected-warning@-2 {{Unknown}}
116+
// expected-warning@-2 {{4 S64b}}
117+
// expected-warning@-2 {{0 S64b}}
86118
free(q);
87119
}
88120

@@ -97,19 +129,19 @@ void test_single_element_array_possible_fam(void) {
97129
clang_analyzer_dump(clang_analyzer_getExtent(&likely_fam));
98130
clang_analyzer_dump(clang_analyzer_getExtent(likely_fam.data));
99131
// expected-warning@-2 {{8 S64b}}
100-
// expected-warning@-2 {{Unknown}}
132+
// expected-warning@-2 {{4 S64b}}
101133

102134
FAM *p = (FAM *)alloca(sizeof(FAM));
103135
clang_analyzer_dump(clang_analyzer_getExtent(p));
104136
clang_analyzer_dump(clang_analyzer_getExtent(p->data));
105-
// expected-warning@-2 {{8 U64b}}
106-
// expected-warning@-2 {{Unknown}}
137+
// expected-warning@-2 {{8 S64b}}
138+
// expected-warning@-2 {{4 S64b}}
107139

108140
FAM *q = (FAM *)malloc(sizeof(FAM));
109141
clang_analyzer_dump(clang_analyzer_getExtent(q));
110142
clang_analyzer_dump(clang_analyzer_getExtent(q->data));
111-
// expected-warning@-2 {{8 U64b}}
112-
// expected-warning@-2 {{Unknown}}
143+
// expected-warning@-2 {{8 S64b}}
144+
// expected-warning@-2 {{4 S64b}}
113145
free(q);
114146
#else
115147
FAM likely_fam;
@@ -121,13 +153,13 @@ void test_single_element_array_possible_fam(void) {
121153
FAM *p = (FAM *)alloca(sizeof(FAM));
122154
clang_analyzer_dump(clang_analyzer_getExtent(p));
123155
clang_analyzer_dump(clang_analyzer_getExtent(p->data));
124-
// expected-warning@-2 {{8 U64b}}
156+
// expected-warning@-2 {{8 S64b}}
125157
// expected-warning@-2 {{4 S64b}}
126158

127159
FAM *q = (FAM *)malloc(sizeof(FAM));
128160
clang_analyzer_dump(clang_analyzer_getExtent(q));
129161
clang_analyzer_dump(clang_analyzer_getExtent(q->data));
130-
// expected-warning@-2 {{8 U64b}}
162+
// expected-warning@-2 {{8 S64b}}
131163
// expected-warning@-2 {{4 S64b}}
132164
free(q);
133165
#endif

0 commit comments

Comments
 (0)