https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/209263
From aa3253eb5edd8ced3573d183753a4e2f4b2f7cda Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Mon, 13 Jul 2026 19:02:43 +0100 Subject: [PATCH 1/2] [analyzer] Recognize missing MPI collectives in MPI-Checker Fixes a false positive in the optin.mpi.MPI-Checker where legitimate code was flagged with "Request 'X' has no matching nonblocking call." Background for reviewers unfamiliar with MPI or this checker ------------------------------------------------------------ MPI (Message Passing Interface) is the standard API for communication between processes in HPC/cluster programs (C/C++/Fortran). Many MPI operations come in a *nonblocking* form, prefixed with "I" (for "immediate"): e.g. MPI_Isend, MPI_Iscatter. A nonblocking call returns immediately and hands back an MPI_Request handle; the caller must later complete it with MPI_Wait (or MPI_Waitall). Forgetting to wait, or reusing a request before waiting, are real bugs (buffer reuse races, leaked requests). The MPI-Checker (originally contributed as a GSoC 2016 project, and is a domain-specific check - hence it's in "optin"). It tracks each MPI_Request region through a small state machine and reports: three misuse patterns: * **missing wait**: a nonblocking call whose request is never waited on * **double nonblocking**: a request reused by a second nonblocking call before being waited on * **unmatched wait**: MPI_Wait on a request with no originating nonblocking call Root cause ---------- `MPIFunctionClassifier` maintains a hand-written list of known MPI functions. That list was incomplete: several standard collectives -- notably the vector variants (`MPI_Iscatterv`, `MPI_Igatherv`, ...), the scans, the reduce-scatter family, and `MPI_Ibarrier` -- were entirely absent. Because `isNonBlockingType()` returned `false` for them, `checkDoubleNonblocking()` never recorded the request as Nonblocking, so the matching `MPI_Wait` looked unmatched and produced a FP. Note that only `isNonBlockingType()`/`isWaitType()`/`isMPI_Wait()`/`isMPI_Waitall()` actually drive the checker; the per-shape predicates (`isScatterType`, `isGatherType`, `isCollToColl`, etc.) and their backing containers are currently unused. `checkDoubleNonblocking()` also reads the request from the *last* argument, which is generic since every nonblocking MPI function places `MPI_Request*` last -- so no per-function argument handling is needed. Fix --- Register the remaining standard collectives and mark the nonblocking ("I") variants as nonblocking: * **vector variants**: `MPI_Scatterv`/`Iscatterv`, `MPI_Gatherv`/`Igatherv`, `MPI_Allgatherv`/`Iallgatherv`, `MPI_Alltoallv`/`Ialltoallv`, `MPI_Alltoallw`/`Ialltoallw` * **nonblocking barrier**: `MPI_Ibarrier` * **scans**: `MPI_Scan`/`Iscan`, `MPI_Exscan`/`Iexscan` * **reduce-scatter**: `MPI_Reduce_scatter`/`Ireduce_scatter`, `MPI_Reduce_scatter_block`/`Ireduce_scatter_block` These functions are not referenced by any per-shape predicate, so they are added directly to the classification containers without a dedicated `IdentifierInfo` member. Neighborhood collectives (`MPI_Ineighbor_*`) and the wait-family beyond `Wait`/`Waitall` (`Waitany`/`Waitsome`/`Test*`) remain out of scope. Adds regression tests covering the reported repro (no warning), and confirms double-nonblocking and missing-wait are still detected for the new functions. Fixes #208107 Co-authored-by: Claude Opus 4.8 <[email protected]> --- .../MPI-Checker/MPIFunctionClassifier.cpp | 42 +++++++++++++++++++ clang/test/Analysis/MPIMock.h | 7 ++++ clang/test/Analysis/mpichecker.cpp | 40 ++++++++++++++++++ 3 files changed, 89 insertions(+) diff --git a/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.cpp b/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.cpp index 277b3ed2e1056..2d618c19b191f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.cpp @@ -174,6 +174,48 @@ void MPIFunctionClassifier::initCollectiveIdentifiers(ASTContext &ASTCtx) { MPINonBlockingTypes.push_back(IdentInfo_MPI_Ialltoall); MPIType.push_back(IdentInfo_MPI_Ialltoall); assert(IdentInfo_MPI_Ialltoall); + + // Register the remaining standard collectives. These are not referenced by + // any of the individual is<Kind>Type() predicates, so they don't need a + // dedicated IdentifierInfo member; the checker only relies on them being + // classified as collective / nonblocking. All nonblocking (MPI_I...) + // variants take an MPI_Request as their last argument. + auto AddCollective = [&](StringRef Name) { + IdentifierInfo *II = &ASTCtx.Idents.get(Name); + MPICollectiveTypes.push_back(II); + MPIType.push_back(II); + return II; + }; + auto AddNonBlockingCollective = [&](StringRef Name) { + MPINonBlockingTypes.push_back(AddCollective(Name)); + }; + + // Vector ("v"/"w") variants of the collectives above. + AddCollective("MPI_Scatterv"); + AddNonBlockingCollective("MPI_Iscatterv"); + AddCollective("MPI_Gatherv"); + AddNonBlockingCollective("MPI_Igatherv"); + AddCollective("MPI_Allgatherv"); + AddNonBlockingCollective("MPI_Iallgatherv"); + AddCollective("MPI_Alltoallv"); + AddNonBlockingCollective("MPI_Ialltoallv"); + AddCollective("MPI_Alltoallw"); + AddNonBlockingCollective("MPI_Ialltoallw"); + + // Nonblocking barrier. + AddNonBlockingCollective("MPI_Ibarrier"); + + // Prefix reductions (scans). + AddCollective("MPI_Scan"); + AddNonBlockingCollective("MPI_Iscan"); + AddCollective("MPI_Exscan"); + AddNonBlockingCollective("MPI_Iexscan"); + + // Reduce-scatter family. + AddCollective("MPI_Reduce_scatter"); + AddNonBlockingCollective("MPI_Ireduce_scatter"); + AddCollective("MPI_Reduce_scatter_block"); + AddNonBlockingCollective("MPI_Ireduce_scatter_block"); } void MPIFunctionClassifier::initAdditionalIdentifiers(ASTContext &ASTCtx) { diff --git a/clang/test/Analysis/MPIMock.h b/clang/test/Analysis/MPIMock.h index 01d2d42fc58a3..f3bc0676d132b 100644 --- a/clang/test/Analysis/MPIMock.h +++ b/clang/test/Analysis/MPIMock.h @@ -37,6 +37,7 @@ namespace std { template<class T> struct complex { T real; T imag; }; } #define MPI_COMM_WORLD 0 #define MPI_STATUS_IGNORE 0 #define MPI_STATUSES_IGNORE 0 +#define MPI_REQUEST_NULL 0 #define MPI_SUM 0 // mock functions @@ -53,3 +54,9 @@ int MPI_Reduce(const void *, void *, int, MPI_Datatype, MPI_Op, int, MPI_Comm); int MPI_Ireduce(const void *, void *, int, MPI_Datatype, MPI_Op, int, MPI_Comm, MPI_Request *); int MPI_Bcast(void *, int count, MPI_Datatype, int, MPI_Comm); +int MPI_Iscatterv(const void *, const int[], const int[], MPI_Datatype, void *, + int, MPI_Datatype, int, MPI_Comm, MPI_Request *); +int MPI_Iscan(const void *, void *, int, MPI_Datatype, MPI_Op, MPI_Comm, + MPI_Request *); +int MPI_Ireduce_scatter(const void *, void *, const int[], MPI_Datatype, + MPI_Op, MPI_Comm, MPI_Request *); diff --git a/clang/test/Analysis/mpichecker.cpp b/clang/test/Analysis/mpichecker.cpp index f7644520ebcf5..f2fd6c0951a2f 100644 --- a/clang/test/Analysis/mpichecker.cpp +++ b/clang/test/Analysis/mpichecker.cpp @@ -48,6 +48,46 @@ void matchedWait3() { } } // no error +void matchedWaitIscatterv() { + double buf = 0; + MPI_Request req = MPI_REQUEST_NULL; + MPI_Iscatterv(NULL, NULL, NULL, MPI_DATATYPE_NULL, &buf, 0, MPI_DATATYPE_NULL, + 0, MPI_COMM_WORLD, &req); + MPI_Wait(&req, MPI_STATUS_IGNORE); +} // no error: this used to report a FP before GH208107. + +void missingWaitIscatterv() { + double buf = 0; + MPI_Request req = MPI_REQUEST_NULL; + MPI_Iscatterv(NULL, NULL, NULL, MPI_DATATYPE_NULL, &buf, 0, MPI_DATATYPE_NULL, + 0, MPI_COMM_WORLD, &req); +} // expected-warning {{Request 'req' has no matching wait.}} + +void doubleNonblockingIscatterv() { + double buf = 0; + MPI_Request req = MPI_REQUEST_NULL; + MPI_Iscatterv(NULL, NULL, NULL, MPI_DATATYPE_NULL, &buf, 0, MPI_DATATYPE_NULL, + 0, MPI_COMM_WORLD, &req); + // expected-warning@+1 {{Double nonblocking on request 'req'.}} + MPI_Iscatterv(NULL, NULL, NULL, MPI_DATATYPE_NULL, &buf, 0, MPI_DATATYPE_NULL, + 0, MPI_COMM_WORLD, &req); + MPI_Wait(&req, MPI_STATUS_IGNORE); +} + +void matchedWaitIscan() { + double buf = 0; + MPI_Request req = MPI_REQUEST_NULL; + MPI_Iscan(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, MPI_COMM_WORLD, &req); + MPI_Wait(&req, MPI_STATUS_IGNORE); +} // no error + +void missingWaitIreduceScatter() { + double buf = 0; + MPI_Request req = MPI_REQUEST_NULL; + MPI_Ireduce_scatter(MPI_IN_PLACE, &buf, NULL, MPI_DOUBLE, MPI_SUM, + MPI_COMM_WORLD, &req); +} // expected-warning {{Request 'req' has no matching wait.}} + void missingWait1() { // Check missing wait for dead region. double buf = 0; MPI_Request sendReq1; From ff28b1c55ff490e471581b04bdb7d24762450b47 Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Tue, 14 Jul 2026 15:18:07 +0100 Subject: [PATCH 2/2] Drop the confusing comment --- .../Checkers/MPI-Checker/MPIFunctionClassifier.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.cpp b/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.cpp index 2d618c19b191f..5fc27dc3ed51b 100644 --- a/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.cpp @@ -175,11 +175,6 @@ void MPIFunctionClassifier::initCollectiveIdentifiers(ASTContext &ASTCtx) { MPIType.push_back(IdentInfo_MPI_Ialltoall); assert(IdentInfo_MPI_Ialltoall); - // Register the remaining standard collectives. These are not referenced by - // any of the individual is<Kind>Type() predicates, so they don't need a - // dedicated IdentifierInfo member; the checker only relies on them being - // classified as collective / nonblocking. All nonblocking (MPI_I...) - // variants take an MPI_Request as their last argument. auto AddCollective = [&](StringRef Name) { IdentifierInfo *II = &ASTCtx.Idents.get(Name); MPICollectiveTypes.push_back(II); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
