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 cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits