Alexander_Droste added inline comments.
================
Comment at: lib/StaticAnalyzer/Core/MemRegion.cpp:653
@@ +652,3 @@
+ // name by calling 'getDescriptiveName' recursively.
+ else {
+ std::string Idx = ER->getDescriptiveName(false);
----------------
zaks.anna wrote:
> xazax.hun wrote:
> > Alexander_Droste wrote:
> > > I wasn't able to build a test case yet for which the analyzer could not
> > > determine the constant value. Is there a way to trick the analyzer so
> > > that the else case is used ? Then I could test for something like
> > > `'sendReq1[a][7][b]'`.
> > You can try use a value returned from a function that has an unknown body.
> > E.g.:
> >
> > int getUnknown();
> >
> > void f() {
> > int a = getUnKnown();
> > }
> What happens when you try 'sendReq1[a][7][b]'? Does it know the values for
> "a" and "b" for some reason? If 'a' would be an input parameter and the
> analyzer did not see a call site, it won't know the value of 'a'.
If the return value of a function is used for which the body is not known Clang
crashes.
```
int getUnknown(void);
int idxA = getUnknown();
MPI_Request sendReq1[10][10][10];
MPI_Wait(&sendReq1[1][idxA][9], MPI_STATUS_IGNORE); //
expected-warning{{Request 'sendReq1[1][7][9]' has no matching nonblocking
call.}}
```
Clang also crashes if the index-variable is not initialized.
```
int idxA;
MPI_Request sendReq1[10][10][10];
MPI_Wait(&sendReq1[1][idxA][9], MPI_STATUS_IGNORE); //
expected-warning{{Request 'sendReq1[1][7][9]' has no matching nonblocking
call.}}
```
In case the variable is initialized with a constant, the `ConcreteInt` is
determined.
================
Comment at: test/Analysis/MemRegion.cpp:3
@@ +2,3 @@
+
+#include "MPIMock.h"
+
----------------
zaks.anna wrote:
> Alexander_Droste wrote:
> > The problem about these tests is that they introduce a cyclic commit
> > dependency. MPI-Checker depends on `getDescriptiveName`.
> > `getDescriptiveName` depends on MPI-Checker because of the tests.
> >
> > Further, MPI-Checker depends on this function:
> >
> > ```
> > SourceRange MemRegion::sourceRange() const {
> > const VarRegion *const VR = dyn_cast<VarRegion>(this->getBaseRegion());
> > const FieldRegion *const FR = dyn_cast<FieldRegion>(this);
> > const CXXBaseObjectRegion*const COR = dyn_cast<CXXBaseObjectRegion>(this);
> >
> > // Check for more specific regions first.
> > // FieldRegion
> > if (FR) {
> > return FR->getDecl()->getSourceRange();
> > }
> > // CXXBaseObjectRegion
> > else if (COR) {
> > return COR->getDecl()->getSourceRange();
> > }
> > // VarRegion
> > else if (VR) {
> > return VR->getDecl()->getSourceRange();
> > }
> > // Return invalid source range (can be checked by client).
> > else {
> > return SourceRange{};
> > }
> > }
> > ```
> > Initially, my idea was to submit the `sourceRange` patch after
> > `getDescriptiveName`. Maybe it would be most convenient, to include the
> > `sourceRange` function into this patch and finally commit all connected
> > patches in one go.
> > getDescriptiveName depends on MPI-Checker because of the tests.
>
> Would committing the tests separately fix the cyclic commit dependency
> problem? (Though, I'd still prefer to review them along with this patch.)
Yes, that would fix the cyclic commit dependency.
http://reviews.llvm.org/D16044
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
