[PATCH] D37187: [Analyzer] Fix Bug 25609 - Assertion UNREACHABLE: 'Unexpected ProgramPoint' with widen-loops=true

Wang Liushuai via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 30 19:13:49 PDT 2017


MTC marked an inline comment as done.
MTC added a comment.

Hi peter,

Thank you very much for your help.

> - I think the current display information is ambiguous. If I did not know the code then I would not understand what this stands for. (That is just my opinon.)  I think Artem's "Contents of <...> are wiped" idea is better but I would go with something like this: "Invalidated previously known information on <...>". (Maybe remove the word 'previously' if its too long.) It still can be weird for the user that why this happened but at least in case of a false positive he/she can see that why this was even considered by the analyzer.

Yes, that's the problem. The information displayed to the user should be clear, but not difficult to understand.

> - Right now there is a "race condition" between your patch and D36690 <https://reviews.llvm.org/D36690>. So in order to avoid "conflicts" I'd ask you to add some variable changing effect to the body of the loop. For example in the last test case the variable 'num' is the one which you use to show the effect of widening. In this case a line like `num++` or `num = i` would ensure that the more precise widening invalidates it as well. Other thing is that handling loops which contains pointer operation or nested loops will be a little bit more conservative, so you these will not be widened like now. (However, test_for_bug_25609()) still should be valid.

I'll update the test file,:D.

> - Please upload the diff with context (git flag: -U99999) since it is really helpful for the review.

About ‘git diff’, that's my mistake, and I'll add this option next time. Thank you very much for pointing it out!

> - +1 inline comment below.




https://reviews.llvm.org/D37187





More information about the cfe-commits mailing list