Alexander_Droste marked 3 inline comments as done.
Alexander_Droste added a comment.
> I still have to do an overall pass over this checker, but it looks much
> better than the first version!
:D
================
Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:47
@@ +46,3 @@
+ BReporter->reportDoubleNonblocking(PreCallEvent, *Req, MR, ExplNode);
+ Ctx.addTransition(ErrorNode->getState(), ErrorNode);
+ }
----------------
zaks.anna wrote:
> Correct, it should be ErrorNode. Moreover, if you do not modify the State and
> use the state from the predecessor node, which is the case here, you do not
> need to pass a State to it. You do not need to pass the tag either in his
> case - a page generated for your checker will be used. By default the
> CheckerContext will use the state of the predecessor node and that default
> tag. (If you want to pass your tag, it's fine.)
>
> You do need to pass the state if you modify the state as in the next example.
Ah I see, this is covered by the default parameters.
================
Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:135
@@ +134,3 @@
+
+void MPIChecker::checkMissingWaitsGlobals(ExplodedGraph &G) const {
+
----------------
zaks.anna wrote:
> I do not think I've seen this code before. If you add new functionality,
> please submit it in a new commit. Otherwise, there is a high chance it will
> go unnoticed.
>
> Are there examples of the new warnings it caught?
>
> checkEndAnalysis is rarely used by checkers, so you might not need it here.
> Specifically, you should be notified by the checkDeadSymbols before a symbol
> dies even if it dies because it's the end of path.
Yes, I added this at some point after the initial patch, after I realized that
the following test case fails:
```
MPI_Request sendReq1;
void missingWait2() { // Check missing wait for global variable.
int rank = 0;
double buf = 0;
MPI_Comm_rank(MPI_COMM_WORLD, &rank);
MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
&sendReq1);
} // expected-warning{{Request 'sendReq1' has no matching wait.}}
```
if the checker solely relies on `checkDeadSymbols`. I just rechecked this
behavior by commenting out
```
BReporter->reportMissingWait(Req.second, Req.first, *NodeIt);
```
in `checkMissingWaitsGlobals` and the test still fails then. Are global
variables really covered by `checkDeadSymbols`?
http://reviews.llvm.org/D12761
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
