danix800 created this revision.
danix800 added reviewers: steakhal, donat.nagy, dcoughlin.
danix800 added a project: clang.
Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, 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.
1. MPI_Waitall should respect count arg
2. Add more defensive checking, this also:
This also fixes https://github.com/llvm/llvm-project/issues/64647
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D158813
Files:
clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
clang/test/Analysis/mpichecker-crash-gh64647.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/mpichecker-crash-gh64647.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/mpichecker-crash-gh64647.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=optin.mpi.MPI-Checker -verify %s
+
+#include "MPIMock.h"
+
+bool contains();
+void do_a() {
+ if (contains()) {
+ MPI_Request request_item;
+ MPI_Wait(&request_item, MPI_STATUS_IGNORE);
+ // expected-warning@-1 {{Request 'request_item' has no matching nonblocking call.}}
+ }
+ do_a();
+}
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
@@ -43,6 +43,9 @@
// double nonblocking detected
if (Req && Req->CurrentState == Request::State::Nonblocking) {
ExplodedNode *ErrorNode = Ctx.generateNonFatalErrorNode();
+ if (!ErrorNode)
+ return;
+
BReporter.reportDoubleNonblocking(PreCallEvent, *Req, MR, ErrorNode,
Ctx.getBugReporter());
Ctx.addTransition(ErrorNode->getState(), ErrorNode);
@@ -83,6 +86,9 @@
if (!Req) {
if (!ErrorNode) {
ErrorNode = Ctx.generateNonFatalErrorNode(State, &Tag);
+ if (!ErrorNode)
+ break;
+
State = ErrorNode->getState();
}
// A wait has no matching nonblocking call.
@@ -115,6 +121,9 @@
if (!ErrorNode) {
ErrorNode = Ctx.generateNonFatalErrorNode(State, &Tag);
+ if (!ErrorNode)
+ break;
+
State = ErrorNode->getState();
}
BReporter.reportMissingWait(Req.second, Req.first, ErrorNode,
@@ -161,20 +170,41 @@
return;
}
- DefinedOrUnknownSVal ElementCount = getDynamicElementCount(
- Ctx.getState(), SuperRegion, Ctx.getSValBuilder(),
- CE.getArgExpr(1)->getType()->getPointeeType());
- const llvm::APSInt &ArrSize =
- ElementCount.castAs<nonloc::ConcreteInt>().getValue();
+ QualType ElemType = CE.getArgExpr(1)->getType()->getPointeeType();
+ ProgramStateRef State = Ctx.getState();
+ SValBuilder &SVB = Ctx.getSValBuilder();
+ ASTContext &ASTCtx = Ctx.getASTContext();
- for (size_t i = 0; i < ArrSize; ++i) {
- const NonLoc Idx = Ctx.getSValBuilder().makeArrayIndex(i);
+ auto ElementCount =
+ getDynamicElementCountWithOffset(State, CE.getArgSVal(1), ElemType)
+ .getAs<nonloc::ConcreteInt>();
+ if (!ElementCount)
+ return;
+ const llvm::APSInt &ArrSize = ElementCount->getValue();
- const ElementRegion *const ER = RegionManager.getElementRegion(
- CE.getArgExpr(1)->getType()->getPointeeType(), Idx, SuperRegion,
- Ctx.getASTContext());
+ CharUnits ElemSizeInChars = ASTCtx.getTypeSizeInChars(ElemType);
+ int64_t ElemSizeInBits =
+ (ElemSizeInChars.isZero() ? 1 : ElemSizeInChars.getQuantity()) *
+ ASTCtx.getCharWidth();
+ const NonLoc MROffset =
+ SVB.makeArrayIndex(MR->getAsOffset().getOffset() / ElemSizeInBits);
- ReqRegions.push_back(ER->getAs<MemRegion>());
+ SVal Count = CE.getArgSVal(0);
+ for (size_t i = 0; i < ArrSize; ++i) {
+ const NonLoc Idx = Ctx.getSValBuilder().makeArrayIndex(i);
+ auto CountReached = SVB.evalBinOp(State, BO_GE, Idx, Count, ASTCtx.BoolTy)
+ .getAs<DefinedOrUnknownSVal>();
+ if (CountReached && State->assume(*CountReached, true))
+ break;
+
+ auto ElementRegionIndex =
+ SVB.evalBinOp(State, BO_Add, Idx, MROffset, SVB.getArrayIndexType())
+ .getAs<NonLoc>();
+ if (ElementRegionIndex) {
+ const ElementRegion *const ER = RegionManager.getElementRegion(
+ ElemType, *ElementRegionIndex, SuperRegion, Ctx.getASTContext());
+ ReqRegions.push_back(ER);
+ }
}
} else if (FuncClassifier->isMPI_Wait(CE.getCalleeIdentifier())) {
ReqRegions.push_back(MR);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
