Alexander_Droste marked 3 inline comments as done.
================
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())) {
+
----------------
zaks.anna wrote:
> 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.)
I think this should work but would slightly change the way diagnostics are
presented in a specific case. If more than 2 nonblocking calls are using a
request in sequence, the expected note (`Request is previously used by
nonblocking call here.`) would not point to the previous but to the first
nonblocking call of that sequence. What do you think; should the `VisitNode`
function be changed regardless or should it stay as it is because of this
specific case?
================
Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:86
@@ +85,3 @@
+ // A wait has no matching nonblocking call.
+ BReporter->reportUnmatchedWait(PreCallEvent, ReqRegion, ErrorNode);
+ }
----------------
zaks.anna wrote:
> 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.)
Sure, I'll add those tests. The case where none of the requests is matched
should be covered by `missingNonBlockingMultiple`.
================
Comment at: test/Analysis/MPIChecker.cpp:98
@@ +97,3 @@
+
+ MPI_Request req;
+ MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
&req); // expected-note{{Request is previously used by nonblocking call here.}}
----------------
zaks.anna wrote:
> Do you see the notes in the report? I think you should pass
> "-analyzer-output=text" to see the notes.
If I pass `-analyzer-output=text` I see the notes in the report. But adding
that flag also produces a lot of 'undesired' output like: `Loop condition is
true. Entering loop body.`
================
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);
----------------
zaks.anna wrote:
> 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.
>
> Are we allowed to call reduce -> wait -> reduce with the same req? If so,
> let's test for that.
Yes, that's allowed. I'll add a test.
> Also, can calls to reduce and wait occur in different functions?
This is also possible.
> 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.
I would then simply create a new pair of `.cpp` and `.h` files in the test
folder where I define those functions so that the MPI-Checker tests can use
them.
http://reviews.llvm.org/D12761
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
