[clang-tools-extra] r278553 - [clang-tidy] MPIBufferDerefCheck

Author: alexander_droste Date: Fri Aug 12 14:30:31 2016 New Revision: 278553 URL: http://llvm.org/viewvc/llvm-project?rev=278553&view=rev Log: [clang-tidy] MPIBufferDerefCheck ... This check verifies if a buffer passed to an MPI (Message Passing Interface) function is sufficiently dereferenced. B

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

This revision was automatically updated to reflect the committed changes. Closed by commit rL278553: [clang-tidy] MPIBufferDerefCheck (authored by Alexander_Droste). Changed prior to commit: https://reviews.llvm.org/D22729?vs=66971&id=67884#toc Repository: rL LLVM https://reviews.llvm.org/

r278534 - Revert test commit

Author: alexander_droste Date: Fri Aug 12 12:46:23 2016 New Revision: 278534 URL: http://llvm.org/viewvc/llvm-project?rev=278534&view=rev Log: Revert test commit Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MPI-Checker

r278533 - Test commit - first LLVM repo commit

Author: alexander_droste Date: Fri Aug 12 12:43:58 2016 New Revision: 278533 URL: http://llvm.org/viewvc/llvm-project?rev=278533&view=rev Log: Test commit - first LLVM repo commit Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MPI-Checker/MPITypes.h Modified: cfe/trunk/lib/StaticAnalyzer/Ch

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste added a comment. No but I guess it would be a good idea to ask for commit access. I'll proceed like suggested here: http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access. https://reviews.llvm.org/D22729 ___ cfe-commits

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste added a comment. Great; thanks again for the review! https://reviews.llvm.org/D22729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 66971. Alexander_Droste marked an inline comment as done. Alexander_Droste added a comment. - check for `nullptr` in `addBuffer` https://reviews.llvm.org/D22729 Files: clang-tidy/mpi/BufferDerefCheck.cpp clang-tidy/mpi/BufferDerefCheck.h clan

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste marked an inline comment as done. Comment at: clang-tidy/mpi/BufferDerefCheck.cpp:88 @@ +87,3 @@ +while (true) { + if (BufferType->isPointerType()) { +BufferType = BufferType->getPointeeType().getTypePtr(); I just realized, you me

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 66940. Alexander_Droste marked 3 inline comments as done. Alexander_Droste added a comment. - add release docs - simplify reverse for loop -> `rbegin`, `rend` https://reviews.llvm.org/D22729 Files: clang-tidy/mpi/BufferDerefCheck.cpp clang-tidy

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste marked 4 inline comments as done. Comment at: clang-tidy/mpi/BufferDerefCheck.cpp:87 @@ +86,3 @@ +// Capture the depth and types of indirections for the passed buffer. +while (true) { + if (BufferType->isPointerType()) { hokein wrote:

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 66761. Alexander_Droste added a comment. - add needed extra line for docs https://reviews.llvm.org/D22729 Files: clang-tidy/mpi/BufferDerefCheck.cpp clang-tidy/mpi/BufferDerefCheck.h clang-tidy/mpi/CMakeLists.txt clang-tidy/mpi/MPITidyModul

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 66760. Alexander_Droste added a comment. - clarify lambda argument with comment - explicit cast to `int` for the number of indirections in for loop https://reviews.llvm.org/D22729 Files: clang-tidy/mpi/BufferDerefCheck.cpp clang-tidy/mpi/Buffer

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste marked 3 inline comments as done. Alexander_Droste added a comment. https://reviews.llvm.org/D22729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste added a comment. Thanks for re-committing this patch! Repository: rL LLVM https://reviews.llvm.org/D21962 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste added a comment. > Looks like the dependency of this patch https://reviews.llvm.org/D21962 got > reverted by some reasons. I know, I updated the patch after that, in order to fix the problem. https://reviews.llvm.org/D22729 ___ c

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste added a comment. Hi, thanks for having a look! Comment at: clang-tidy/mpi/BufferDerefCheck.cpp:63 @@ +62,3 @@ +if (FuncClassifier.isReduceType(Identifier)) { + addBuffer(0); + addBuffer(1); hokein wrote: > Could you add some commen

Re: [PATCH] D20811: [analyzer] Model some library functions

Alexander_Droste added a comment. > It has been originally written as a large set of files. If you feel strongly > about it, we could merge it into a single file. That makes sense to me. > @Alexander_Droste, what do you think? Hi, I would still strongly prefer to keep them in separate files i

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste removed rL LLVM as the repository for this revision. Alexander_Droste updated this revision to Diff 65404. Alexander_Droste added a comment. Hi, thanks for the notification! Obviously, on some systems `char` is unsigned by default which is why the check now tolerates distinct si

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 65359. Alexander_Droste added a comment. - remove trailing whitespace https://reviews.llvm.org/D22729 Files: clang-tidy/mpi/BufferDerefCheck.cpp clang-tidy/mpi/BufferDerefCheck.h clang-tidy/mpi/CMakeLists.txt clang-tidy/mpi/MPITidyModule.cp

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 65239. Alexander_Droste added a comment. - fix two MPIdatatype tags https://reviews.llvm.org/D22729 Files: clang-tidy/mpi/BufferDerefCheck.cpp clang-tidy/mpi/BufferDerefCheck.h clang-tidy/mpi/CMakeLists.txt clang-tidy/mpi/MPITidyModule.cpp

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 65238. Alexander_Droste added a comment. - make `=` count accurate for `mpi-buffer-deref.rst` https://reviews.llvm.org/D22729 Files: clang-tidy/mpi/BufferDerefCheck.cpp clang-tidy/mpi/BufferDerefCheck.h clang-tidy/mpi/CMakeLists.txt clang-t

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste added a comment. > Do you have commit access? No, I don't have commit access. https://reviews.llvm.org/D21962 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

Alexander_Droste created this revision. Alexander_Droste added a reviewer: alexfh. Alexander_Droste added a subscriber: cfe-commits. This check verifies if a buffer passed to an MPI (Message Passing Interface) function is sufficiently dereferenced. Buffers should be passed as a single pointer or a

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 65222. Alexander_Droste marked an inline comment as done. Alexander_Droste added a comment. - use parentheses instead of braces https://reviews.llvm.org/D21962 Files: clang-tidy/CMakeLists.txt clang-tidy/mpi/CMakeLists.txt clang-tidy/mpi/MPIT

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 65074. Alexander_Droste added a comment. - fix typo https://reviews.llvm.org/D21962 Files: clang-tidy/CMakeLists.txt clang-tidy/mpi/CMakeLists.txt clang-tidy/mpi/MPITidyModule.cpp clang-tidy/mpi/TypeMismatchCheck.cpp clang-tidy/mpi/TypeMi

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 65072. Alexander_Droste added a comment. This update addresses all requested changes. All integration tests are still passing. https://reviews.llvm.org/D21962 Files: clang-tidy/CMakeLists.txt clang-tidy/mpi/CMakeLists.txt clang-tidy/mpi/MPIT

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste marked 5 inline comments as done. Alexander_Droste added a comment. > You need to add this to ClangTidyMain.cpp: Thanks for pointing this out and reviewing the code once more! https://reviews.llvm.org/D21962 ___ cfe-commits mailin

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 65052. Alexander_Droste added a comment. - use new `MPIFunctionClassifier.h` path - change `getAsCXXRecordDecl()->getNameAsString()` -> `getAsCXXRecordDecl()->getName()` - add comment to `addPair` lambda - rename `ArgumentExpression` to `BufferExprs`

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste marked 7 inline comments as done. Alexander_Droste added a comment. https://reviews.llvm.org/D21962 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D22671: MPI-Checker: move MPIFunctionClassifier.h

Alexander_Droste created this revision. Alexander_Droste added reviewers: zaks.anna, dcoughlin. Alexander_Droste added a subscriber: cfe-commits. This patch moves the MPIFunctionClassifier header to `clang/include/clang/StaticAnalyzer/Checkers`, in order to make it accessible in other parts of th

[PATCH] D22670: MPI-Checker fix two comments

Alexander_Droste created this revision. Alexander_Droste added reviewers: zaks.anna, dcoughlin. Alexander_Droste added a subscriber: cfe-commits. This patch corrects two comments which do not match the current behavior of the checker. https://reviews.llvm.org/D22670 Files: lib/StaticAnalyzer/

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste marked 5 inline comments as done. Alexander_Droste added a comment. https://reviews.llvm.org/D21962 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 64985. Alexander_Droste added a comment. - use `llvm::StringMap` instead of `std::map` - `getQualifiedNameAsString` -> `getName` - remove redundant map lookup - create MPIi module - replace `misc` with `mpi` within the check I created an MPI module

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste marked 2 inline comments as done. Alexander_Droste added a comment. Thanks for looking over this once more. I'll set up an extra MPI folder and rename the check. One comment inline. Comment at: clang-tidy/misc/MpiTypeMismatchCheck.cpp:218 @@ +217,3 @@ + + Str

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 63422. Alexander_Droste added a comment. - simplify `argumentType()` function http://reviews.llvm.org/D21962 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/MpiTypeMismatchCheck.cpp clang-tidy/misc/M

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 63142. Alexander_Droste added a comment. - fix typo http://reviews.llvm.org/D21962 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/MpiTypeMismatchCheck.cpp clang-tidy/misc/MpiTypeMismatchCheck.h do

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste marked 4 inline comments as done. Alexander_Droste added a comment. http://reviews.llvm.org/D21962 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste marked 22 inline comments as done. Comment at: clang-tidy/misc/MpiTypeMismatchCheck.cpp:153 @@ +152,3 @@ + {BuiltinType::LongDouble, "MPI_C_LONG_DOUBLE_COMPLEX"}}; + + const auto *Builtin = Sure, I can do that. I marked all addressed comment

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 63091. Alexander_Droste added a comment. - make static functions free functions - make static containers local to their corresponding functions - only assign the buffer type name in case of an error - fix capitalization and punctuation in error diagno

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 62694. Alexander_Droste added a comment. - remove TODO http://reviews.llvm.org/D21962 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/MpiTypeMismatchCheck.cpp clang-tidy/misc/MpiTypeMismatchCheck.h

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 62688. Alexander_Droste added a comment. Thanks for the review! This update should address the requested changes: - functions and variables are made static if possible - usage of `ArrayRef` instead of `SmallVector<..> &` - assignment init instead of

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 62653. Alexander_Droste added a comment. - remove superflous `clang::` http://reviews.llvm.org/D21962 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/MpiTypeMismatchCheck.cpp clang-tidy/misc/MpiTypeM

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 62640. Alexander_Droste added a comment. - fix comments http://reviews.llvm.org/D21962 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/MpiTypeMismatchCheck.cpp clang-tidy/misc/MpiTypeMismatchCheck.h

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 62639. Alexander_Droste added a comment. - fix some comments http://reviews.llvm.org/D21962 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/MpiTypeMismatchCheck.cpp clang-tidy/misc/MpiTypeMismatchChe

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste updated this revision to Diff 62638. Alexander_Droste added a comment. - change comment style to LLVM style - remove 'misc-m-p-i-type-mismatch' entry from `list.rst` (misc-mpi-type-mismatch is the correct one) http://reviews.llvm.org/D21962 Files: clang-tidy/misc/CMakeLists.

Re: [PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste added a comment. Hi Alexander, this is the check that verifies buffer type / MPI datatype inconsistencies that was originally part of http://reviews.llvm.org/D12761. Comment at: docs/clang-tidy/checks/list.rst:63 @@ -62,2 +62,3 @@ misc-inefficient-algorithm

[PATCH] D21962: MPITypeMismatchCheck for Clang-Tidy

Alexander_Droste created this revision. Alexander_Droste added a reviewer: alexfh. Alexander_Droste added a subscriber: cfe-commits. This check verifies if buffer type and MPI (Message Passing Interface) datatype pairs match. All MPI datatypes defined by the MPI standard (3.1) are verified by this

Re: [PATCH] D18120: Message Passing Interface mock header

Alexander_Droste closed this revision. Alexander_Droste added a comment. This was committed, as part of http://reviews.llvm.org/rL272529. http://reviews.llvm.org/D18120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

Re: [PATCH] D18309: sourceRange function for MemRegion

Alexander_Droste closed this revision. Alexander_Droste added a comment. This was committed, as part of http://reviews.llvm.org/rL272529. http://reviews.llvm.org/D18309 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

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

Alexander_Droste added a comment. Hi Devin, this is much cleaner. Looks good for me! http://reviews.llvm.org/D21081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

Alexander_Droste added a comment. F2057622: MPI-Checker.diff Hi, I now solved this, by using a `small_vector` of `MPIBugReporter`s (MPIChecker.h line: 101). So each time a report is generated, a new `MPIBugReporter` is added to that vector, in order to refer

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

Alexander_Droste added a comment. Hi Devin, thanks for fixing the GCC build errors and setting up the commit! Unfortunately, creating an `MPIBugReporter` instance only when a bug occurs does cause a separate issue. The instance seems to be freed, before the reports get flushed. void MPIChecke

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

Alexander_Droste updated this revision to Diff 54001. Alexander_Droste added a comment. - 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 rela

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

Alexander_Droste marked 20 inline comments as done. Alexander_Droste added a comment. http://reviews.llvm.org/D12761 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste marked 18 inline comments as done. Alexander_Droste added a comment. http://reviews.llvm.org/D16044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste added a comment. > The memory region for the va_list that was obtained from the analyzer in same > case was indeed an element region in the va_list checker. I fixed this issue, > and now it works properly. Then this patch might be ready to commit. :) http://reviews.llvm.org/

Re: [PATCH] D18309: sourceRange function for MemRegion

Alexander_Droste updated this revision to Diff 53983. Alexander_Droste added a comment. - remove superflous '\param' from header comment http://reviews.llvm.org/D18309 Files: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h lib/StaticAnalyzer/Core/MemRegion.cpp Index: include/cl

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

Alexander_Droste updated this revision to Diff 53982. Alexander_Droste added a comment. - 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 re

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

Alexander_Droste added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp:96 @@ +95,3 @@ +if (const CallExpr *CE = clang::dyn_cast(SP->getStmt())) { + + auto FuncIdentifier = CE->getDirectCallee()->getIdentifier(); zak

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

Alexander_Droste added inline comments. Comment at: test/Analysis/MPIChecker.cpp:99 @@ +98,3 @@ + MPI_Wait(&req, MPI_STATUS_IGNORE); +} + Alexander_Droste wrote: > zaks.anna wrote: > > This are explaining the path on which the problem occurs; the users will > >

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

Alexander_Droste added a comment. 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 enc

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste added a comment. Might the problem be in the va_list checker? Obviously the va_list variable is identified as an ElementRegion what seems not to be correct. Only if the passed region is an ElementRegion indices get appended. http://reviews.llvm.org/D16044 __

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste added a comment. > Looking at the code it seems that this data structure: > typedef SmallVector RegionVector; > tricks the function into rating the MemRegion as an ElementRegion. That's not the problem. I'm using the same data structure in the MPI-Checker patch. http://revi

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste added a comment. Looking at the code it seems that this data structure: `typedef SmallVector RegionVector;` tricks the function into rating the MemRegion as an ElementRegion. http://reviews.llvm.org/D16044 ___ cfe-commits mailing li

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste added a comment. > I tested this patch using http://reviews.llvm.org/D15227 > Unfortunately for non-array variables the getDescriptiveName returned > var_name[0]. Note the spurious [0] part. > Could you look into that? Could you provide a code example where this effect turns

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste added a comment. > What is the status of this? > As far as I understand it is blocked on that there is no checker that we > could use to test this function with unknown variables as indexes? Yes, this is blocked due to the MPI-Checker dependency. The best thing I can offer at

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

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); Alexander_Droste wrote: > zaks.

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste added a comment. > It's a bug in the checker. dyn_cast should not be called on a null pointer. > You could either check for nun or call dyn_cast_or_null. Thanks for pointing this out! Adding guards that check for `nullptr` fixed the problem. http://reviews.llvm.org/D16044

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

Alexander_Droste updated this revision to Diff 52148. Alexander_Droste added a comment. - check memory regions for `nullptr` in `checkDoubleNonblocking` and `checkUnmatchedWaits` before they get passed to `dyn_cast` http://reviews.llvm.org/D12761 Files: lib/StaticAnalyzer/Checkers/CMakeLists

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste added a comment. Here's the crash report that appears in case of the unknown function body: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 clang 0x000105f544b7 abort + 39 (Signals.inc:440) 1 clang

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

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 wr

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

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); I wasn't able to build a test c

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

Alexander_Droste updated the summary for this revision. Alexander_Droste updated this revision to Diff 52056. Alexander_Droste added a comment. - added test that uses wrapper functions around MPI functions - added test to check behavior in case MPI functions are used in other translation units -

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

Alexander_Droste updated this revision to Diff 52059. Alexander_Droste added a comment. - fix typo http://reviews.llvm.org/D12761 Files: lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp lib/Stat

[PATCH] D18595: MPI-Checker test helper

Alexander_Droste created this revision. Alexander_Droste added a reviewer: zaks.anna. Alexander_Droste added subscribers: dcoughlin, xazax.hun, cfe-commits. This file contains definitions that help to test MPI-Checker functionality, on functions not visible to the analyzer, as they are defined in

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

Alexander_Droste marked 3 inline comments as done. Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp:95 @@ +94,3 @@ + if (Optional SP = N->getLocation().getAs()) { +if (const CallExpr *CE = clang::dyn_cast(SP->getStmt())) { + zaks.anna wr

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

Alexander_Droste marked 6 inline comments as done. Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp:39 @@ +38,3 @@ + + if (Range.isValid()) +Report->addRange(Range); `sourceRange` patch -> http://reviews.llvm.org/D18309

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

Alexander_Droste updated this revision to Diff 51179. Alexander_Droste added a comment. - remove `checkMissingWaitsGlobals` to prevent potential false positives http://reviews.llvm.org/D12761 Files: lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/Checkers.td lib/Sta

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste added a comment. I submitted the sourceRange patch here: http://reviews.llvm.org/D18309 If also this patch would get committed as part of the package, there would be no need for an incremental commit procedure. http://reviews.llvm.org/D16044 _

[PATCH] D18309: sourceRange function for MemRegion

Alexander_Droste created this revision. Alexander_Droste added a reviewer: zaks.anna. Alexander_Droste added subscribers: dcoughlin, xazax.hun, cfe-commits. Retrieve source range from memory region. The range retrieval is based on the decl obtained from the memory region. http://reviews.llvm.or

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste added a comment. > I'd be fine if we test this function with the usual regression tests by > observing the output of the MPI checker. We could update that test with more > checks once the function is updated. > With that approach, you'd be committing both patches at the same ti

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

Alexander_Droste added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:136 @@ +135,3 @@ + auto NodeIt = G.eop_begin(); + const auto NodeEndIt = G.eop_end(); + zaks.anna wrote: > The analyzer does not do a good job tracking glo

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste added inline comments. Comment at: test/Analysis/MemRegion.cpp:3 @@ +2,3 @@ + +#include "MPIMock.h" + The problem about these tests is that they introduce a cyclic commit dependency. MPI-Checker depends on `getDescriptiveName`. `getDescriptiveNam

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste updated this revision to Diff 50551. Alexander_Droste added a comment. - create `MemRegion.cpp`, in order to set up test cases for `getDescriptiveName` http://reviews.llvm.org/D16044 Files: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h lib/StaticAnalyzer/Core/

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

Alexander_Droste added inline comments. Comment at: test/Analysis/MPIChecker.cpp:97 @@ +96,3 @@ + +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_DOU

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

Alexander_Droste updated this revision to Diff 50548. Alexander_Droste marked an inline comment as done. Alexander_Droste added a comment. - omit superfluous arguments passed to `generateNonFatalErrorNode` http://reviews.llvm.org/D12761 Files: lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/

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

Alexander_Droste marked 3 inline comments as done. Alexander_Droste added a comment. > I still have to do an overall pass over this checker, but it looks much > better than the first version! :D Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:47 @@ +46,3 @@

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

Alexander_Droste updated this revision to Diff 50526. Alexander_Droste added a comment. - use MPI mock header in integration test file http://reviews.llvm.org/D12761 Files: lib/StaticAnalyzer/Checkers/CMakeLists.txt lib/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/MPI-C

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste added a comment. Thanks for pointing this out. What's actually the best way to test the function? If I test this function with an integration test, I need to rely on a checker which uses the function to output diagnostics. Is it possible to test this with a unit test? My assump

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

Alexander_Droste retitled this revision from "getVariableName() for MemRegion" to "getDescriptiveName() for MemRegion". Alexander_Droste updated the summary for this revision. Alexander_Droste updated this revision to Diff 50114. Alexander_Droste added a comment. Looking at the `printPretty` impl

Re: [PATCH] D16044: getVariableName() for MemRegion

Alexander_Droste added inline comments. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:160 @@ -153,1 +159,3 @@ + /// \returns variable name for memory region + std::string getVariableName() const; }; dcoughlin wrote: > Alexander_Droste

Re: [PATCH] D16044: getVariableName() for MemRegion

Alexander_Droste added a comment. Hi Devin, thanks for taking the time! Yes, this is kind of implicitly tested by the MPI patch but I agree that it is necessary to add tests to MemRegion. Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:160 @@ -153,1 +159

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

Alexander_Droste added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:46 @@ +45,3 @@ +ExplodedNode *ErrorNode = Ctx.generateNonFatalErrorNode(State, &Tag); +BReporter->reportDoubleNonblocking(PreCallEvent, *Req, MR, ExplNode); +Ctx.

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

Alexander_Droste updated this revision to Diff 49028. Alexander_Droste added a comment. - removed commented out line - removed dashes '–––' from comment in `MPIChecker.cpp` testfile - (diff is created with `clang` as pwd) http://reviews.llvm.org/D12761 Files: lib/StaticAnalyzer/Checkers/CMake

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

Alexander_Droste added inline comments. Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp:38 @@ +37,3 @@ + Report->addRange(MPICallEvent.getSourceRange()); + SourceRange Range = RequestRegion->sourceRange(); + // util::sourceRange(RequestRegion); --

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

Alexander_Droste updated this revision to Diff 49023. Alexander_Droste marked 2 inline comments as done. Alexander_Droste added a comment. - fixed checkUnmatchedWaits (added ErrorNode) - non fatal error node for double nonblocking - renamed BugReporter variable to BReporter - description why custo

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

Alexander_Droste marked 36 inline comments as done. Comment at: tools/clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPICheckerPathSensitive.cpp:79 @@ +78,3 @@ + if (!ReqRegions.empty()) { +Ctx.addTransition(State); + } Alexander_Droste wrote: > zaks.anna wr

Re: [PATCH] D16044: getVariableName() for MemRegion

Alexander_Droste updated this revision to Diff 48959. Alexander_Droste added a comment. - removed `reverse` - fixed twine usage I think `VariableName.insert(VariableName.size() - 1, ArrayIndices);` is needed here. varName, indices -> 'varname[...]' If append would be used, the second single qu

Re: [PATCH] D16044: getVariableName() for MemRegion

Alexander_Droste updated this revision to Diff 48953. Alexander_Droste added a comment. - remove string idx variable -> remove string copy assignments - use Twine to reduce temporary string objects, built during concatenation Is `ArrayIndices = llvm::Twine(ArrayIndices + "]" + intValAsString.str(

Re: [PATCH] D16044: getVariableName() for MemRegion

Alexander_Droste updated this revision to Diff 48949. Alexander_Droste added a comment. - Changed `idx = R->getVariableName();` to `idx = ER->getVariableName();` as it better describes what is happening on a semantic level. http://reviews.llvm.org/D16044 Files: include/clang/StaticAnalyzer

  1   2   >