[clang] 1255906 - [analyzer] Fix a few size-type inconsistency relating to DynamicExtent
via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 1 06:05:58 PDT 2023
Author: dingfei
Date: 2023-09-01T21:03:16+08:00
New Revision: 12559064e05a11e8418425de59d8745f0cfb1122
URL: https://github.com/llvm/llvm-project/commit/12559064e05a11e8418425de59d8745f0cfb1122
DIFF: https://github.com/llvm/llvm-project/commit/12559064e05a11e8418425de59d8745f0cfb1122.diff
LOG: [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 https://github.com/llvm/llvm-project/issues/64920
Reviewed By: donat.nagy, steakhal
Differential Revision: https://reviews.llvm.org/D158499
Added:
Modified:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h
clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
clang/test/Analysis/array-bound-v2-constraint-check.c
clang/test/Analysis/flexible-array-members.c
clang/test/Analysis/memory-model.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h
index cfd7aa9664b696..50d5d254415af3 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h
@@ -53,6 +53,11 @@ ProgramStateRef setDynamicExtent(ProgramStateRef State, const MemRegion *MR,
/// (bufptr) // extent is unknown
SVal getDynamicExtentWithOffset(ProgramStateRef State, SVal BufV);
+/// \returns The stored element count of the region represented by a symbolic
+/// value \p BufV.
+DefinedOrUnknownSVal getDynamicElementCountWithOffset(ProgramStateRef State,
+ SVal BufV, QualType Ty);
+
} // namespace ento
} // namespace clang
diff --git a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
index 15ba29050e9032..2c7bacac33a687 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -325,12 +325,12 @@ void ExprInspectionChecker::analyzerDump(const CallExpr *CE,
void ExprInspectionChecker::analyzerGetExtent(const CallExpr *CE,
CheckerContext &C) const {
- const MemRegion *MR = getArgRegion(CE, C);
- if (!MR)
+ const Expr *Arg = getArgExpr(CE, C);
+ if (!Arg)
return;
ProgramStateRef State = C.getState();
- DefinedOrUnknownSVal Size = getDynamicExtent(State, MR, C.getSValBuilder());
+ SVal Size = getDynamicExtentWithOffset(State, C.getSVal(Arg));
State = State->BindExpr(CE, C.getLocationContext(), Size);
C.addTransition(State);
@@ -338,12 +338,12 @@ void ExprInspectionChecker::analyzerGetExtent(const CallExpr *CE,
void ExprInspectionChecker::analyzerDumpExtent(const CallExpr *CE,
CheckerContext &C) const {
- const MemRegion *MR = getArgRegion(CE, C);
- if (!MR)
+ const Expr *Arg = getArgExpr(CE, C);
+ if (!Arg)
return;
- DefinedOrUnknownSVal Size =
- getDynamicExtent(C.getState(), MR, C.getSValBuilder());
+ ProgramStateRef State = C.getState();
+ SVal Size = getDynamicExtentWithOffset(State, C.getSVal(Arg));
printAndReport(C, Size);
}
@@ -362,8 +362,8 @@ void ExprInspectionChecker::analyzerDumpElementCount(const CallExpr *CE,
assert(!ElementTy->isPointerType());
- DefinedOrUnknownSVal ElementCount =
- getDynamicElementCount(C.getState(), MR, C.getSValBuilder(), ElementTy);
+ DefinedOrUnknownSVal ElementCount = getDynamicElementCountWithOffset(
+ C.getState(), C.getSVal(getArgExpr(CE, C)), ElementTy);
printAndReport(C, ElementCount);
}
diff --git a/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp b/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
index 6a86536492cd3d..6cf06413b5372c 100644
--- a/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
@@ -30,7 +30,9 @@ DefinedOrUnknownSVal getDynamicExtent(ProgramStateRef State,
MR = MR->StripCasts();
if (const DefinedOrUnknownSVal *Size = State->get<DynamicExtentMap>(MR))
- return *Size;
+ if (auto SSize =
+ SVB.convertToArrayIndex(*Size).getAs<DefinedOrUnknownSVal>())
+ return *SSize;
return MR->getMemRegionManager().getStaticSize(MR, SVB);
}
@@ -40,6 +42,32 @@ DefinedOrUnknownSVal getElementExtent(QualType Ty, SValBuilder &SVB) {
SVB.getArrayIndexType());
}
+static DefinedOrUnknownSVal getConstantArrayElementCount(SValBuilder &SVB,
+ const MemRegion *MR) {
+ MR = MR->StripCasts();
+
+ const auto *TVR = MR->getAs<TypedValueRegion>();
+ if (!TVR)
+ return UnknownVal();
+
+ if (const ConstantArrayType *CAT =
+ SVB.getContext().getAsConstantArrayType(TVR->getValueType()))
+ return SVB.makeIntVal(CAT->getSize(), /* isUnsigned = */ false);
+
+ return UnknownVal();
+}
+
+static DefinedOrUnknownSVal
+getDynamicElementCount(ProgramStateRef State, SVal Size,
+ DefinedOrUnknownSVal ElementSize) {
+ SValBuilder &SVB = State->getStateManager().getSValBuilder();
+
+ auto ElementCount =
+ SVB.evalBinOp(State, BO_Div, Size, ElementSize, SVB.getArrayIndexType())
+ .getAs<DefinedOrUnknownSVal>();
+ return ElementCount.value_or(UnknownVal());
+}
+
DefinedOrUnknownSVal getDynamicElementCount(ProgramStateRef State,
const MemRegion *MR,
SValBuilder &SVB,
@@ -47,17 +75,16 @@ DefinedOrUnknownSVal getDynamicElementCount(ProgramStateRef State,
assert(MR != nullptr && "Not-null region expected");
MR = MR->StripCasts();
- DefinedOrUnknownSVal Size = getDynamicExtent(State, MR, SVB);
- SVal ElementSize = getElementExtent(ElementTy, SVB);
+ DefinedOrUnknownSVal ElementSize = getElementExtent(ElementTy, SVB);
+ if (ElementSize.isZeroConstant())
+ return getConstantArrayElementCount(SVB, MR);
- SVal ElementCount =
- SVB.evalBinOp(State, BO_Div, Size, ElementSize, SVB.getArrayIndexType());
-
- return ElementCount.castAs<DefinedOrUnknownSVal>();
+ return getDynamicElementCount(State, getDynamicExtent(State, MR, SVB),
+ ElementSize);
}
SVal getDynamicExtentWithOffset(ProgramStateRef State, SVal BufV) {
- SValBuilder &SvalBuilder = State->getStateManager().getSValBuilder();
+ SValBuilder &SVB = State->getStateManager().getSValBuilder();
const MemRegion *MRegion = BufV.getAsRegion();
if (!MRegion)
return UnknownVal();
@@ -68,15 +95,28 @@ SVal getDynamicExtentWithOffset(ProgramStateRef State, SVal BufV) {
if (!BaseRegion)
return UnknownVal();
- NonLoc OffsetInBytes = SvalBuilder.makeArrayIndex(
- Offset.getOffset() /
- MRegion->getMemRegionManager().getContext().getCharWidth());
- DefinedOrUnknownSVal ExtentInBytes =
- getDynamicExtent(State, BaseRegion, SvalBuilder);
+ NonLoc OffsetInChars =
+ SVB.makeArrayIndex(Offset.getOffset() / SVB.getContext().getCharWidth());
+ DefinedOrUnknownSVal ExtentInBytes = getDynamicExtent(State, BaseRegion, SVB);
+
+ return SVB.evalBinOp(State, BinaryOperator::Opcode::BO_Sub, ExtentInBytes,
+ OffsetInChars, SVB.getArrayIndexType());
+}
+
+DefinedOrUnknownSVal getDynamicElementCountWithOffset(ProgramStateRef State,
+ SVal BufV,
+ QualType ElementTy) {
+ const MemRegion *MR = BufV.getAsRegion();
+ if (!MR)
+ return UnknownVal();
+
+ SValBuilder &SVB = State->getStateManager().getSValBuilder();
+ DefinedOrUnknownSVal ElementSize = getElementExtent(ElementTy, SVB);
+ if (ElementSize.isZeroConstant())
+ return getConstantArrayElementCount(SVB, MR);
- return SvalBuilder.evalBinOp(State, BinaryOperator::Opcode::BO_Sub,
- ExtentInBytes, OffsetInBytes,
- SvalBuilder.getArrayIndexType());
+ return getDynamicElementCount(State, getDynamicExtentWithOffset(State, BufV),
+ ElementSize);
}
ProgramStateRef setDynamicExtent(ProgramStateRef State, const MemRegion *MR,
diff --git a/clang/test/Analysis/array-bound-v2-constraint-check.c b/clang/test/Analysis/array-bound-v2-constraint-check.c
index dc138196b56a94..91f748e655ce2d 100644
--- a/clang/test/Analysis/array-bound-v2-constraint-check.c
+++ b/clang/test/Analysis/array-bound-v2-constraint-check.c
@@ -1,10 +1,10 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.security.ArrayBoundV2,debug.ExprInspection \
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,alpha.security.ArrayBoundV2,debug.ExprInspection \
// RUN: -analyzer-config eagerly-assume=false -verify %s
void clang_analyzer_eval(int);
void clang_analyzer_printState(void);
-typedef unsigned long long size_t;
+typedef typeof(sizeof(int)) size_t;
const char a[] = "abcd"; // extent: 5 bytes
void symbolic_size_t_and_int0(size_t len) {
@@ -83,6 +83,18 @@ void symbolic_longlong_and_int0(long long len) {
clang_analyzer_eval(len <= 2); // expected-warning {{UNKNOWN}}
}
+void *malloc(size_t);
+void free(void *);
+void symbolic_longlong_and_int0_dynamic_extent(long long len) {
+ char *b = malloc(5);
+ (void)b[len + 1]; // no-warning
+ // len: [-1,3]
+ clang_analyzer_eval(-1 <= len && len <= 3); // expected-warning {{TRUE}}
+ clang_analyzer_eval(0 <= len); // expected-warning {{UNKNOWN}}
+ clang_analyzer_eval(len <= 2); // expected-warning {{UNKNOWN}}
+ free(b);
+}
+
void symbolic_longlong_and_int1(long long len) {
(void)a[len]; // no-warning
// len: [0,4]
diff --git a/clang/test/Analysis/flexible-array-members.c b/clang/test/Analysis/flexible-array-members.c
index dc131c6f1a437c..1318b35769b257 100644
--- a/clang/test/Analysis/flexible-array-members.c
+++ b/clang/test/Analysis/flexible-array-members.c
@@ -44,20 +44,52 @@ void test_incomplete_array_fam(void) {
clang_analyzer_dump(clang_analyzer_getExtent(&fam));
clang_analyzer_dump(clang_analyzer_getExtent(fam.data));
// expected-warning at -2 {{4 S64b}}
- // expected-warning at -2 {{Unknown}}
+ // expected-warning at -2 {{0 S64b}}
FAM *p = (FAM *)alloca(sizeof(FAM));
clang_analyzer_dump(clang_analyzer_getExtent(p));
clang_analyzer_dump(clang_analyzer_getExtent(p->data));
- // expected-warning at -2 {{4 U64b}}
- // expected-warning at -2 {{Unknown}}
+ // expected-warning at -2 {{4 S64b}}
+ // expected-warning at -2 {{0 S64b}}
FAM *q = (FAM *)malloc(sizeof(FAM));
clang_analyzer_dump(clang_analyzer_getExtent(q));
clang_analyzer_dump(clang_analyzer_getExtent(q->data));
- // expected-warning at -2 {{4 U64b}}
- // expected-warning at -2 {{Unknown}}
+ // expected-warning at -2 {{4 S64b}}
+ // expected-warning at -2 {{0 S64b}}
+ free(q);
+
+ q = (FAM *)malloc(sizeof(FAM) + sizeof(int) * 2);
+ clang_analyzer_dump(clang_analyzer_getExtent(q));
+ clang_analyzer_dump(clang_analyzer_getExtent(q->data));
+ // expected-warning at -2 {{12 S64b}}
+ // expected-warning at -2 {{8 S64b}}
free(q);
+
+ typedef struct __attribute__((packed)) {
+ char c;
+ int data[];
+ } PackedFAM;
+
+ PackedFAM *t = (PackedFAM *)malloc(sizeof(PackedFAM) + sizeof(int) * 2);
+ clang_analyzer_dump(clang_analyzer_getExtent(t));
+ clang_analyzer_dump(clang_analyzer_getExtent(t->data));
+ // expected-warning at -2 {{9 S64b}}
+ // expected-warning at -2 {{8 S64b}}
+ free(t);
+}
+
+void test_too_small_base(void) {
+ typedef struct FAM {
+ long c;
+ int data[];
+ } FAM;
+ short s = 0;
+ FAM *p = (FAM *) &s;
+ clang_analyzer_dump(clang_analyzer_getExtent(p));
+ clang_analyzer_dump(clang_analyzer_getExtent(p->data));
+ // expected-warning at -2 {{2 S64b}}
+ // expected-warning at -2 {{-6 S64b}}
}
void test_zero_length_array_fam(void) {
@@ -70,19 +102,19 @@ void test_zero_length_array_fam(void) {
clang_analyzer_dump(clang_analyzer_getExtent(&fam));
clang_analyzer_dump(clang_analyzer_getExtent(fam.data));
// expected-warning at -2 {{4 S64b}}
- // expected-warning at -2 {{Unknown}}
+ // expected-warning at -2 {{0 S64b}}
FAM *p = (FAM *)alloca(sizeof(FAM));
clang_analyzer_dump(clang_analyzer_getExtent(p));
clang_analyzer_dump(clang_analyzer_getExtent(p->data));
- // expected-warning at -2 {{4 U64b}}
- // expected-warning at -2 {{Unknown}}
+ // expected-warning at -2 {{4 S64b}}
+ // expected-warning at -2 {{0 S64b}}
FAM *q = (FAM *)malloc(sizeof(FAM));
clang_analyzer_dump(clang_analyzer_getExtent(q));
clang_analyzer_dump(clang_analyzer_getExtent(q->data));
- // expected-warning at -2 {{4 U64b}}
- // expected-warning at -2 {{Unknown}}
+ // expected-warning at -2 {{4 S64b}}
+ // expected-warning at -2 {{0 S64b}}
free(q);
}
@@ -97,19 +129,19 @@ void test_single_element_array_possible_fam(void) {
clang_analyzer_dump(clang_analyzer_getExtent(&likely_fam));
clang_analyzer_dump(clang_analyzer_getExtent(likely_fam.data));
// expected-warning at -2 {{8 S64b}}
- // expected-warning at -2 {{Unknown}}
+ // expected-warning at -2 {{4 S64b}}
FAM *p = (FAM *)alloca(sizeof(FAM));
clang_analyzer_dump(clang_analyzer_getExtent(p));
clang_analyzer_dump(clang_analyzer_getExtent(p->data));
- // expected-warning at -2 {{8 U64b}}
- // expected-warning at -2 {{Unknown}}
+ // expected-warning at -2 {{8 S64b}}
+ // expected-warning at -2 {{4 S64b}}
FAM *q = (FAM *)malloc(sizeof(FAM));
clang_analyzer_dump(clang_analyzer_getExtent(q));
clang_analyzer_dump(clang_analyzer_getExtent(q->data));
- // expected-warning at -2 {{8 U64b}}
- // expected-warning at -2 {{Unknown}}
+ // expected-warning at -2 {{8 S64b}}
+ // expected-warning at -2 {{4 S64b}}
free(q);
#else
FAM likely_fam;
@@ -121,13 +153,13 @@ void test_single_element_array_possible_fam(void) {
FAM *p = (FAM *)alloca(sizeof(FAM));
clang_analyzer_dump(clang_analyzer_getExtent(p));
clang_analyzer_dump(clang_analyzer_getExtent(p->data));
- // expected-warning at -2 {{8 U64b}}
+ // expected-warning at -2 {{8 S64b}}
// expected-warning at -2 {{4 S64b}}
FAM *q = (FAM *)malloc(sizeof(FAM));
clang_analyzer_dump(clang_analyzer_getExtent(q));
clang_analyzer_dump(clang_analyzer_getExtent(q->data));
- // expected-warning at -2 {{8 U64b}}
+ // expected-warning at -2 {{8 S64b}}
// expected-warning at -2 {{4 S64b}}
free(q);
#endif
diff --git a/clang/test/Analysis/memory-model.cpp b/clang/test/Analysis/memory-model.cpp
index 4aa88f97477eb5..fbdef4fddbea93 100644
--- a/clang/test/Analysis/memory-model.cpp
+++ b/clang/test/Analysis/memory-model.cpp
@@ -66,8 +66,8 @@ void field_ref(S a) {
void field_ptr(S *a) {
clang_analyzer_dump(&a->f); // expected-warning {{Element{SymRegion{reg_$0<S * a>},0 S64b,struct S}.f}}
- clang_analyzer_dumpExtent(&a->f); // expected-warning {{4 S64b}}
- clang_analyzer_dumpElementCount(&a->f); // expected-warning {{1 S64b}}
+ clang_analyzer_dumpExtent(&a->f); // expected-warning {{extent_$1{SymRegion{reg_$0<S * a>}}}}
+ clang_analyzer_dumpElementCount(&a->f); // expected-warning {{(extent_$1{SymRegion{reg_$0<S * a>}}) / 4U}}
}
void symbolic_array() {
@@ -90,7 +90,7 @@ void symbolic_placement_new() {
void symbolic_malloc() {
int *a = (int *)malloc(12);
clang_analyzer_dump(a); // expected-warning {{Element{HeapSymRegion{conj}}
- clang_analyzer_dumpExtent(a); // expected-warning {{12 U64b}}
+ clang_analyzer_dumpExtent(a); // expected-warning {{12 S64b}}
clang_analyzer_dumpElementCount(a); // expected-warning {{3 S64b}}
free(a);
}
@@ -98,22 +98,22 @@ void symbolic_malloc() {
void symbolic_alloca() {
int *a = (int *)alloca(12);
clang_analyzer_dump(a); // expected-warning {{Element{HeapSymRegion{conj}}
- clang_analyzer_dumpExtent(a); // expected-warning {{12 U64b}}
+ clang_analyzer_dumpExtent(a); // expected-warning {{12 S64b}}
clang_analyzer_dumpElementCount(a); // expected-warning {{3 S64b}}
}
void symbolic_complex() {
int *a = (int *)malloc(4);
- clang_analyzer_dumpExtent(a); // expected-warning {{4 U64b}}
+ clang_analyzer_dumpExtent(a); // expected-warning {{4 S64b}}
clang_analyzer_dumpElementCount(a); // expected-warning {{1 S64b}}
int *b = (int *)realloc(a, sizeof(int) * 2);
- clang_analyzer_dumpExtent(b); // expected-warning {{8 U64b}}
+ clang_analyzer_dumpExtent(b); // expected-warning {{8 S64b}}
clang_analyzer_dumpElementCount(b); // expected-warning {{2 S64b}}
free(b);
int *c = (int *)calloc(3, 4);
- clang_analyzer_dumpExtent(c); // expected-warning {{12 U64b}}
+ clang_analyzer_dumpExtent(c); // expected-warning {{12 S64b}}
clang_analyzer_dumpElementCount(c); // expected-warning {{3 S64b}}
free(c);
}
@@ -123,7 +123,7 @@ void signedness_equality() {
char *b = (char *)malloc(13);
clang_analyzer_dump(clang_analyzer_getExtent(a)); // expected-warning {{13 S64b}}
- clang_analyzer_dump(clang_analyzer_getExtent(b)); // expected-warning {{13 U64b}}
+ clang_analyzer_dump(clang_analyzer_getExtent(b)); // expected-warning {{13 S64b}}
clang_analyzer_eval(clang_analyzer_getExtent(a) ==
clang_analyzer_getExtent(b));
// expected-warning at -2 {{TRUE}}
@@ -155,3 +155,31 @@ void user_defined_new() {
operator delete[](a, std::align_val_t(32));
}
+
+int a[5][0];
+struct T {
+ int x[];
+};
+
+T t[5];
+void zero_sized_element() {
+ clang_analyzer_dumpExtent(a); // expected-warning {{0 S64b}}
+ clang_analyzer_dumpElementCount(a); // expected-warning {{5 S64b}}
+ clang_analyzer_dumpExtent(t); // expected-warning {{0 S64b}}
+ clang_analyzer_dumpElementCount(t); // expected-warning {{5 S64b}}
+}
+
+void extent_with_offset() {
+ int nums[] = {1,2,3};
+ char *p = (char*)&nums[1];
+
+ clang_analyzer_dumpExtent(p); // expected-warning {{8 S64b}}
+ clang_analyzer_dumpElementCount(p); // expected-warning {{8 S64b}}
+ ++p;
+ clang_analyzer_dumpExtent(p); // expected-warning {{7 S64b}}
+ clang_analyzer_dumpElementCount(p); // expected-warning {{7 S64b}}
+ ++p;
+ int *q = (int*)p;
+ clang_analyzer_dumpExtent(q); // expected-warning {{6 S64b}}
+ clang_analyzer_dumpElementCount(q); // expected-warning {{1 S64b}}
+}
More information about the cfe-commits
mailing list