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

[Alexander Droste via cfe-commits]

Alexander_Droste marked 2 inline comments as done.
Alexander_Droste added a comment.

In http://reviews.llvm.org/D12761#248382, @zaks.anna wrote:

> > > It's more user friendly to report this issue at the last point where the request is available rather than the last line of the function.
>
> > 
>
> > >  This looks similar to leak report checking. Is it?
>
> > 
>
> > 
>
> > Yes, that's not very common but possible.
>
> >  Does this change involve the use of a BugReporterVisitor?!
>
>
> You should register/report the diagnostic at check::DeadSymbols event instead of check:: EndFunction; that way the error will be reported earlier.
>
> > You're right, it's very similar.
>


Sorry, I should have been more precise. The check tests if a request is in use by a nonblocking 
call at the end of a function. But the request can still be alive after return. You can think of it as
a pointer for which the memory must be freed in the function it was allocated. But the pointer 
can exist in global space outside a function. Multiple functions can use the same request.
So the symbol is not necessarily dead at the end of a function.


================
Comment at: tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp:33
@@ +32,3 @@
+  // There is no way to reason about a symbolic region.
+  if (MR->getBaseRegion()->getAs<SymbolicRegion>())
+    return;
----------------
Is that assumption correct?

================
Comment at: tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h:65
@@ +64,3 @@
+
+// Register data structure for path-sensitive analysis. The map stores MPI
+// requests which are identified by their memory region. Requests are used in
----------------
The LastUser defines the state of the request and is used in the diagnostic.
As the report will make use of the BugReportVisitor because the LastUser can 
be in another file, should I change the state marking to make use of an enum?
Currently the function classifier is used, to check what kind of function
the LastUser is, to obtain the requests state. Would this fail if the LastUser is from another file?


http://reviews.llvm.org/D12761