baloghadamsoftware added inline comments.

================
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:605
+  if (Pos && !Pos->isValid()) {
+    // If I do not put a tag here, some invalidation tests will fail
+    static CheckerProgramPointTag Tag("InvalidatedIteratorChecker",
----------------
NoQ wrote:
> This needs investigation, because it may be nasty.
> 
> `generateNonFatalErrorNode()` returns null when the exact same non-fatal 
> error node, also produced by the iterator checker with the exact same program 
> state and exact same program point and exact same tag on the program point 
> already exists. As far as i understand, the only difference your tag makes is 
> that the tag is now different, so it is not merged with the existing node. 
> However, it is worth it to try to find out why the node gets merged at all.
> 
> This may be caused by an accidental state split. For example, if you are 
> calling `generateNonFatalErrorNode()` twice in the same checker callback 
> without chaining them together (passing node returned by one as an argument 
> to another), this in fact splits states. I'm not immediately seeing such 
> places in the code - you seem to be aware of this problem and avoiding it 
> well. But still, looking at the topology of the exploded graph in the failing 
> test should help finding out what is going on.
I made some more investigation this time. Unfortunately the case is not what 
you suggest. Only one non-fatal error node is produced. I tested it with a 
common tag (a global static so the tag is exactly the same at every 
`generateNonFatalErrorNode()`, but the tests still pass. I printed out the 
exploded graph and I found that there are indeed two nodes with the same state 
ID. The tag is the default tag automatically generated from the name of the 
checker. The first state is created in function `checkPreStatement()` for 
`CXXOperatorCallExpr` where I copy the state of the iterator from the formal to 
the actual `this` parameter. All the test fails happen at the dereference 
operator call (`*`) of another operator call (`++` or `--`). After this copy, 
when I call `generateNonFatalErrorNode()` I get `nullptr` because at some point 
under the hood (I debugged it when I created it originally) the error node is 
considered as "not new". If I use a custom tag here, the state ID remains, not 
the node ID changes.


https://reviews.llvm.org/D32747



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to