[PATCH] D12761: MPI-Checker patch for Clang Static Analyzer

[Anna Zaks via cfe-commits]

zaks.anna added inline comments.

================
Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp:95
@@ +94,3 @@
+  if (Optional<PreStmt> SP = N->getLocation().getAs<PreStmt>()) {
+    if (const CallExpr *CE = clang::dyn_cast<CallExpr>(SP->getStmt())) {
+
----------------
Would it be possible to identify the "interesting node" by just looking at how the state you track changes. This function will be called when we walk the path upward from the error node. For example, you can notice that in the previous state, the Request did not exist and in this state it is Nonblocking. That would allow you to conclude that between those 2 states a function was called. (This is the trick we use in the other checkers (ex: malloc); it simplifies implementation and makes it more robust - we do not need to pattern match as much.)

================
Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:86
@@ +85,3 @@
+      // A wait has no matching nonblocking call.
+      BReporter->reportUnmatchedWait(PreCallEvent, ReqRegion, ErrorNode);
+    }
----------------
Can you add a test cases for this report when there are multiple ReqRegions? Looks like all of the test cases with MPI_Waitall succeed. (I test a case where one of the regions has a match in Waitall and one does not as well as when several/all do not have a match.)

================
Comment at: test/Analysis/MPIChecker.cpp:113
@@ +112,3 @@
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &rs.req); // expected-note{{Request is previously used by nonblocking call here.}}
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &rs.req); // expected-warning{{Double nonblocking on request 'rs.req'.}}
+  MPI_Wait(&rs.req, MPI_STATUS_IGNORE);
----------------
Are we allowed to call reduce -> wait -> reduce with the same req? If so, let's test for that.

Also, can calls to reduce and wait occur in different functions? If so, we need to test cases where they occur in different functions that are all in the same translation unit - all of the implementations are visible to the analyzer. And also test the cases where we pass the request to a function, whose body is not visible to the analyzer, which is the case when the function is defined in another translation unit.



http://reviews.llvm.org/D12761