MPI-Checker patch for Clang Static Analyzer

[Alexander_Droste]

Summary

This patch adds a static analysis checker to verify the correct usage of the MPI API in C and C++.

3 path-sensitive checks are included:

• Double nonblocking: Double request usage by nonblocking calls without intermediate wait
• Missing wait: Nonblocking call without matching wait.
• Unmatched wait: Waiting for a request that was never used by a nonblocking call

Examples of how to use the checker can be found on: GitHub https://github.com/0ax1/MPI-Checker

[Alexander_Droste]

Thanks for the review!

tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td

524 ↗

(On Diff #34664)

I ran the checker on CombBLAS.
All reports were correct and nothing crashed. -> changed the help text

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Container.h

30 ↗

(On Diff #34664)

I think it would belong to <algorithm>. As this is part of libc++, how shall I proceed?

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp

25 ↗

(On Diff #34664)

I originally intended to additionally use warnings but then decided
to mark all reports as error types. I simply forgot to remove the warning
constant from the code.

114 ↗

(On Diff #34664)

Yes, the last user can be in another file.
I'll look into the visitor implementation.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp

24 ↗

(On Diff #34664)

I changed the parameter name to PreCallEvent.

32 ↗

(On Diff #34664)

The problem about symbolic regions is rather in collectUsedMemRegions() than
in checkDoubleNonblocking() why the guard is again used in checkWaitUsage().
From my understanding, it is not possible to find out for a symbolic region what element count and subregions (in case of an array) it has. All elements of a request array are supposed to be marked as waited in checkWaitUsage() if the wait function used is MPI_Waitall(). MPI_Waitall() receives an array of requests as an argument, in order to wait for all requests (or rather nonblocking operations) to complete. collectUsedMemRegions() iterates over the array to collect all single requests used. Therefore, the region is cannot be symbolic.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h

64 ↗

(On Diff #34664)

I updated the comment.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Utility.h

15 ↗

(On Diff #34664)

Ok, I leave this unmodified for the moment.

tools/clang/test/Analysis/MPIChecker.c

114 ↗

(On Diff #34664)

1. Yes, that's not very common but possible.
2. Does this change involve the use of a BugReporterVisitor?!

You're right, it's very similar.

[Alexander_Droste]

• capitalized bug types
• made "MPI Error" constant a member variable
• removed unused "MPI Warning" constant
• renamed precallevent parameter
• cxx type matching support
• null pointer type matching support
• changed help description text to "Checks MPI code"

[zaks.anna]

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.

[Alexander_Droste]

In 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.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp

33 ↗

(On Diff #34982)

Is that assumption correct?

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h

65 ↗

(On Diff #34982)

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?

[Alexander_Droste]

Meanwhile, I sketched a BugReportVisitor class for the 'double wait' check called
DoubleWaitVisitor which I modeled as a nested class of MPIBugReporter.
Currently, the retrieval of requests always fails in VisitNode(). (see // This is never reached...)
I'm not sure what's wrong, as I set up the class in the same way as presented in other checkers.
Maybe I overlooked something.

Further, I have some questions about the BugReportVisitor mechanism:

Why does the BugReportVisitor have a Profile() function?

// Why is this pattern used in most BugReporterVisitorImpl subclasses in the Profile() function?
static int X = 0;
ID.AddPointer(&X);

Does the function calling VisitNode() traverse backwards in the graph, as long as nullptr is returned?

[Alexander_Droste]

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp

137 ↗

(On Diff #35175)

I figured out what the problem about the request retrieval is:
REGISTER_MAP_WITH_PROGRAMSTATE(RequestMap, const clang::ento::clang::mpi::Request)
The documentation states: "The macro should not be used inside namespaces, or for traits that must
be accessible from more than one translation unit." I think I need some advice here, how to make the
same RequestMap accessible in multiple translation units.

[Alexander_Droste]

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp

137 ↗

(On Diff #35175)

How about this:
We could capture the PathDiagnosticLocation of the LastUser in each mpi::Request as a member. The RequestMap could then be local to MPICheckerPathSensitive.cpp and no visitors would be needed. The bug reporter class would then not need to know anything about the request map. The only function I can't find to realise this concept would be something like: Report->addPathDiagnosticLocation().

[zaks.anna]

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.

Is it the API requirement that the wait is performed in the same function?

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp

137 ↗

(On Diff #35175)

Almost all other checkers occupy a single file per checker. This is not much more complicated than the others so you could do the same. This would probably be better for consistency, but I do not have such a strong opinion about it and see why you might want to have separate files.

See TaintManager.h for the example of how to share the program state maps across translation units.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h

66 ↗

(On Diff #35175)

Look at the ways the other checkers reference a node that occurred earlier along the symbolic execution path. For example, malloc checker tracks the state of each pointer, which could be "Allocated", "Freed"... When we report a leak, for example, the state would be "Allocated" and the symbol would be dead. The BugReporterVisitor walks up the symbolic path on which the issue occurred and prints an extra note at the place where the node is allocated. It knows at which node the pointer is allocated because that is where the state of the pointer changes from untracked to "Allocated".

[Alexander_Droste]

In D12761#250296, @zaks.anna wrote:

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.

Is it the API requirement that the wait is performed in the same function?

That's a good catch. It is no API requirement that a nonblocking call is matched by a wait within
the scope of a function. I think for C this would be reasonable but for CPP it is not sufficient,
as the matching could be realised with RAII or lambdas. I will change the check so that
it makes use of the checkDeadSymbols callback.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp

137 ↗

(On Diff #35175)

Thanks for pointing out TaintManager.h. I'll try to derive a solution from that.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h

66 ↗

(On Diff #35175)

Ok, I will use the same pattern. Thanks for the explanation.

[dcoughlin]

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp

34 ↗

(On Diff #35175)

You may be able to use StoreManager::iterBindings(), which iterates over symbolic bindings in the store, to handle symbolic regions. See StackAddrEscapeChecker::checkEndFunction() for an example.

[Alexander_Droste]

• changed RequestMap so that the same is used across translation units
• usage of a BugReporterVisitor in reportMissingWait, reportDoubleWait, reportDoubleNonblocking (reportUnmatchedWait needs no visitor)
• based missingWait check on checkDeadSymbol callback
• doubleWaits & doubleNonblocking errors where the 'same' function uses a request twice (e.g. in a loop) got a specific diagnostic, to make the report easily understandable
• fine grained guard in checkDoubleNonblocking & checkWaitUsage

• generalized variable name retrieval (for memory region)
• handle multidimensional arrays in variable name retrieval (for memory region)

• added/adapted integration tests
• removed some helper functions in MPIBugReporter and Utility
• removed UtilityTest.cpp

---

One thing that is still not perfect, is where the last part of the diagnostic
is presented for missing waits. This does not consistently seem to be the end of the scope
of the request (the point where the symbol dies). I compared this with the malloc checker which has
the same behavior. The last diagnostic seems to be presented after the next statement below the last function,
marked by the BugReportVisitor, using the request/memory. I think it would be nice if the last
part of the diagnostic would be presented at the end of the scope.

[Alexander_Droste]

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp

35 ↗

(On Diff #35718)

Thanks for the hint! I tried iterBindings but can't get it to add more functionality.
It seems to me that only TypedRegions are iterable.

[Alexander_Droste]

• dynamic init of MPICheckerPathSensitive in checkDeadSymbols()
• updated RequestMap comment

[zaks.anna]

We gave a talk about building checkers a while back; even though it does not talk about the bug reporter visitors, it might be worth watching if you haven't already seen it.

http://llvm.org/devmtg/2012-11/
Building a Checker in 24 hours

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp

68 ↗

(On Diff #35800)

There is no reason to add an empty transition to the graph. Maybe you want to get the predecessor instead?

106 ↗

(On Diff #35800)

generateNonFatalErrorNode will add a transition to the exploded graph, which means that you should only generate it if there is an error and you do not need to call addTransition later in the function.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h

67 ↗

(On Diff #35800)

Still it looks like you have several pieces of information in the state that are redundant.. Looks like you've added more now.

For example, why do we need RequestId? Each report will only talk about a single request, is this not the case?

Do you absolutely need to store LastUser and VariableName?

Note that storing in the state is very expensive; on the other hand, we can afford to perform costly analysis on post-processing of an error report. This is why all other checkers store minimal information in the state, usually just a single enum and determine all the other information during the walk over the error path in BugReporterVisitor. The visitor will visit all nodes that you visited while reporting this bug, so it should have access to all the information you need.

[Alexander_Droste]

We gave a talk about building checkers a while back; even though it does not talk about the bug reporter visitors, it might be worth watching if you haven't already seen it.

I have watched it some months ago but I'm sure it is a good idea to revisit the talk. Thanks for the idea!

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp

68 ↗

(On Diff #35800)

Yes, that's better!

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h

67 ↗

(On Diff #35800)

I need the RequestID to distinct function calls of the same type
and location using the request multiple times along the path.
Like in a loop:

for int i ...
   nonblocking(.., request) // double nonblocking

Do you absolutely need to store LastUser and VariableName?

Absolutely not. :D I will remove the string member.
The variable name retrieval can be delayed to the point where the report is created.

I can also remove the enum, as the state of the request can be derived from the LastUser.
The reason why I added the enum member, is that I was concerned about that the CallEventRef
can be invalid to reference again if the call is inlined from an implementation defined in a header.
As this seems not to be the case, I can remove the redundancy.

This is why all other checkers store minimal information in the state, usually just a single enum and determine all the other information during the walk over the error path in BugReporterVisitor.

Ah, I get it. Until now that seemed a bit overly complicated to me.

[zaks.anna]

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h

67 ↗

(On Diff #35800)

I suspect the only thing you need in Request is the enum; everything else can be determined while visiting the path.

This should be in ento namespace not in the clang namespace.

[Alexander_Droste]

Sorry about the delay.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h

67 ↗

(On Diff #35800)

I think you're right that the enum is sufficient.
Thus for better diagnostics the source range
of the CallEventRef (LastUser)
should be obtained by the BugReportVisitor.
Is it possible to obtain a CallEventRef based on
a memory region? So conceptually this would mean
to find the last function call (before the error node) using
a specific region.

[Alexander_Droste]

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h

67 ↗

(On Diff #35800)

I'll give this pattern a try:

const Stmt *S = nullptr;
ProgramPoint ProgLoc = N->getLocation();
if (Optional<StmtPoint> SP = ProgLoc.getAs<StmtPoint>()) {
  S = SP->getStmt();
}
// if S then S->getSourceRange

Looks like it could work.

[Alexander_Droste]

• reduced the Request class members to single 1 byte sized enum
• implemented VisitNode for RequestNodeVisitor to find previous call event using the request
• tolerate null pointer constants in type checking (any MPI type is accepted as a combination)
• removed double wait check as it turned out that calling a wait on a request that was previously used by a wait is technically valid
• minor improvement for variableName function

[Alexander_Droste]

• simplification of logic related to null pointer buffer (checkBufferTypeMatch)

[Alexander_Droste]

• detect missing waits on global variables
• some cleanup

[Alexander_Droste]

• pass files being part of the diff with relative instead of absolute paths

[Alexander_Droste]

Hi,

I think the points mentioned should be all addressed now.
The only thing left is that some of the more generic functionality
maybe should be moved from MPI-Checker related files to others.
Therefor, I need some advice where to put these functions (if necessary).

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Container.h

31 ↗

(On Diff #38900)

I just found /include/llvm/ADT/STLExtras.h. Wouldn't that be a good fit for the contains() function?

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp

107 ↗

(On Diff #38900)

I need addTransition() later to take removed requests into account.

[zaks.anna]

I've been mostly looking at the path sensitive checks. Maybe clang-tidy team would be interested in reviewing the syntactic checks.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Container.h

31 ↗

(On Diff #38900)

This sounds right.
I suggest submitting separate patches to the corresponding components.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp

107 ↗

(On Diff #38900)

What you have now, is generating a fork - two successors from the initial predecessor. You need to use the addTransition method that takes a predecessor and pass the ErrorNode there. You should also use the State from that node, not the initial state.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Utility.h

54 ↗

(On Diff #38900)

Since this takes no arguments but MemRegion, seems like it could go on MemRegion. Please, submit this as a separate patch.

[alexfh]

Before I can tell you whether this checker makes sense as a clang-tidy check, can you please explain to me (don't want to deduce this from the code) what specific issues does this check target? Is this limited to a set of AST patterns or is there (or is going to be) some path-based or data flow analysis?

[zaks.anna]

This patch contains a mix of checks, some are path sensitive and some are AST checks. See MPICheckerAST.cpp for the latter.

[alexfh]

The only conclusion I can make is that this patch is huge and very inconvenient to review. I'm fine with moving the AST-based checks to clang-tidy, but I strongly prefer each separate check to go in a separate patch.

[Alexander_Droste]

@anna
Thanks for having a look once more! I will submit these parts as separate patches.

@Alexander
This should be only about the AST-based checks, as Anna takes care of the path-sensitive ones.
I think this is not about moving the checks to clang-tidy because they are not linters but detect
errors. The AST-based functionality is roughly structured as followed:

MPIChecker.cpp : This is the entry point class where checkASTDecl is the only callback for AST-based
checks. There the TranslationUnitVisitor is used which traverses the complete TU passed.
During the traversal each check is executed in the course of the VisitCallExpr callback.
The AST-based checks are implemented in MPICheckerAST.(cpp|h). The TypeVisitor , contained
in TypeVisitor.h, is used as a helper class, in order to detect type properties.

This patch contains 3 AST-based checks (type mismatch, incorrect buffer referencing, invalid argument type)
which are described here in detail (see 3.1 AST-Based Checks) :
https://dl.acm.org/ft_gateway.cfm?id=2833159&ftid=1644251&dwn=1&CFID=566596797&CFTOKEN=98261226

[alexfh]

In D12761#304651, @Alexander_Droste wrote:

@anna
Thanks for having a look once more! I will submit these parts as separate patches.

@Alexander
This should be only about the AST-based checks, as Anna takes care of the path-sensitive ones.
I think this is not about moving the checks to clang-tidy because they are not linters but detect
errors.

Well, many clang-tidy checks have a close to 100% true positive rate and also detect errors of different kinds (as opposed to purely stylistic issues). See http://clang.llvm.org/extra/clang-tidy/checks/misc-undelegated-constructor.html for an example. There's nothing that prevents adding more error-detecting checks to clang-tidy.

I also think, that true power of the static analyzer is the path-based analysis capability, but clang-tidy provides a more convenient way to implement AST- or preprocessor-based checks, so most of the AST-based checks should live there (unless they are generic and high-quality enough to be a compiler diagnostic).

The AST-based functionality is roughly structured as followed:

I'm not an expert in the static analyzer code, so I'm not the one to review even AST-based checks there. However, if you decide to move them to clang-tidy, I'll be happy to review that (I suggest introducing a top-level module for MPI-related checks and putting all relevant AST-based checks there).

MPIChecker.cpp : This is the entry point class where checkASTDecl is the only callback for AST-based
checks. There the TranslationUnitVisitor is used which traverses the complete TU passed.
During the traversal each check is executed in the course of the VisitCallExpr callback.
The AST-based checks are implemented in MPICheckerAST.(cpp|h). The TypeVisitor , contained
in TypeVisitor.h, is used as a helper class, in order to detect type properties.

This patch contains 3 AST-based checks (type mismatch, incorrect buffer referencing, invalid argument type)
which are described here in detail (see 3.1 AST-Based Checks) :
https://dl.acm.org/ft_gateway.cfm?id=2833159&ftid=1644251&dwn=1&CFID=566596797&CFTOKEN=98261226

[Alexander_Droste]

Ah ok, I wasn't aware that clang-tidy is not restricted to checks which verify stylistic issues.
What makes it more convenient to integrate the checks in clang-tidy? Is it how the AST-Matcher
functionality can be accessed?

I'm not an expert in the static analyzer code, so I'm not the one to review even AST-based checks there.

The AST-based checks basically use zero functionality from the static analyzer codebase
except the entry callback and the functionality to generate bug reports.

There's nothing that prevents adding more error-detecting checks to clang-tidy.

Wrt. to the checks being part of this patch I have some concerns:

The first problem I see is that some functionlity is shared between the AST-based
and path-sensitive checks. But I think this should be restricted to the MPIFunctionClassifier class.

Another question I have is how this would affect the usability. At the moment
all detected bugs (AST-based and path-sensitive) appear in a single report,
generated by scan-build. Is there still a way to get the results in a single report
if the checks get moved to clang-tidy?

Would the implementation need to change to AST-Matcher based?
The point about the current implementation is that the AST-based checks
are also already backed by a range of tests defined in
tools/clang/test/Analysis/MPIChecker.cpp.

[alexfh]

In D12761#304823, @Alexander_Droste wrote:

Ah ok, I wasn't aware that clang-tidy is not restricted to checks which verify stylistic issues.
What makes it more convenient to integrate the checks in clang-tidy? Is it how the AST-Matcher
functionality can be accessed?

Yes, less boilerplate code needed to work with AST matchers, powerful clang diagnostic infrastructure is used to report errors, also the add_new_check.py script ;)

I'm not an expert in the static analyzer code, so I'm not the one to review even AST-based checks there.

The AST-based checks basically use zero functionality from the static analyzer codebase
except the entry callback and the functionality to generate bug reports.

Well, that's one of the ways to say that I have little experience and interest in reviewing patches to the static analyzer ;)

There's nothing that prevents adding more error-detecting checks to clang-tidy.

Wrt. to the checks being part of this patch I have some concerns:

The first problem I see is that some functionlity is shared between the AST-based
and path-sensitive checks. But I think this should be restricted to the MPIFunctionClassifier class.

If this class stays in the static analyzer, the clang-tidy check code could probably just depend on the relevant library and use this class?

Another question I have is how this would affect the usability. At the moment
all detected bugs (AST-based and path-sensitive) appear in a single report,
generated by scan-build. Is there still a way to get the results in a single report
if the checks get moved to clang-tidy?

clang-tidy can run static analyzer checks. However, it only supports the plain text output format, which is sub-optimal for representing the results of the path-based checks. So the single report scenario is not easy to implement right now. Gábor Horváth proposed a solution for this a few months ago (PList output for clang-tidy), but a proper implementation of this seemed to require some changes in the clang's diagnostic subsystem.

Would the implementation need to change to AST-Matcher based?

That's not a 100% necessary step (you can always add a matcher for translationUnitDecl() and then traverse the AST using a RecursiveASTVisitor, for example), but this usually results in a more compact and easy to understand code.

The point about the current implementation is that the AST-based checks
are also already backed by a range of tests defined in
tools/clang/test/Analysis/MPIChecker.cpp.

Clang-tidy checks should also be backed by tests ;)

[Alexander_Droste]

Yes, less boilerplate code needed to work with AST matchers, powerful clang diagnostic infrastructure is used to report errors, also the add_new_check.py script ;)

Pretty convincing advertisement line :).

If this class stays in the static analyzer, the clang-tidy check code could probably just depend on the relevant library and use this class?

Sounds straightforward.

That's not a 100% necessary step (you can always add a matcher for translationUnitDecl() and then traverse the AST using a RecursiveASTVisitor, for example), but this usually results in a more compact and easy to understand code.

I guess, in case I need to rewrite the code, I just wanted to know how the AST-Matchers work anyway. :)

Clang-tidy checks should also be backed by tests ;)

I kind of assumed that I need to rewrite the tests if the implementation gets changed.

I'll finish with the path-sensitive checks first and will submit the patch for the AST-based checks to clang-tidy after that.
Thanks for offering a review!

[xazax.hun]

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Utility.h

54 ↗

(On Diff #38900)

A different implementation to achieve this functionality is proposed here: http://reviews.llvm.org/D15924
The proposed implementation is much lighter and has less functionality, it will not print the name of the struct and the index of the array (which might be unknown in most of the cases). Does that implementation works for you? Is that sufficient? Please review it.

[Alexander_Droste]

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp

107 ↗

(On Diff #38900)

Isn't this covered by the first case?
If there's an ErrorNode pass it to addTransition() else just update the state regarding removed requests.

[Alexander_Droste]

• removed AST related functionality from patch, as AST-based checks will be integrated into clang tidy
• fix checkMissingWaits()
• moved getVariableName() from patch (currently under review here: http://reviews.llvm.org/D16044)
• moved is_contained() from patch (currently under review here: http://reviews.llvm.org/D16053)

[Alexander_Droste]

• removed files not being part of this patch anymore

[zaks.anna]

Hi Alexander,

Sorry for the delay!

The patch should be rebased from the clang repo; for example, you could run "svn diff" from the clang directory. More comments inline.

tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td

75 ↗

(On Diff #45129)

This should probably go under the 'option' package. What do you think?

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h

24 ↗

(On Diff #45129)

Should this be clang::ento::mpi? Alternatively, you could place everything into a single file and have it live in anonymous namespace. This would also be more consistent with the existing checkers.

40 ↗

(On Diff #45129)

Looks like some of the AST stuff was deleted and some was kept. Please, either remove all of it or keep all of it in.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

26 ↗

(On Diff #45129)

nit: Please, remove the comments with "----" for consistency.

43 ↗

(On Diff #45129)

I'd stress "path" instead of "sensitive" in the name. Also, this indirection is redundant if you remove the AST checks.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp

43 ↗

(On Diff #45129)

You should use 'generateErrorNode' or 'generateNonFatalErrorNode' to generate the node on which the error should be reported. If the error is non-fatal, you'd need to use the generated node below. Specifically, use it's state and pass it as the predecessor when adding a transition.

74 ↗

(On Diff #45129)

This is called in a loop. Should you break once the first error is reported?

Also, as before you should use the CheckerContext API to produce a node on which the error should be reported.

79 ↗

(On Diff #45129)

Don't forget to specify a predecessor here once the code above changes.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.h

87 ↗

(On Diff #45129)

Please, use a name other than 'BugReporter' to avoid confusing it with the BugReporter in the analyzer.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.h

45 ↗

(On Diff #45129)

Some of these classifier functions are not used either..

77 ↗

(On Diff #45129)

MPIPointToCollTypes and MPICollToPointTypes do not seem to be used.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h

56 ↗

(On Diff #45129)

Let's add some documentation on why you are not using the standard macro REGISTER_MAP_WITH_PROGRAMSTATE. (Might help to avoid confusion lear on.)

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Utility.cpp

21 ↗

(On Diff #45129)

I'd like to remove the Utility.cpp completely by either making the helper functions static or moving them to other shared components.

40 ↗

(On Diff #45129)

This is unused.

47 ↗

(On Diff #45129)

This is not used..

tools/clang/test/Analysis/MPIChecker.cpp

112 ↗

(On Diff #45129)

This is not testing the extra information provided by bug reporter visitor; you should use "// expected-note {...}"

[Alexander_Droste]

Hi Anna,

thanks for having a look once more!

tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td

75 ↗

(On Diff #45129)

Do you mean the 'optin' package? I could not find an 'option' package in Checkers.td.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h

24 ↗

(On Diff #45129)

I would prefer to put this into clang::ento::mpi, rather than having everything in a single file.

40 ↗

(On Diff #45129)

Sorry about the inconsistent change. I will remove the AST related
functionality completety from this patch, as it will be part of the clang-tidy patch.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

43 ↗

(On Diff #45129)

If sufficient, I would rename MPICheckerPathSensitive to MPICheckerPath then.
Do you mind the indirection?

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp

74 ↗

(On Diff #45129)

I don't think break should be called after the first error is reported. Each memory region
represents a different request, why this should be rated as multiple errors.

79 ↗

(On Diff #45129)

Will do.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.h

87 ↗

(On Diff #45129)

I see, that could be confusing. Maybe renaming the variable to BReporter will do.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.h

45 ↗

(On Diff #45129)

These model distinct MPI function classes. I agree that it would be better to remove the unused ones, in order to keep the interface as narrow as possible.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h

56 ↗

(On Diff #45129)

Sure, I can do that.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Utility.cpp

21 ↗

(On Diff #45129)

So where shall we put sourceRange()? It is only used by the BugReporter class.
I could make it a member function of the Reporter class. Or would you prefer this
as a member function of MemRegion?

47 ↗

(On Diff #45129)

These are also leftovers from the AST checks.

tools/clang/test/Analysis/MPIChecker.cpp

112 ↗

(On Diff #45129)

I didn't know about expected-note, thanks!

[zaks.anna]

tools/clang/lib/StaticAnalyzer/Checkers/Checkers.td

75 ↗

(On Diff #45129)

Yes, 'option'. I think it got spell checked.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

43 ↗

(On Diff #45129)

I indirection buying us anything? I think it makes the code more difficult to follow,

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp

74 ↗

(On Diff #45129)

You might be right, I am not 100% sure what's more user-friendly.. Presumably the fixes are different in each case? Could you add a test case where multiple errors on the same call site are reported? (Ups can use expected-warning@+1 to report multiple warnings on the same line.)

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/Utility.cpp

21 ↗

(On Diff #45129)

I don't see why we should't add it to MemRegion. Let's submit that as a separate patch. (If someone else has a better idea they'll tell us:))

[Alexander_Droste]

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

43 ↗

(On Diff #45129)

I think, I like the entry point class being separated from the class which does the actual checking. But the necessity for dynamicInit() maybe outweighs that. I can collapse the two.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp

74 ↗

(On Diff #45129)

This is comparable to calling free on different elements, for which memory was never allocated. So each report informs about a distinct missing nonblocking call. I'll give expected-warning@+1 a try.

[Alexander_Droste]

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp

79 ↗

(On Diff #45129)

Changed this to the same ErrorNode/State pattern like in checkMissingWaits.

tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.h

45 ↗

(On Diff #45129)

I'd like to keep some of the classes/identifiers that are not used in this patch
but will be in the clang-tidy patch, as discussed with Alexander Kornienko.

tools/clang/test/Analysis/MPIChecker.cpp

112 ↗

(On Diff #45129)

I changed the plain comments to // expected-note{{..}} but all tests using an expected note fail by claiming:

error: 'note' diagnostics expected but not seen:
File .../MPIChecker.cpp Line 112: Request is previously used by nonblocking call here.

Though, running the MPI-Checker on this function, the HTML report contains this exact note
at the expected position. The output is produced at the end of MPIBugReporter::RequestNodeVisitor::VisitNode.

Is there something missing, in order to make this an expected note?

[Alexander_Droste]

• fixed checkUnmatchedWaits (added ErrorNode)
• non fatal error node for double nonblocking
• renamed BugReporter variable to BReporter
• description why custom RequestMap is used
• collapse pathsensitivechecker with mpichecker
• mpi-checker move to optin package
• moved sourceRange() memregion.cpp/h
• remove Utility.cpp/h
• put everything into clang::ento
• remove '––––' from comments
• test case checking for multiple missing nonblocking calls in the same line (// expected-warning-re 1+{{Request {{.*}} has..)
• removed some of the mpi classifications/identifiers

[Alexander_Droste]

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp

39

The MemRegion sourceRange() patch is not under review yet. I will submit the patch for after http://reviews.llvm.org/D16044 got accepted.

[Alexander_Droste]

• removed commented out line
• removed dashes '–––' from comment in MPIChecker.cpp testfile
• (diff is created with clang as pwd)

[Alexander_Droste]

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

47

This should actually be reportDoubleNonblocking(..., ErrorNode); instead of ExplNode, right?

[Alexander_Droste]

• use MPI mock header in integration test file

[zaks.anna]

Some comments inline. I still have to do an overall pass over this checker, but it looks much better than the first version!

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

48

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.

85

Here you do need to pass State.

136

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.

[Alexander_Droste]

I still have to do an overall pass over this checker, but it looks much better than the first version!

:D

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

48

Ah I see, this is covered by the default parameters.

136

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?

[Alexander_Droste]

• omit superfluous arguments passed to generateNonFatalErrorNode

[Alexander_Droste]

test/Analysis/MPIChecker.cpp

97

All the expected-notes still fail, even though the diagnostics are presented in a report.

[zaks.anna]

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

136

The analyzer does not do a good job tracking global variables. You might get false positives, specifically, where the variable is released in another translation unit or by calling function that the analyzer does not inline.

[Alexander_Droste]

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

136

So shall we remove or keep the function?

[zaks.anna]

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

136

I think you will get false positives of the nature I describe above if you keep it. If so, you should remove it. The user experience is much better if you avoid false positives.

[Alexander_Droste]

• remove checkMissingWaitsGlobals to prevent potential false positives

[Alexander_Droste]

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp

39

sourceRange patch -> http://reviews.llvm.org/D18309

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

136

I removed the function.

[zaks.anna]

test/Analysis/MPIChecker.cpp

99

Do you see the notes in the report? I think you should pass "-analyzer-output=text" to see the notes.

[zaks.anna]

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp

96

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.)

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

87

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.)

test/Analysis/MPIChecker.cpp

114

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.

[Alexander_Droste]

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp

96

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?

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

87

Sure, I'll add those tests. The case where none of the requests is matched should be covered by missingNonBlockingMultiple.

test/Analysis/MPIChecker.cpp

99

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.

114

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.

[Alexander_Droste]

• added test that uses wrapper functions around MPI functions
• added test to check behavior in case MPI functions are used in other translation units
• added more MPI_Waitall tests -> missingNonBlockingWaitall*

[Alexander_Droste]

• fix typo

[Alexander_Droste]

• check memory regions for nullptr in checkDoubleNonblocking and checkUnmatchedWaits before they get passed to dyn_cast<ElementRegion>

[zaks.anna]

Alexander,

This patch is in a pretty good shape. I am fine with committing it and iterating with smaller updates in tree if it is more convenient for you.

One task that I would like to very strongly encourage is running this on a lot of code. You will find a lot of issues this way that are hard to discover during code review. Both false positives and crashes.

Thanks!
Anna.

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp

97

The advantage of using the state is that it will be much more robust to any further changes to the compiler/checker because you will not be pattern matching the AST but instead will be checking the state, which the core reasoning is based on. One example that comes to mind is indirect calls. You will reduce the amount of code here as well, simplifying maintainability. This is the pattern we use in other checkers as well, so there is a remote chance we could introduce a new simplified API that will do the walk for the checker writers.

With respect to your example. Does it come up in practice? Wouldn't you warn on the second nonblocking request anyway? Could you add such an example to the tests? (Would be good in any case. If you leave the code as is, you can point to that example as the motivation.)

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

88

This is done, right?

test/Analysis/MPIChecker.cpp

100

This are explaining the path on which the problem occurs; the users will see them as well. There should not be a lot of those, you do not have a lot of conditions. Would it be reasonable to change the tests to incorporate those. Other alternative is to have another tests file that tests the notes in that mode.

What do you think?

115

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.

You do not have to do that. You could just declare the functions and not define them. It will be equivalent to having the definitions in the other TUs.

[Alexander_Droste]

Hi Anna,

I am fine with committing it and iterating with smaller updates in tree if it is more convenient for you.

This sounds good! The last thing I'll change before are the improvements you pointed out.

One task that I would like to very strongly encourage is running this on a lot of code.

Good idea. I'll do that.

Thanks a lot for all the time and effort you invested into the review!

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

88

Yep.

test/Analysis/MPIChecker.cpp

100

I'm fine with adding the notes to this test file.

115

Like you suggested, I'll simply declare the functions without definition.

[Alexander_Droste]

test/Analysis/MPIChecker.cpp

100

I had a look once more how much output this actually produces. If -analyzer-output=text is passed, 42 notes are missing. I'll add another test file which will test for these notes.

[Alexander_Droste]

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp

97

I'll change this to the pattern you suggested.

With respect to your example. Does it come up in practice?

It's for sure a little contrived.

Wouldn't you warn on the second nonblocking request anyway?

Yes.

Could you add such an example to the tests?

Sure.

[Alexander_Droste]

• added test file to test for note diagnostics
• changed BugReportVisitor to detect request usage purely based on state and existence of a request
• added test that showcases a triple nonblocking usage of a request

[zaks.anna]

Awesome!

Thank you for investing SO MUCH time into improving the checker and addressing the review comments.
Do you have commit access?

Nit: Please, use lower case letters for test names to be consistent with the rest.

[Alexander_Droste]

• lower case letters for test filenames

Yeah; I'm excited to see that this code will now be part of LLVM. Thanks to everybody reviewing the patch!
I don't have commit access. Can you commit the bundle of related patches for me?

[xazax.hun]

Anna, will you commit this, or do you want me to commit the patches?

[dcoughlin]

This doesn't compile under gcc so it broke the bots. The fix is to move the specialization of clang::ento::ProgramStateTrait for RequestMapImpl out of the global namespace and into clang::ento. I will apply and recommit.

[dcoughlin]

Fixed the compilation issues with gcc in r271977 r271981, but it is still failing with Address Sanitizer diagnostics:

18751==ERROR: AddressSanitizer: stack-use-after-return on address 0x7f0c695ebc70 at pc 0x00000867b44c bp 0x7ffe3b01d6f0 sp 0x7ffe3b01d6e8

READ of size 8 at 0x7f0c695ebc70 thread T0

#0 0x867b44b in getSourceManager /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:448:46
#1 0x867b44b in clang::ento::BugReporter::emitReport(std::__1::unique_ptr<clang::ento::BugReport, std::__1::default_delete<clang::ento::BugReport> >) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Core/BugReporter.cpp:3240
#2 0x8636355 in clang::ento::mpi::MPIBugReporter::reportUnmatchedWait(clang::ento::CallEvent const&, clang::ento::MemRegion const*, clang::ento::ExplodedNode const*) const /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp:82:3
#3 0x843e1ee in clang::ento::mpi::MPIChecker::checkUnmatchedWaits(clang::ento::CallEvent const&, clang::ento::CheckerContext&) const /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:88:7
#4 0x8447dc8 in checkPreCall /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.h:36:5
#5 0x8447dc8 in void clang::ento::check::PreCall::_checkCall<clang::ento::mpi::MPIChecker>(void*, clang::ento::CallEvent const&, clang::ento::CheckerContext&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/include/clang/StaticAnalyzer/Core/Checker.h:168
#6 0x86db2f2 in operator() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:59:12
#7 0x86db2f2 in runChecker /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp:269
#8 0x86db2f2 in expandGraphWithCheckers<(anonymous namespace)::CheckCallContext> /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp:123
#9 0x86db2f2 in clang::ento::CheckerManager::runCheckersForCallEvent(bool, clang::ento::ExplodedNodeSet&, clang::ento::ExplodedNodeSet const&, clang::ento::CallEvent const&, clang::ento::ExprEngine&, bool) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp:285
#10 0x8782a84 in runCheckersForPreCall /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:252:5
#11 0x8782a84 in clang::ento::ExprEngine::evalCall(clang::ento::ExplodedNodeSet&, clang::ento::ExplodedNode*, clang::ento::CallEvent const&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:509
#12 0x878205b in clang::ento::ExprEngine::VisitCallExpr(clang::CallExpr const*, clang::ento::ExplodedNode*, clang::ento::ExplodedNodeSet&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:487:5
#13 0x872a0d8 in clang::ento::ExprEngine::Visit(clang::Stmt const*, clang::ento::ExplodedNode*, clang::ento::ExplodedNodeSet&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:1104:7
#14 0x87210bf in clang::ento::ExprEngine::ProcessStmt(clang::CFGStmt, clang::ento::ExplodedNode*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:448:5
#15 0x87207ae in clang::ento::ExprEngine::processCFGElement(clang::CFGElement, clang::ento::ExplodedNode*, unsigned int, clang::ento::NodeBuilderContext*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:297:7
#16 0x86fc7a6 in clang::ento::CoreEngine::HandlePostStmt(clang::CFGBlock const*, unsigned int, clang::ento::ExplodedNode*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:515:5
#17 0x86fafed in clang::ento::CoreEngine::dispatchWorkItem(clang::ento::ExplodedNode*, clang::ProgramPoint, clang::ento::WorkListUnit const&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:274:7
#18 0x86f9891 in clang::ento::CoreEngine::ExecuteWorkList(clang::LocationContext const*, unsigned int, llvm::IntrusiveRefCntPtr<clang::ento::ProgramState const>) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:231:5
#19 0x67f1ab5 in ExecuteWorkList /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:109:12
#20 0x67f1ab5 in (anonymous namespace)::AnalysisConsumer::ActionExprEngine(clang::Decl*, bool, clang::ento::ExprEngine::InliningModes, llvm::DenseSet<clang::Decl const*, llvm::DenseMapInfo<clang::Decl const*> >*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:664
#21 0x67f0ae2 in RunPathSensitiveChecks /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:694:5
#22 0x67f0ae2 in (anonymous namespace)::AnalysisConsumer::HandleCode(clang::Decl*, unsigned int, clang::ento::ExprEngine::InliningModes, llvm::DenseSet<clang::Decl const*, llvm::DenseMapInfo<clang::Decl const*> >*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:632
#23 0x67dc135 in HandleDeclsCallGraph /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:495:5
#24 0x67dc135 in (anonymous namespace)::AnalysisConsumer::HandleTranslationUnit(clang::ASTContext&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:547
#25 0x687dae4 in clang::ParseAST(clang::Sema&, bool, bool) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Parse/ParseAST.cpp:167:3
#26 0x51c090b in clang::FrontendAction::Execute() /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Frontend/FrontendAction.cpp:457:8
#27 0x510f614 in clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:876:7
#28 0x537fa92 in clang::ExecuteCompilerInvocation(clang::CompilerInstance*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:241:18
#29 0x8fc2f2 in cc1_main(llvm::ArrayRef<char const*>, char const*, void*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/tools/driver/cc1_main.cpp:116:13
#30 0x8f6a88 in ExecuteCC1Tool /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/tools/driver/driver.cpp:301:12
#31 0x8f6a88 in main /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/tools/driver/driver.cpp:381
#32 0x7f0c6c208f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
#33 0x816952 in _start (/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/clang-3.9+0x816952)

Address 0x7f0c695ebc70 is located in stack of thread T0 at offset 1136 in frame

  #0 0x67f130f in (anonymous namespace)::AnalysisConsumer::ActionExprEngine(clang::Decl*, bool, clang::ento::ExprEngine::InliningModes, llvm::DenseSet<clang::Decl const*, llvm::DenseMapInfo<clang::Decl const*> >*) /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:644

This frame has 5 object(s):
  [32, 184) 'P.i'
  [256, 260) 'FD.i'
  [272, 296) 'ref.tmp.i'
  [336, 344) 'agg.tmp.i'
  [368, 1248) 'Eng' <== Memory access at offset 1136 is inside this variable

HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext

(longjmp and C++ exceptions *are* supported)

SUMMARY: AddressSanitizer: stack-use-after-return /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:448:46 in getSourceManager
Shadow bytes around the buggy address:

0x0fe20d2b5730: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
0x0fe20d2b5740: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
0x0fe20d2b5750: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
0x0fe20d2b5760: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
0x0fe20d2b5770: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5

>0x0fe20d2b5780: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5[f5]f5

0x0fe20d2b5790: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
0x0fe20d2b57a0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
0x0fe20d2b57b0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
0x0fe20d2b57c0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
0x0fe20d2b57d0: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5

Shadow byte legend (one shadow byte represents 8 application bytes):

Addressable:           00
Partially addressable: 01 02 03 04 05 06 07 
Heap left redzone:       fa
Heap right redzone:      fb
Freed heap region:       fd
Stack left redzone:      f1
Stack mid redzone:       f2
Stack right redzone:     f3
Stack partial redzone:   f4
Stack after return:      f5
Stack use after scope:   f8
Global redzone:          f9
Global init order:       f6
Poisoned by user:        f7
Container overflow:      fc
Array cookie:            ac
Intra object redzone:    bb
ASan internal:           fe
Left alloca redzone:     ca
Right alloca redzone:    cb

18751==ABORTING

---

Testing: 0
FAIL: Clang :: Analysis/mpicheckernotes.cpp (317 of 9433)

lib/StaticAnalyzer/Checkers/CMakeLists.txt

   MallocChecker.cpp
   MallocOverflowSecurityChecker.cpp
   MallocSizeofChecker.cpp
+  MPI-Checker/MPIBugReporter.cpp
+  MPI-Checker/MPIChecker.cpp
+  MPI-Checker/MPIFunctionClassifier.cpp
   NSAutoreleasePoolChecker.cpp
   NSErrorChecker.cpp
   NoReturnFunctionChecker.cpp

lib/StaticAnalyzer/Checkers/Checkers.td

 def LocalizabilityAlpha : Package<"localizability">, InPackage<CocoaAlpha>;
 def LocalizabilityOptIn : Package<"localizability">, InPackage<CocoaOptIn>;
 
+def MPI : Package<"mpi">, InPackage<OptIn>;
+
 def LLVM : Package<"llvm">;
 def Debug : Package<"debug">;
 

   DescFile<"LocalizationChecker.cpp">;
 }
 
+let ParentPackage = MPI in {
+def MPIChecker : Checker<"MPI-Checker">,
+  HelpText<"Checks MPI code">,
+  DescFile<"MPIChecker.cpp">;
+} // end "MPI"
+
 //===----------------------------------------------------------------------===//
 // Checkers for LLVM development.
 //===----------------------------------------------------------------------===//

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.h

+//===-- MPIBugReporter.h - bug reporter -----------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file defines prefabricated reports which are emitted in
+/// case of MPI related bugs, detected by path-sensitive analysis.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPIBUGREPORTER_H
+#define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPIBUGREPORTER_H
+
+#include "MPIFunctionClassifier.h"
+#include "MPITypes.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+
+namespace clang {
+namespace ento {
+namespace mpi {
+
+class MPIBugReporter {
+public:
+  MPIBugReporter(BugReporter &BR, const CheckerBase &CB,
+                 const MPIFunctionClassifier &FC)
+      : BReporter{BR}, FuncClassifier{FC} {
+    UnmatchedWaitBugType.reset(new BugType(&CB, "Unmatched wait", MPIError));
+    DoubleNonblockingBugType.reset(
+        new BugType(&CB, "Double nonblocking", MPIError));
+    MissingWaitBugType.reset(new BugType(&CB, "Missing wait", MPIError));
+  }
+
+  /// Report duplicate request use by nonblocking calls without intermediate
+  /// wait.
+  ///
+  /// \param MPICallEvent MPI call that caused the double nonblocking
+  /// \param Req request that was used by two nonblocking calls in sequence
+  /// \param RequestRegion memory region of the request
+  /// \param ExplNode node in the graph the bug appeared at
+  void reportDoubleNonblocking(const CallEvent &MPICallEvent,
+                               const Request &Req,
+                               const MemRegion *const RequestRegion,
+                               const ExplodedNode *const ExplNode) const;
+
+  /// Report a missing wait for a nonblocking call. A missing wait report
+  /// is emitted if a nonblocking call is not matched in the scope of a
+  /// function.
+  ///
+  /// \param Req request that is not matched by a wait
+  /// \param RequestRegion memory region of the request
+  /// \param ExplNode node in the graph the bug appeared at
+  void reportMissingWait(const Request &Req,
+                         const MemRegion *const RequestRegion,
+                         const ExplodedNode *const ExplNode) const;
+
+  /// Report a wait on a request that has not been used at all before.
+  ///
+  /// \param CE wait call that uses the request
+  /// \param ReqRegion memory region of the request
+  /// \param ExplNode node in the graph the bug appeared at
+  void reportUnmatchedWait(const CallEvent &CE,
+                           const MemRegion *const RequestRegion,
+                           const ExplodedNode *const ExplNode) const;
+
+private:
+  const std::string MPIError{"MPI Error"};
+
+  // path-sensitive bug types
+  std::unique_ptr<BugType> UnmatchedWaitBugType;
+  std::unique_ptr<BugType> MissingWaitBugType;
+  std::unique_ptr<BugType> DoubleNonblockingBugType;
+
+  BugReporter &BReporter;
+  const MPIFunctionClassifier &FuncClassifier;
+
+  /// Bug visitor class to find the node where the request region was previously
+  /// used in order to include it into the BugReport path.
+  class RequestNodeVisitor : public BugReporterVisitorImpl<RequestNodeVisitor> {
+  public:
+    RequestNodeVisitor(const MemRegion *const MemoryRegion,
+                       const std::string &ErrText,
+                       const MPIFunctionClassifier &FC)
+        : RequestRegion(MemoryRegion), ErrorText{ErrText}, FuncClassifier{FC} {}
+
+    void Profile(llvm::FoldingSetNodeID &ID) const override {
+      static int X = 0;
+      ID.AddPointer(&X);
+      ID.AddPointer(RequestRegion);
+    }
+
+    PathDiagnosticPiece *VisitNode(const ExplodedNode *N,
+                                   const ExplodedNode *PrevN,
+                                   BugReporterContext &BRC,
+                                   BugReport &BR) override;
+
+  private:
+    const MemRegion *const RequestRegion;
+    bool IsNodeFound{false};
+    std::string ErrorText;
+    const MPIFunctionClassifier &FuncClassifier;
+  };
+};
+
+} // end of namespace: mpi
+} // end of namespace: ento
+} // end of namespace: clang
+
+#endif

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp

+//===-- MPIBugReporter.cpp - bug reporter -----------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file defines prefabricated reports which are emitted in
+/// case of MPI related bugs, detected by path-sensitive analysis.
+///
+//===----------------------------------------------------------------------===//
+
+#include "MPIBugReporter.h"
+#include "MPIChecker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+
+namespace clang {
+namespace ento {
+namespace mpi {
+
+void MPIBugReporter::reportDoubleNonblocking(
+    const CallEvent &MPICallEvent, const ento::mpi::Request &Req,
+    const MemRegion *const RequestRegion,
+    const ExplodedNode *const ExplNode) const {
+
+  std::string ErrorText;
+  ErrorText = "Double nonblocking on request " +
+              RequestRegion->getDescriptiveName() + ". ";
+
+  auto Report = llvm::make_unique<BugReport>(*DoubleNonblockingBugType,
+                                             ErrorText, ExplNode);
+
+  Report->addRange(MPICallEvent.getSourceRange());
+  SourceRange Range = RequestRegion->sourceRange();
+
+  if (Range.isValid())
+    Report->addRange(Range);
+
+  Report->addVisitor(llvm::make_unique<RequestNodeVisitor>(
+      RequestRegion, "Request is previously used by nonblocking call here. ",
+      FuncClassifier));
+  Report->markInteresting(RequestRegion);
+
+  BReporter.emitReport(std::move(Report));
+}
+
+void MPIBugReporter::reportMissingWait(
+    const ento::mpi::Request &Req, const MemRegion *const RequestRegion,
+    const ExplodedNode *const ExplNode) const {
+  std::string ErrorText{"Request " + RequestRegion->getDescriptiveName() +
+                        " has no matching wait. "};
+
+  auto Report =
+      llvm::make_unique<BugReport>(*MissingWaitBugType, ErrorText, ExplNode);
+
+  SourceRange Range = RequestRegion->sourceRange();
+  if (Range.isValid())
+    Report->addRange(Range);
+  Report->addVisitor(llvm::make_unique<RequestNodeVisitor>(
+      RequestRegion, "Request is previously used by nonblocking call here. ",
+      FuncClassifier));
+  Report->markInteresting(RequestRegion);
+
+  BReporter.emitReport(std::move(Report));
+}
+
+void MPIBugReporter::reportUnmatchedWait(
+    const CallEvent &CE, const clang::ento::MemRegion *const RequestRegion,
+    const ExplodedNode *const ExplNode) const {
+  std::string ErrorText{"Request " + RequestRegion->getDescriptiveName() +
+                        " has no matching nonblocking call. "};
+
+  auto Report =
+      llvm::make_unique<BugReport>(*UnmatchedWaitBugType, ErrorText, ExplNode);
+
+  Report->addRange(CE.getSourceRange());
+  SourceRange Range = RequestRegion->sourceRange();
+  if (Range.isValid())
+    Report->addRange(Range);
+
+  BReporter.emitReport(std::move(Report));
+}
+
+PathDiagnosticPiece *MPIBugReporter::RequestNodeVisitor::VisitNode(
+    const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC,
+    BugReport &BR) {
+
+  if (IsNodeFound)
+    return nullptr;
+
+  if (Optional<PreStmt> SP = N->getLocation().getAs<PreStmt>()) {
+    if (const CallExpr *CE = clang::dyn_cast<CallExpr>(SP->getStmt())) {
+
+      auto FuncIdentifier = CE->getDirectCallee()->getIdentifier();
+      CallEventManager &Mgr =
+          N->getState()->getStateManager().getCallEventManager();
+      // Find previous call that uses the request.
+      CallEventRef<> Call =
+          Mgr.getSimpleCall(CE, N->getState(), N->getLocationContext());
+
+      bool ReqUsedByCall{false};
+      // nonblocking
+      if (FuncClassifier.isNonBlockingType(FuncIdentifier)) {
+        SVal SV = Call->getArgSVal(Call->getNumArgs() - 1);
+        if (SV.getAsRegion() == RequestRegion) {
+          ReqUsedByCall = true;
+        }
+      }
+      // waitall
+      else if (FuncClassifier.isMPI_Waitall(FuncIdentifier)) {
+        const MemRegion *const OtherReqRegion =
+            Call->getArgSVal(1).getAsRegion();
+        if (OtherReqRegion == RequestRegion ||
+            (RequestRegion->isSubRegionOf(OtherReqRegion))) {
+          ReqUsedByCall = true;
+        }
+      }
+      // wait
+      else if (FuncClassifier.isMPI_Wait(FuncIdentifier)) {
+        if (Call->getArgSVal(0).getAsRegion() == RequestRegion) {
+          ReqUsedByCall = true;
+        }
+      }
+
+      if (ReqUsedByCall) {
+        // Do not include further locations.
+        IsNodeFound = true;
+        PathDiagnosticLocation Pos(CE, BRC.getSourceManager(),
+                                   N->getLocationContext());
+
+        return new PathDiagnosticEventPiece(Pos, ErrorText);
+      }
+    }
+  }
+
+  return nullptr;
+}
+
+} // end of namespace: mpi
+} // end of namespace: ento
+} // end of namespace: clang

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.h

+//===-- MPIChecker.h - Verify MPI API usage- --------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file defines the main class of MPI-Checker which serves as an entry
+/// point. It is created once for each translation unit analysed.
+/// The checker defines path-sensitive checks, to verify correct usage of the
+/// MPI API.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPICHECKER_H
+#define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPICHECKER_H
+
+#include "MPIBugReporter.h"
+#include "MPIFunctionClassifier.h"
+#include "MPITypes.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+namespace clang {
+namespace ento {
+namespace mpi {
+
+class MPIChecker
+    : public Checker<check::PreCall, check::DeadSymbols, check::EndAnalysis> {
+public:
+  // path-sensitive callbacks
+  void checkPreCall(const CallEvent &CE, CheckerContext &Ctx) const {
+    dynamicInit(Ctx);
+    checkUnmatchedWaits(CE, Ctx);
+    checkDoubleNonblocking(CE, Ctx);
+  }
+
+  void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &Ctx) const {
+    dynamicInit(Ctx);
+    checkMissingWaits(SymReaper, Ctx);
+  }
+
+  void checkEndAnalysis(ExplodedGraph &G, BugReporter &, ExprEngine &) const {
+    checkMissingWaitsGlobals(G);
+  }
+
+  void dynamicInit(CheckerContext &Ctx) const {
+    if (IsInitialized)
+      return;
+    const_cast<std::unique_ptr<MPIFunctionClassifier> &>(FuncClassifier)
+        .reset(new MPIFunctionClassifier{Ctx.getAnalysisManager()});
+
+    const_cast<std::unique_ptr<MPIBugReporter> &>(BReporter).reset(
+        new MPIBugReporter{Ctx.getBugReporter(), *this, *FuncClassifier});
+
+    const_cast<bool &>(IsInitialized) = true;
+  }
+
+  /// Checks if a request is used by nonblocking calls multiple times
+  /// in sequence without intermediate wait. The check contains a guard,
+  /// in order to only inspect nonblocking functions.
+  ///
+  /// \param PreCallEvent MPI call to verify
+  void checkDoubleNonblocking(const clang::ento::CallEvent &PreCallEvent,
+                              clang::ento::CheckerContext &Ctx) const;
+
+  /// Checks if a request is used by a wait multiple times in sequence without
+  /// intermediate nonblocking call or if the request used by the wait
+  /// function was not used at all before. The check contains a guard,
+  /// in order to only inspect wait functions.
+  ///
+  /// \param PreCallEvent MPI call to verify
+  void checkUnmatchedWaits(const clang::ento::CallEvent &PreCallEvent,
+                           clang::ento::CheckerContext &Ctx) const;
+
+  /// Check if a nonblocking call is not matched by a wait.
+  /// If a memory region is not alive and the last function using the
+  /// request was a nonblocking call, this is rated as a missing wait.
+  void checkMissingWaits(clang::ento::SymbolReaper &SymReaper,
+                         clang::ento::CheckerContext &Ctx) const;
+
+  /// Check if a nonblocking call is not matched by a wait. This function
+  /// invokes the verification based on global request variables. Therefore,
+  /// all end nodes of a graph built for a translation unit are inspected.
+  /// \param G Graph constructed during path-sensitive analysis for the
+  /// analysed translation unit.
+  void checkMissingWaitsGlobals(clang::ento::ExplodedGraph &G) const;
+
+private:
+  /// Collects all memory regions of a request(array) used by a wait
+  /// function. If the wait function uses a single request, this is a single
+  /// region. For wait functions using multiple requests, multiple regions
+  /// representing elements in the array are collected.
+  ///
+  /// \param ReqRegions vector the regions get pushed into
+  /// \param MR top most region to iterate
+  /// \param CE MPI wait call using the request(s)
+  void allRegionsUsedByWait(
+      llvm::SmallVector<const clang::ento::MemRegion *, 2> &ReqRegions,
+      const clang::ento::MemRegion *const MR, const clang::ento::CallEvent &CE,
+      clang::ento::CheckerContext &Ctx) const;
+
+  /// Returns the memory region used by a wait function.
+  /// Distinguishes between MPI_Wait and MPI_Waitall.
+  ///
+  /// \param CE MPI wait call
+  const clang::ento::MemRegion *
+  topRegionUsedByWait(const clang::ento::CallEvent &CE) const;
+
+  const std::unique_ptr<MPIFunctionClassifier> FuncClassifier;
+  const std::unique_ptr<MPIBugReporter> BReporter;
+  bool IsInitialized{false};
+};
+
+} // end of namespace: mpi
+} // end of namespace: ento
+} // end of namespace: clang
+
+#endif

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp

+//===-- MPIChecker.cpp - Checker Entry Point Class --------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file defines the main class of MPI-Checker which serves as an entry
+/// point. It is created once for each translation unit analysed.
+/// The checker defines path-sensitive checks, to verify correct usage of the
+/// MPI API.
+///
+//===----------------------------------------------------------------------===//
+
+#include "MPIChecker.h"
+#include "../ClangSACheckers.h"
+
+namespace clang {
+namespace ento {
+namespace mpi {
+
+void MPIChecker::checkDoubleNonblocking(const CallEvent &PreCallEvent,
+                                        CheckerContext &Ctx) const {
+  if (!FuncClassifier->isNonBlockingType(PreCallEvent.getCalleeIdentifier())) {
+    return;
+  }
+  const MemRegion *const MR =
+      PreCallEvent.getArgSVal(PreCallEvent.getNumArgs() - 1).getAsRegion();
+  const ElementRegion *const ER = dyn_cast<ElementRegion>(MR);
+
+  // The region must be typed, in order to reason about it.
+  if (!isa<TypedRegion>(MR) || (ER && !isa<TypedRegion>(ER->getSuperRegion())))
+    return;
+
+  ProgramStateRef State = Ctx.getState();
+  const Request *const Req = State->get<RequestMap>(MR);
+
+  // double nonblocking detected
+  if (Req && Req->CurrentState == Request::State::Nonblocking) {
+    ExplodedNode *ErrorNode = Ctx.generateNonFatalErrorNode();
+    BReporter->reportDoubleNonblocking(PreCallEvent, *Req, MR, ErrorNode);
+    Ctx.addTransition(ErrorNode->getState(), ErrorNode);
+  }
+  // no error
+  else {
+    State = State->set<RequestMap>(MR, Request::State::Nonblocking);
+    Ctx.addTransition(State);
+  }
+}
+
+void MPIChecker::checkUnmatchedWaits(const CallEvent &PreCallEvent,
+                                     CheckerContext &Ctx) const {
+  if (!FuncClassifier->isWaitType(PreCallEvent.getCalleeIdentifier()))
+    return;
+  const MemRegion *const MR = topRegionUsedByWait(PreCallEvent);
+  const ElementRegion *const ER = dyn_cast<ElementRegion>(MR);
+  if (!MR)
+    return;
+
+  // The region must be typed, in order to reason about it.
+  if (!isa<TypedRegion>(MR) || (ER && !isa<TypedRegion>(ER->getSuperRegion())))
+    return;
+
+  llvm::SmallVector<const MemRegion *, 2> ReqRegions;
+  allRegionsUsedByWait(ReqRegions, MR, PreCallEvent, Ctx);
+  if (ReqRegions.empty())
+    return;
+
+  ProgramStateRef State = Ctx.getState();
+  static CheckerProgramPointTag Tag("MPI-Checker", "UnmatchedWait");
+  ExplodedNode *ErrorNode{nullptr};
+
+  // Check all request regions used by the wait function.
+  for (const auto &ReqRegion : ReqRegions) {
+    const Request *const Req = State->get<RequestMap>(ReqRegion);
+    State = State->set<RequestMap>(ReqRegion, Request::State::Wait);
+    if (!Req) {
+      if (!ErrorNode) {
+        ErrorNode = Ctx.generateNonFatalErrorNode(State, &Tag);
+        State = ErrorNode->getState();
+      }
+      // A wait has no matching nonblocking call.
+      BReporter->reportUnmatchedWait(PreCallEvent, ReqRegion, ErrorNode);
+    }
+  }
+
+  if (!ErrorNode) {
+    Ctx.addTransition(State);
+  } else {
+    Ctx.addTransition(State, ErrorNode);
+  }
+}
+
+void MPIChecker::checkMissingWaits(SymbolReaper &SymReaper,
+                                   CheckerContext &Ctx) const {
+  if (!SymReaper.hasDeadSymbols())
+    return;
+
+  ProgramStateRef State = Ctx.getState();
+  const auto &Requests = State->get<RequestMap>();
+  if (Requests.isEmpty())
+    return;
+
+  static CheckerProgramPointTag Tag("MPI-Checker", "MissingWait");
+  ExplodedNode *ErrorNode{nullptr};
+
+  auto ReqMap = State->get<RequestMap>();
+  for (const auto &Req : ReqMap) {
+    if (!SymReaper.isLiveRegion(Req.first)) {
+      if (Req.second.CurrentState == Request::State::Nonblocking) {
+
+        if (!ErrorNode) {
+          ErrorNode = Ctx.generateNonFatalErrorNode(State, &Tag);
+          State = ErrorNode->getState();
+        }
+        BReporter->reportMissingWait(Req.second, Req.first, ErrorNode);
+      }
+      State = State->remove<RequestMap>(Req.first);
+    }
+  }
+
+  // Transition to update the state regarding removed requests.
+  if (!ErrorNode) {
+    Ctx.addTransition(State);
+  } else {
+    Ctx.addTransition(State, ErrorNode);
+  }
+}
+
+void MPIChecker::checkMissingWaitsGlobals(ExplodedGraph &G) const {
+
+  auto NodeIt = G.eop_begin();
+  const auto NodeEndIt = G.eop_end();
+
+  while (NodeIt != NodeEndIt) {
+    ProgramStateRef State = (*NodeIt)->getState();
+    const auto &ReqMap = State->get<RequestMap>();
+    for (const auto &Req : ReqMap) {
+      if (Req.second.CurrentState == Request::State::Nonblocking) {
+        BReporter->reportMissingWait(Req.second, Req.first, *NodeIt);
+      }
+    }
+    ++NodeIt;
+  }
+}
+
+const MemRegion *MPIChecker::topRegionUsedByWait(const CallEvent &CE) const {
+
+  if (FuncClassifier->isMPI_Wait(CE.getCalleeIdentifier())) {
+    return CE.getArgSVal(0).getAsRegion();
+  } else if (FuncClassifier->isMPI_Waitall(CE.getCalleeIdentifier())) {
+    return CE.getArgSVal(1).getAsRegion();
+  } else {
+    return (const MemRegion *)nullptr;
+  }
+}
+
+void MPIChecker::allRegionsUsedByWait(
+    llvm::SmallVector<const MemRegion *, 2> &ReqRegions,
+    const MemRegion *const MR, const CallEvent &CE, CheckerContext &Ctx) const {
+
+  MemRegionManager *const RegionManager = MR->getMemRegionManager();
+
+  if (FuncClassifier->isMPI_Waitall(CE.getCalleeIdentifier())) {
+    const MemRegion *SuperRegion{nullptr};
+    if (const ElementRegion *const ER = MR->getAs<ElementRegion>()) {
+      SuperRegion = ER->getSuperRegion();
+    }
+
+    // A single request is passed to MPI_Waitall.
+    if (!SuperRegion) {
+      ReqRegions.push_back(MR);
+      return;
+    }
+
+    const auto &size = Ctx.getStoreManager().getSizeInElements(
+        Ctx.getState(), SuperRegion,
+        CE.getArgExpr(1)->getType()->getPointeeType());
+    const llvm::APSInt &ArrSize = size.getAs<nonloc::ConcreteInt>()->getValue();
+
+    for (size_t i = 0; i < ArrSize; ++i) {
+      const NonLoc Idx = Ctx.getSValBuilder().makeArrayIndex(i);
+
+      const ElementRegion *const ER = RegionManager->getElementRegion(
+          CE.getArgExpr(1)->getType()->getPointeeType(), Idx, SuperRegion,
+          Ctx.getASTContext());
+
+      ReqRegions.push_back(ER->getAs<MemRegion>());
+    }
+  } else if (FuncClassifier->isMPI_Wait(CE.getCalleeIdentifier())) {
+    ReqRegions.push_back(MR);
+  }
+}
+
+} // end of namespace: mpi
+} // end of namespace: ento
+} // end of namespace: clang
+
+// Registers the checker for static analysis.
+void clang::ento::registerMPIChecker(CheckerManager &MGR) {
+  MGR.registerChecker<clang::ento::mpi::MPIChecker>();
+}

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.h

+//===-- MPIFunctionClassifier.h - classifies MPI functions ----*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file defines functionality to identify and classify MPI functions.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPIFUNCTIONCLASSIFIER_H
+#define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPIFUNCTIONCLASSIFIER_H
+
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+namespace clang {
+namespace ento {
+namespace mpi {
+
+class MPIFunctionClassifier {
+public:
+  MPIFunctionClassifier(AnalysisManager &AM) { identifierInit(AM); }
+
+  // general identifiers
+  bool isMPIType(const IdentifierInfo *const IdentInfo) const;
+  bool isNonBlockingType(const IdentifierInfo *const IdentInfo) const;
+
+  // point-to-point identifiers
+  bool isPointToPointType(const IdentifierInfo *const IdentInfo) const;
+
+  // collective identifiers
+  bool isCollectiveType(const IdentifierInfo *const IdentInfo) const;
+  bool isCollToColl(const IdentifierInfo *const IdentInfo) const;
+  bool isScatterType(const IdentifierInfo *const IdentInfo) const;
+  bool isGatherType(const IdentifierInfo *const IdentInfo) const;
+  bool isAllgatherType(const IdentifierInfo *const IdentInfo) const;
+  bool isAlltoallType(const IdentifierInfo *const IdentInfo) const;
+  bool isReduceType(const IdentifierInfo *const IdentInfo) const;
+  bool isBcastType(const IdentifierInfo *const IdentInfo) const;
+
+  // additional identifiers
+  bool isMPI_Wait(const IdentifierInfo *const IdentInfo) const;
+  bool isMPI_Waitall(const IdentifierInfo *const IdentInfo) const;
+  bool isWaitType(const IdentifierInfo *const IdentInfo) const;
+
+private:
+  // Initializes function identifiers, to recognize them during analysis.
+  void identifierInit(AnalysisManager &AM);
+  void initPointToPointIdentifiers(AnalysisManager &AM);
+  void initCollectiveIdentifiers(AnalysisManager &AM);
+  void initAdditionalIdentifiers(AnalysisManager &AM);
+
+  // The containers are used, to enable classification of MPI-functions during
+  // analysis.
+  llvm::SmallVector<IdentifierInfo *, 12> MPINonBlockingTypes;
+
+  llvm::SmallVector<IdentifierInfo *, 10> MPIPointToPointTypes;
+  llvm::SmallVector<IdentifierInfo *, 16> MPICollectiveTypes;
+
+  llvm::SmallVector<IdentifierInfo *, 4> MPIPointToCollTypes;
+  llvm::SmallVector<IdentifierInfo *, 4> MPICollToPointTypes;
+  llvm::SmallVector<IdentifierInfo *, 6> MPICollToCollTypes;
+
+  llvm::SmallVector<IdentifierInfo *, 32> MPIType;
+
+  // point-to-point functions
+  IdentifierInfo *IdentInfo_MPI_Send{nullptr}, *IdentInfo_MPI_Isend{nullptr},
+      *IdentInfo_MPI_Ssend{nullptr}, *IdentInfo_MPI_Issend{nullptr},
+      *IdentInfo_MPI_Bsend{nullptr}, *IdentInfo_MPI_Ibsend{nullptr},
+      *IdentInfo_MPI_Rsend{nullptr}, *IdentInfo_MPI_Irsend{nullptr},
+      *IdentInfo_MPI_Recv{nullptr}, *IdentInfo_MPI_Irecv{nullptr};
+
+  // collective functions
+  IdentifierInfo *IdentInfo_MPI_Scatter{nullptr},
+      *IdentInfo_MPI_Iscatter{nullptr}, *IdentInfo_MPI_Gather{nullptr},
+      *IdentInfo_MPI_Igather{nullptr}, *IdentInfo_MPI_Allgather{nullptr},
+      *IdentInfo_MPI_Iallgather{nullptr}, *IdentInfo_MPI_Bcast{nullptr},
+      *IdentInfo_MPI_Ibcast{nullptr}, *IdentInfo_MPI_Reduce{nullptr},
+      *IdentInfo_MPI_Ireduce{nullptr}, *IdentInfo_MPI_Allreduce{nullptr},
+      *IdentInfo_MPI_Iallreduce{nullptr}, *IdentInfo_MPI_Alltoall{nullptr},
+      *IdentInfo_MPI_Ialltoall{nullptr}, *IdentInfo_MPI_Barrier{nullptr};
+
+  // additional functions
+  IdentifierInfo *IdentInfo_MPI_Comm_rank{nullptr},
+      *IdentInfo_MPI_Comm_size{nullptr}, *IdentInfo_MPI_Wait{nullptr},
+      *IdentInfo_MPI_Waitall{nullptr};
+};
+
+} // end of namespace: mpi
+} // end of namespace: ento
+} // end of namespace: clang
+
+#endif

lib/StaticAnalyzer/Checkers/MPI-Checker/MPIFunctionClassifier.cpp

+//===-- MPIFunctionClassifier.cpp - classifies MPI functions ----*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file defines functionality to identify and classify MPI functions.
+///
+//===----------------------------------------------------------------------===//
+
+#include "MPIFunctionClassifier.h"
+#include "llvm/ADT/STLExtras.h"
+
+namespace clang {
+namespace ento {
+namespace mpi {
+
+void MPIFunctionClassifier::identifierInit(AnalysisManager &AM) {
+  // Initialize function identifiers.
+  initPointToPointIdentifiers(AM);
+  initCollectiveIdentifiers(AM);
+  initAdditionalIdentifiers(AM);
+}
+
+void MPIFunctionClassifier::initPointToPointIdentifiers(
+    clang::ento::AnalysisManager &AM) {
+  ASTContext &ASTCtx = AM.getASTContext();
+
+  // Copy identifiers into the correct classification containers.
+  IdentInfo_MPI_Send = &ASTCtx.Idents.get("MPI_Send");
+  MPIPointToPointTypes.push_back(IdentInfo_MPI_Send);
+  MPIType.push_back(IdentInfo_MPI_Send);
+  assert(IdentInfo_MPI_Send);
+
+  IdentInfo_MPI_Isend = &ASTCtx.Idents.get("MPI_Isend");
+  MPIPointToPointTypes.push_back(IdentInfo_MPI_Isend);
+  MPINonBlockingTypes.push_back(IdentInfo_MPI_Isend);
+  MPIType.push_back(IdentInfo_MPI_Isend);
+  assert(IdentInfo_MPI_Isend);
+
+  IdentInfo_MPI_Ssend = &ASTCtx.Idents.get("MPI_Ssend");
+  MPIPointToPointTypes.push_back(IdentInfo_MPI_Ssend);
+  MPIType.push_back(IdentInfo_MPI_Ssend);
+  assert(IdentInfo_MPI_Ssend);
+
+  IdentInfo_MPI_Issend = &ASTCtx.Idents.get("MPI_Issend");
+  MPIPointToPointTypes.push_back(IdentInfo_MPI_Issend);
+  MPINonBlockingTypes.push_back(IdentInfo_MPI_Issend);
+  MPIType.push_back(IdentInfo_MPI_Issend);
+  assert(IdentInfo_MPI_Issend);
+
+  IdentInfo_MPI_Bsend = &ASTCtx.Idents.get("MPI_Bsend");
+  MPIPointToPointTypes.push_back(IdentInfo_MPI_Bsend);
+  MPIType.push_back(IdentInfo_MPI_Bsend);
+  assert(IdentInfo_MPI_Bsend);
+
+  IdentInfo_MPI_Ibsend = &ASTCtx.Idents.get("MPI_Ibsend");
+  MPIPointToPointTypes.push_back(IdentInfo_MPI_Ibsend);
+  MPINonBlockingTypes.push_back(IdentInfo_MPI_Ibsend);
+  MPIType.push_back(IdentInfo_MPI_Ibsend);
+  assert(IdentInfo_MPI_Ibsend);
+
+  IdentInfo_MPI_Rsend = &ASTCtx.Idents.get("MPI_Rsend");
+  MPIPointToPointTypes.push_back(IdentInfo_MPI_Rsend);
+  MPIType.push_back(IdentInfo_MPI_Rsend);
+  assert(IdentInfo_MPI_Rsend);
+
+  IdentInfo_MPI_Irsend = &ASTCtx.Idents.get("MPI_Irsend");
+  MPIPointToPointTypes.push_back(IdentInfo_MPI_Irsend);
+  MPIType.push_back(IdentInfo_MPI_Irsend);
+  assert(IdentInfo_MPI_Irsend);
+
+  IdentInfo_MPI_Recv = &ASTCtx.Idents.get("MPI_Recv");
+  MPIPointToPointTypes.push_back(IdentInfo_MPI_Recv);
+  MPIType.push_back(IdentInfo_MPI_Recv);
+  assert(IdentInfo_MPI_Recv);
+
+  IdentInfo_MPI_Irecv = &ASTCtx.Idents.get("MPI_Irecv");
+  MPIPointToPointTypes.push_back(IdentInfo_MPI_Irecv);
+  MPINonBlockingTypes.push_back(IdentInfo_MPI_Irecv);
+  MPIType.push_back(IdentInfo_MPI_Irecv);
+  assert(IdentInfo_MPI_Irecv);
+}
+
+void MPIFunctionClassifier::initCollectiveIdentifiers(AnalysisManager &AM) {
+  ASTContext &ASTCtx = AM.getASTContext();
+
+  // Copy identifiers into the correct classification containers.
+  IdentInfo_MPI_Scatter = &ASTCtx.Idents.get("MPI_Scatter");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Scatter);
+  MPIPointToCollTypes.push_back(IdentInfo_MPI_Scatter);
+  MPIType.push_back(IdentInfo_MPI_Scatter);
+  assert(IdentInfo_MPI_Scatter);
+
+  IdentInfo_MPI_Iscatter = &ASTCtx.Idents.get("MPI_Iscatter");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Iscatter);
+  MPIPointToCollTypes.push_back(IdentInfo_MPI_Iscatter);
+  MPINonBlockingTypes.push_back(IdentInfo_MPI_Iscatter);
+  MPIType.push_back(IdentInfo_MPI_Iscatter);
+  assert(IdentInfo_MPI_Iscatter);
+
+  IdentInfo_MPI_Gather = &ASTCtx.Idents.get("MPI_Gather");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Gather);
+  MPICollToPointTypes.push_back(IdentInfo_MPI_Gather);
+  MPIType.push_back(IdentInfo_MPI_Gather);
+  assert(IdentInfo_MPI_Gather);
+
+  IdentInfo_MPI_Igather = &ASTCtx.Idents.get("MPI_Igather");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Igather);
+  MPICollToPointTypes.push_back(IdentInfo_MPI_Igather);
+  MPINonBlockingTypes.push_back(IdentInfo_MPI_Igather);
+  MPIType.push_back(IdentInfo_MPI_Igather);
+  assert(IdentInfo_MPI_Igather);
+
+  IdentInfo_MPI_Allgather = &ASTCtx.Idents.get("MPI_Allgather");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Allgather);
+  MPICollToCollTypes.push_back(IdentInfo_MPI_Allgather);
+  MPIType.push_back(IdentInfo_MPI_Allgather);
+  assert(IdentInfo_MPI_Allgather);
+
+  IdentInfo_MPI_Iallgather = &ASTCtx.Idents.get("MPI_Iallgather");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Iallgather);
+  MPICollToCollTypes.push_back(IdentInfo_MPI_Iallgather);
+  MPINonBlockingTypes.push_back(IdentInfo_MPI_Iallgather);
+  MPIType.push_back(IdentInfo_MPI_Iallgather);
+  assert(IdentInfo_MPI_Iallgather);
+
+  IdentInfo_MPI_Bcast = &ASTCtx.Idents.get("MPI_Bcast");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Bcast);
+  MPIPointToCollTypes.push_back(IdentInfo_MPI_Bcast);
+  MPIType.push_back(IdentInfo_MPI_Bcast);
+  assert(IdentInfo_MPI_Bcast);
+
+  IdentInfo_MPI_Ibcast = &ASTCtx.Idents.get("MPI_Ibcast");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Ibcast);
+  MPIPointToCollTypes.push_back(IdentInfo_MPI_Ibcast);
+  MPINonBlockingTypes.push_back(IdentInfo_MPI_Ibcast);
+  MPIType.push_back(IdentInfo_MPI_Ibcast);
+  assert(IdentInfo_MPI_Ibcast);
+
+  IdentInfo_MPI_Reduce = &ASTCtx.Idents.get("MPI_Reduce");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Reduce);
+  MPICollToPointTypes.push_back(IdentInfo_MPI_Reduce);
+  MPIType.push_back(IdentInfo_MPI_Reduce);
+  assert(IdentInfo_MPI_Reduce);
+
+  IdentInfo_MPI_Ireduce = &ASTCtx.Idents.get("MPI_Ireduce");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Ireduce);
+  MPICollToPointTypes.push_back(IdentInfo_MPI_Ireduce);
+  MPINonBlockingTypes.push_back(IdentInfo_MPI_Ireduce);
+  MPIType.push_back(IdentInfo_MPI_Ireduce);
+  assert(IdentInfo_MPI_Ireduce);
+
+  IdentInfo_MPI_Allreduce = &ASTCtx.Idents.get("MPI_Allreduce");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Allreduce);
+  MPICollToCollTypes.push_back(IdentInfo_MPI_Allreduce);
+  MPIType.push_back(IdentInfo_MPI_Allreduce);
+  assert(IdentInfo_MPI_Allreduce);
+
+  IdentInfo_MPI_Iallreduce = &ASTCtx.Idents.get("MPI_Iallreduce");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Iallreduce);
+  MPICollToCollTypes.push_back(IdentInfo_MPI_Iallreduce);
+  MPINonBlockingTypes.push_back(IdentInfo_MPI_Iallreduce);
+  MPIType.push_back(IdentInfo_MPI_Iallreduce);
+  assert(IdentInfo_MPI_Iallreduce);
+
+  IdentInfo_MPI_Alltoall = &ASTCtx.Idents.get("MPI_Alltoall");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Alltoall);
+  MPICollToCollTypes.push_back(IdentInfo_MPI_Alltoall);
+  MPIType.push_back(IdentInfo_MPI_Alltoall);
+  assert(IdentInfo_MPI_Alltoall);
+
+  IdentInfo_MPI_Ialltoall = &ASTCtx.Idents.get("MPI_Ialltoall");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Ialltoall);
+  MPICollToCollTypes.push_back(IdentInfo_MPI_Ialltoall);
+  MPINonBlockingTypes.push_back(IdentInfo_MPI_Ialltoall);
+  MPIType.push_back(IdentInfo_MPI_Ialltoall);
+  assert(IdentInfo_MPI_Ialltoall);
+}
+
+void MPIFunctionClassifier::initAdditionalIdentifiers(AnalysisManager &AM) {
+  ASTContext &ASTCtx = AM.getASTContext();
+
+  IdentInfo_MPI_Comm_rank = &ASTCtx.Idents.get("MPI_Comm_rank");
+  MPIType.push_back(IdentInfo_MPI_Comm_rank);
+  assert(IdentInfo_MPI_Comm_rank);
+
+  IdentInfo_MPI_Comm_size = &ASTCtx.Idents.get("MPI_Comm_size");
+  MPIType.push_back(IdentInfo_MPI_Comm_size);
+  assert(IdentInfo_MPI_Comm_size);
+
+  IdentInfo_MPI_Wait = &ASTCtx.Idents.get("MPI_Wait");
+  MPIType.push_back(IdentInfo_MPI_Wait);
+  assert(IdentInfo_MPI_Wait);
+
+  IdentInfo_MPI_Waitall = &ASTCtx.Idents.get("MPI_Waitall");
+  MPIType.push_back(IdentInfo_MPI_Waitall);
+  assert(IdentInfo_MPI_Waitall);
+
+  IdentInfo_MPI_Barrier = &ASTCtx.Idents.get("MPI_Barrier");
+  MPICollectiveTypes.push_back(IdentInfo_MPI_Barrier);
+  MPIType.push_back(IdentInfo_MPI_Barrier);
+  assert(IdentInfo_MPI_Barrier);
+}
+
+// general identifiers
+bool MPIFunctionClassifier::isMPIType(const IdentifierInfo *IdentInfo) const {
+  return llvm::is_contained(MPIType, IdentInfo);
+}
+
+bool MPIFunctionClassifier::isNonBlockingType(
+    const IdentifierInfo *IdentInfo) const {
+  return llvm::is_contained(MPINonBlockingTypes, IdentInfo);
+}
+
+// point-to-point identifiers
+bool MPIFunctionClassifier::isPointToPointType(
+    const IdentifierInfo *IdentInfo) const {
+  return llvm::is_contained(MPIPointToPointTypes, IdentInfo);
+}
+
+// collective identifiers
+bool MPIFunctionClassifier::isCollectiveType(
+    const IdentifierInfo *IdentInfo) const {
+  return llvm::is_contained(MPICollectiveTypes, IdentInfo);
+}
+
+bool MPIFunctionClassifier::isCollToColl(
+    const IdentifierInfo *IdentInfo) const {
+  return llvm::is_contained(MPICollToCollTypes, IdentInfo);
+}
+
+bool MPIFunctionClassifier::isScatterType(
+    const IdentifierInfo *IdentInfo) const {
+  return IdentInfo == IdentInfo_MPI_Scatter ||
+         IdentInfo == IdentInfo_MPI_Iscatter;
+}
+
+bool MPIFunctionClassifier::isGatherType(
+    const IdentifierInfo *IdentInfo) const {
+  return IdentInfo == IdentInfo_MPI_Gather ||
+         IdentInfo == IdentInfo_MPI_Igather ||
+         IdentInfo == IdentInfo_MPI_Allgather ||
+         IdentInfo == IdentInfo_MPI_Iallgather;
+}
+
+bool MPIFunctionClassifier::isAllgatherType(
+    const IdentifierInfo *IdentInfo) const {
+  return IdentInfo == IdentInfo_MPI_Allgather ||
+         IdentInfo == IdentInfo_MPI_Iallgather;
+}
+
+bool MPIFunctionClassifier::isAlltoallType(
+    const IdentifierInfo *IdentInfo) const {
+  return IdentInfo == IdentInfo_MPI_Alltoall ||
+         IdentInfo == IdentInfo_MPI_Ialltoall;
+}
+
+bool MPIFunctionClassifier::isBcastType(const IdentifierInfo *IdentInfo) const {
+  return IdentInfo == IdentInfo_MPI_Bcast || IdentInfo == IdentInfo_MPI_Ibcast;
+}
+
+bool MPIFunctionClassifier::isReduceType(
+    const IdentifierInfo *IdentInfo) const {
+  return IdentInfo == IdentInfo_MPI_Reduce ||
+         IdentInfo == IdentInfo_MPI_Ireduce ||
+         IdentInfo == IdentInfo_MPI_Allreduce ||
+         IdentInfo == IdentInfo_MPI_Iallreduce;
+}
+
+// additional identifiers
+bool MPIFunctionClassifier::isMPI_Wait(const IdentifierInfo *IdentInfo) const {
+  return IdentInfo == IdentInfo_MPI_Wait;
+}
+
+bool MPIFunctionClassifier::isMPI_Waitall(
+    const IdentifierInfo *IdentInfo) const {
+  return IdentInfo == IdentInfo_MPI_Waitall;
+}
+
+bool MPIFunctionClassifier::isWaitType(const IdentifierInfo *IdentInfo) const {
+  return IdentInfo == IdentInfo_MPI_Wait || IdentInfo == IdentInfo_MPI_Waitall;
+}
+
+} // end of namespace: mpi
+} // end of namespace: ento
+} // end of namespace: clang

lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h

+//===-- MPITypes.h - Functionality to model MPI concepts --------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file provides definitions, to model the schema of MPI point-to-point
+/// functions and MPI requests. The mpi::Request class defines a wrapper
+/// class, in order to make MPI requests trackable for path-sensitive analysis.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPITYPES_H
+#define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_MPICHECKER_MPITYPES_H
+
+#include "MPIFunctionClassifier.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "llvm/ADT/SmallSet.h"
+
+namespace clang {
+namespace ento {
+namespace mpi {
+
+class Request {
+public:
+  enum State : unsigned char { Nonblocking, Wait };
+
+  Request(State S) : CurrentState{S} {}
+
+  void Profile(llvm::FoldingSetNodeID &Id) const {
+    Id.AddInteger(CurrentState);
+  }
+
+  bool operator==(const Request &ToCompare) const {
+    return CurrentState == ToCompare.CurrentState;
+  }
+
+  const State CurrentState;
+};
+
+} // end of namespace: mpi
+} // end of namespace: ento
+} // end of namespace: clang
+
+// The RequestMap stores MPI requests which are identified by their memory
+// region. Requests are used in MPI to complete nonblocking operations with wait
+// operations. A custom map implementation is used, in order to make it
+// available in an arbitrary amount of translation units.
+struct RequestMap {};
+typedef llvm::ImmutableMap<const clang::ento::MemRegion *,
+                           clang::ento::mpi::Request>
+    RequestMapImpl;
+template <>
+struct clang::ento::ProgramStateTrait<RequestMap>
+    : public clang::ento::ProgramStatePartialTrait<RequestMapImpl> {
+  static void *GDMIndex() {
+    static int index = 0;
+    return &index;
+  }
+};
+
+#endif

test/Analysis/MPIChecker.cpp

+// RUN: %clang_cc1 -analyze -analyzer-checker=optin.mpi.MPI-Checker -verify %s
+
+// MPI-Checker makes no assumptions about details of an MPI implementation.
+// Typedefs and constants are compared as strings.
+
+#include "MPIMock.h"
+
+//integration tests
+
+void matchedWait1() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+    MPI_Request sendReq1, recvReq1;
+    MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 3, MPI_COMM_WORLD, &sendReq1);
+    MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 3, MPI_COMM_WORLD, &recvReq1);
+
+    MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+    MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+  }
+} // no error
+
+void matchedWait2() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+    MPI_Request sendReq1, recvReq1;
+    MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 4, MPI_COMM_WORLD, &sendReq1);
+    MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 4, MPI_COMM_WORLD, &recvReq1);
+    MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+    MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+  }
+} // no error
+
+void matchedWait3() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank >= 0) {
+    MPI_Request sendReq1, recvReq1;
+    MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 5, MPI_COMM_WORLD, &sendReq1);
+    MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 5, MPI_COMM_WORLD, &recvReq1);
+
+    if (rank > 1000) {
+      MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+      MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+    } else {
+      MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+      MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+    }
+  }
+} // no error
+
+void missingWait1() { // Check missing wait for dead region.
+  int rank = 0;
+  double buf = 0;
+  MPI_Request sendReq1;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
+              &sendReq1); // expected-note{{Request is previously used by nonblocking call here.}}
+} // expected-warning{{Request 'sendReq1' has no matching wait.}}
+
+
+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-note{{Request is previously used by nonblocking call here.}}
+} // expected-warning{{Request 'sendReq1' has no matching wait.}}
+
+void missingWait3() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank == 0) {
+  } else {
+    MPI_Request sendReq1, recvReq1;
+
+    MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 2, MPI_COMM_WORLD, &sendReq1);
+    MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 2, MPI_COMM_WORLD, &recvReq1); // expected-warning{{Request 'sendReq1' has no matching wait.}}
+    MPI_Wait(&recvReq1, MPI_STATUS_IGNORE);
+  }
+}
+
+void doubleNonblocking() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  if (rank == 1) {
+  } else {
+    MPI_Request sendReq1;
+
+    MPI_Isend(&buf, 1, MPI_DOUBLE, rank + 1, 6, MPI_COMM_WORLD, &sendReq1); // expected-note{{Request is previously used by nonblocking call here. }}
+    MPI_Irecv(&buf, 1, MPI_DOUBLE, rank - 1, 6, MPI_COMM_WORLD, &sendReq1); // expected-warning{{Double nonblocking on request 'sendReq1'.}}
+    MPI_Wait(&sendReq1, MPI_STATUS_IGNORE);
+  }
+}
+
+void doubleNonblocking2() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+
+  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.}}
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &req); // expected-warning{{Double nonblocking on request 'req'.}}
+  MPI_Wait(&req, MPI_STATUS_IGNORE);
+}
+
+void doubleNonblocking3() {
+  typedef struct { MPI_Request req; } ReqStruct;
+
+  ReqStruct rs;
+  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, &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);
+}
+
+void doubleNonblocking4() {
+  int rank = 0;
+  double buf = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+
+  MPI_Request req;
+  for (int i = 0; i < 2; ++i) {
+    MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD, &req); // expected-warning{{Double nonblocking on request 'req'.}}
+  }
+  MPI_Wait(&req, MPI_STATUS_IGNORE);
+}
+
+void missingNonBlocking() {
+  int rank = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  MPI_Request sendReq1[10][10][10];
+  MPI_Wait(&sendReq1[1][7][9], MPI_STATUS_IGNORE); // expected-warning{{Request 'sendReq1[1][7][9]' has no matching nonblocking call.}}
+}
+
+void missingNonBlocking2() {
+  int rank = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  typedef struct { MPI_Request req[2][2]; } ReqStruct;
+  ReqStruct rs;
+  MPI_Request *r = &rs.req[0][1];
+  MPI_Wait(r, MPI_STATUS_IGNORE); // expected-warning{{Request 'rs.req[0][1]' has no matching nonblocking call.}}
+}
+
+void missingNonBlockingMultiple() {
+  int rank = 0;
+  MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+  MPI_Request sendReq[4];
+  for (int i = 0; i < 4; ++i) {
+    MPI_Wait(&sendReq[i], MPI_STATUS_IGNORE); // expected-warning-re 1+{{Request {{.*}} has no matching nonblocking call.}}
+  }
+}
+
+void noDoubleRequestUsage() {
+  typedef struct {
+    MPI_Request req;
+    MPI_Request req2;
+  } ReqStruct;
+
+  ReqStruct rs;
+  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,
+              &rs.req);
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
+              &rs.req2);
+  MPI_Wait(&rs.req, MPI_STATUS_IGNORE);
+  MPI_Wait(&rs.req2, MPI_STATUS_IGNORE);
+}
+
+void noDoubleRequestUsage2() {
+  typedef struct {
+    MPI_Request req[2];
+    MPI_Request req2;
+  } ReqStruct;
+
+  ReqStruct rs;
+  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,
+              &rs.req[0]);
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
+              &rs.req[1]);
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
+              &rs.req2);
+  MPI_Wait(&rs.req[0], MPI_STATUS_IGNORE);
+  MPI_Wait(&rs.req[1], MPI_STATUS_IGNORE);
+  MPI_Wait(&rs.req2, MPI_STATUS_IGNORE);
+}
+
+void nestedRequest() {
+  typedef struct {
+    MPI_Request req[2];
+    MPI_Request req2;
+  } ReqStruct;
+
+  ReqStruct rs;
+  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,
+              &rs.req[0]);
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
+              &rs.req[1]);
+  MPI_Ireduce(MPI_IN_PLACE, &buf, 1, MPI_DOUBLE, MPI_SUM, 0, MPI_COMM_WORLD,
+              &rs.req2);
+  MPI_Waitall(2, rs.req, MPI_STATUSES_IGNORE);
+  MPI_Wait(&rs.req2, MPI_STATUS_IGNORE);
+}
+
+void singleRequestInWaitall() {
+  MPI_Request r;
+  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,
+              &r);
+  MPI_Waitall(1, &r, MPI_STATUSES_IGNORE);
+}