xazax.hun closed this revision.
xazax.hun added a comment.
This was committad as the part of http://reviews.llvm.org/rL272529
http://reviews.llvm.org/D16044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailma
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
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/
xazax.hun added a comment.
In http://reviews.llvm.org/D16044#401130, @Alexander_Droste wrote:
> 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 indic
xazax.hun added a comment.
I will look into it tomorrow.
http://reviews.llvm.org/D16044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
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
__
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
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
xazax.hun added a comment.
Sure, the test case of http://reviews.llvm.org/D15227 is faiing because of this.
In test/Analysis/valist-unterminated.c, line 10
Initialized va_list 'va[0]' is leaked
is emitted instead of
Initialized va_list 'va' is leaked
http://reviews.llvm.org/D16044
__
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
xazax.hun 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?
http://reviews.llvm.org/D16044
__
xazax.hun added a comment.
In http://reviews.llvm.org/D16044#398463, @zaks.anna wrote:
> Can other checkers use this function? I am Ok with this being committed with
> limited coverage.
Once I rebase http://reviews.llvm.org/D15227, it will provide us with the
limited coverage. In case that pa
zaks.anna added a comment.
Gabor,
Can other checkers use this function? I am Ok with this being committed with
limited coverage.
http://reviews.llvm.org/D16044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/m
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
xazax.hun 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?
http://reviews.llvm.org/D16044
___
cfe-commits maili
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.
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
zaks.anna 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.
http://reviews.llvm.org/D16044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
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
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
zaks.anna 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);
xazax.hun wrote:
> Alexander_Droste wr
xazax.hun 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:
> I wasn't abl
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
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
_
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
zaks.anna 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 time.
http://re
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
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/
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
zaks.anna added a comment.
I thunk the closest we have is region-store* tests, which is not quite the
same. Feel free to add a new test.
http://reviews.llvm.org/D16044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cg
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
31 matches
Mail list logo
