danix800 created this revision.
danix800 added a project: clang.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, dkrupp,
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware,
xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
danix800 requested review of this revision.
Herald added a subscriber: cfe-commits.
Size-type inconsistency (signedness) causes confusion and even bugs.
For example when signed compared to unsigned the result might not
be expected.
Releted checkers:
1. ArrayBoundCheckerV2: signed compared to unsigned
2. MPIChecker: `MPI_Waitall` should respect `count` arg
3. ExprInspection: use more general API to report more results
Fixes https://github.com/llvm/llvm-project/issues/64920
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D158707
Files:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h
clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.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
clang/test/Analysis/mpichecker.cpp
Index: clang/test/Analysis/mpichecker.cpp
===================================================================
--- clang/test/Analysis/mpichecker.cpp
+++ clang/test/Analysis/mpichecker.cpp
@@ -272,6 +272,49 @@
MPI_Wait(&rs.req2, MPI_STATUS_IGNORE);
} // no error
+void nestedRequestWithCount() {
+ typedef struct {
+ MPI_Request req[3];
+ MPI_Request req2;
+ } ReqStruct;
+
+ ReqStruct rs;
+ int rank = 0;
+ double buf = 0;
+ MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+
+ for (int i = 0; i < 3; ++i)
+ MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
+ &rs.req[i]);
+ MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
+ &rs.req2);
+ MPI_Waitall(2, rs.req, MPI_STATUSES_IGNORE);
+ MPI_Waitall(1, rs.req + 2, MPI_STATUSES_IGNORE);
+ MPI_Wait(&rs.req2, MPI_STATUS_IGNORE);
+} // no error
+
+void nestedRequestWithCountMissingNonBlockingWait() {
+ typedef struct {
+ MPI_Request req[3];
+ MPI_Request req2;
+ } ReqStruct;
+
+ ReqStruct rs;
+ int rank = 0;
+ double buf = 0;
+ MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+
+ for (int i = 0; i < 3; ++i)
+ MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
+ &rs.req[i]);
+ MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
+ &rs.req2);
+ MPI_Waitall(1, rs.req, MPI_STATUSES_IGNORE);
+ // MPI_Waitall(1, rs.req + 1, MPI_STATUSES_IGNORE);
+ MPI_Waitall(1, rs.req + 2, MPI_STATUSES_IGNORE);
+ MPI_Wait(&rs.req2, MPI_STATUS_IGNORE);
+} // expected-warning{{Request 'rs.req[1]' has no matching wait.}}
+
void singleRequestInWaitall() {
MPI_Request r;
int rank = 0;
Index: clang/test/Analysis/memory-model.cpp
===================================================================
--- clang/test/Analysis/memory-model.cpp
+++ clang/test/Analysis/memory-model.cpp
@@ -66,8 +66,8 @@
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_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_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 @@
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@-2 {{TRUE}}
Index: clang/test/Analysis/flexible-array-members.c
===================================================================
--- clang/test/Analysis/flexible-array-members.c
+++ clang/test/Analysis/flexible-array-members.c
@@ -44,20 +44,52 @@
clang_analyzer_dump(clang_analyzer_getExtent(&fam));
clang_analyzer_dump(clang_analyzer_getExtent(fam.data));
// expected-warning@-2 {{4 S64b}}
- // expected-warning@-2 {{Unknown}}
+ // expected-warning@-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@-2 {{4 U64b}}
- // expected-warning@-2 {{Unknown}}
+ // expected-warning@-2 {{4 S64b}}
+ // expected-warning@-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@-2 {{4 U64b}}
- // expected-warning@-2 {{Unknown}}
+ // expected-warning@-2 {{4 S64b}}
+ // expected-warning@-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@-2 {{12 S64b}}
+ // expected-warning@-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@-2 {{9 S64b}}
+ // expected-warning@-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@-2 {{2 S64b}}
+ // expected-warning@-2 {{-6 S64b}}
}
void test_zero_length_array_fam(void) {
@@ -70,19 +102,19 @@
clang_analyzer_dump(clang_analyzer_getExtent(&fam));
clang_analyzer_dump(clang_analyzer_getExtent(fam.data));
// expected-warning@-2 {{4 S64b}}
- // expected-warning@-2 {{Unknown}}
+ // expected-warning@-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@-2 {{4 U64b}}
- // expected-warning@-2 {{Unknown}}
+ // expected-warning@-2 {{4 S64b}}
+ // expected-warning@-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@-2 {{4 U64b}}
- // expected-warning@-2 {{Unknown}}
+ // expected-warning@-2 {{4 S64b}}
+ // expected-warning@-2 {{0 S64b}}
free(q);
}
@@ -97,19 +129,19 @@
clang_analyzer_dump(clang_analyzer_getExtent(&likely_fam));
clang_analyzer_dump(clang_analyzer_getExtent(likely_fam.data));
// expected-warning@-2 {{8 S64b}}
- // expected-warning@-2 {{Unknown}}
+ // expected-warning@-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@-2 {{8 U64b}}
- // expected-warning@-2 {{Unknown}}
+ // expected-warning@-2 {{8 S64b}}
+ // expected-warning@-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@-2 {{8 U64b}}
- // expected-warning@-2 {{Unknown}}
+ // expected-warning@-2 {{8 S64b}}
+ // expected-warning@-2 {{4 S64b}}
free(q);
#else
FAM likely_fam;
@@ -121,13 +153,13 @@
FAM *p = (FAM *)alloca(sizeof(FAM));
clang_analyzer_dump(clang_analyzer_getExtent(p));
clang_analyzer_dump(clang_analyzer_getExtent(p->data));
- // expected-warning@-2 {{8 U64b}}
+ // expected-warning@-2 {{8 S64b}}
// expected-warning@-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@-2 {{8 U64b}}
+ // expected-warning@-2 {{8 S64b}}
// expected-warning@-2 {{4 S64b}}
free(q);
#endif
Index: clang/test/Analysis/array-bound-v2-constraint-check.c
===================================================================
--- clang/test/Analysis/array-bound-v2-constraint-check.c
+++ clang/test/Analysis/array-bound-v2-constraint-check.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.security.ArrayBoundV2,debug.ExprInspection \
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix,alpha.security.ArrayBoundV2,debug.ExprInspection \
// RUN: -analyzer-config eagerly-assume=false -verify %s
void clang_analyzer_eval(int);
@@ -83,6 +83,18 @@
clang_analyzer_eval(len <= 2); // expected-warning {{UNKNOWN}}
}
+void *malloc(unsigned long);
+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]
Index: clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
+++ clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
@@ -79,6 +79,19 @@
SvalBuilder.getArrayIndexType());
}
+DefinedOrUnknownSVal getDynamicElementCountWithOffset(ProgramStateRef State,
+ SVal BufV,
+ QualType ElementTy) {
+ SVal Size = getDynamicExtentWithOffset(State, BufV);
+ SValBuilder &SVB = State->getStateManager().getSValBuilder();
+ SVal ElementSize = getElementExtent(ElementTy, SVB);
+
+ SVal ElementCount =
+ SVB.evalBinOp(State, BO_Div, Size, ElementSize, SVB.getArrayIndexType());
+
+ return ElementCount.castAs<DefinedOrUnknownSVal>();
+}
+
ProgramStateRef setDynamicExtent(ProgramStateRef State, const MemRegion *MR,
DefinedOrUnknownSVal Size, SValBuilder &SVB) {
MR = MR->StripCasts();
Index: clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
@@ -161,18 +161,33 @@
return;
}
- DefinedOrUnknownSVal ElementCount = getDynamicElementCount(
- Ctx.getState(), SuperRegion, Ctx.getSValBuilder(),
- CE.getArgExpr(1)->getType()->getPointeeType());
+ QualType ElemType = CE.getArgExpr(1)->getType()->getPointeeType();
+ ProgramStateRef State = Ctx.getState();
+ SValBuilder &SVB = Ctx.getSValBuilder();
+ ASTContext &ASTCtx = Ctx.getASTContext();
+ DefinedOrUnknownSVal ElementCount =
+ getDynamicElementCountWithOffset(State, CE.getArgSVal(1), ElemType);
const llvm::APSInt &ArrSize =
ElementCount.castAs<nonloc::ConcreteInt>().getValue();
+ SVal Count = CE.getArgSVal(0);
+ const NonLoc MROffset =
+ SVB.makeArrayIndex(MR->getAsOffset().getOffset() /
+ ASTCtx.getTypeSizeInChars(ElemType).getQuantity() /
+ ASTCtx.getCharWidth());
for (size_t i = 0; i < ArrSize; ++i) {
const NonLoc Idx = Ctx.getSValBuilder().makeArrayIndex(i);
+ SVal CountReached =
+ SVB.evalBinOp(State, BO_GE, Idx, Count, ASTCtx.BoolTy);
+ if (!CountReached.isUndef() &&
+ State->assume(*CountReached.getAs<DefinedOrUnknownSVal>(), true))
+ break;
const ElementRegion *const ER = RegionManager.getElementRegion(
- CE.getArgExpr(1)->getType()->getPointeeType(), Idx, SuperRegion,
- Ctx.getASTContext());
+ ElemType,
+ SVB.evalBinOp(State, BO_Add, Idx, MROffset, SVB.getArrayIndexType())
+ .castAs<NonLoc>(),
+ SuperRegion, Ctx.getASTContext());
ReqRegions.push_back(ER->getAs<MemRegion>());
}
Index: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -325,12 +325,12 @@
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::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 @@
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);
}
Index: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -199,7 +199,8 @@
}
// CHECK UPPER BOUND
- DefinedOrUnknownSVal Size = getDynamicExtent(state, Reg, svalBuilder);
+ SVal Size = svalBuilder.convertToArrayIndex(
+ getDynamicExtent(state, Reg, svalBuilder));
if (auto KnownSize = Size.getAs<NonLoc>()) {
auto [state_withinUpperBound, state_exceedsUpperBound] =
compareValueToThreshold(state, ByteOffset, *KnownSize, svalBuilder);
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h
@@ -53,6 +53,11 @@
/// (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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
