Commit Graph

5 Commits

Author SHA1 Message Date
Sean Eveson 00e780e1cc [Analyzer] Fix comments and formatting. NFC.
llvm-svn: 252599
2015-11-10 11:48:55 +00:00
Eugene Zelenko d4304d2f9c Fix some Clang-tidy modernize warnings, other minor fixes.
Differential revision: http://reviews.llvm.org/D14311

llvm-svn: 252081
2015-11-04 21:37:17 +00:00
Sean Eveson 70eece21c2 Reapply r251621 "[Analyzer] Widening loops which do not exit"
It was not the cause of the build bot failure.

llvm-svn: 251702
2015-10-30 15:23:57 +00:00
Sean Eveson 4c7b3bf6ba Revert r251621 "[Analyzer] Widening loops which do not exit" (bot failure)
Seems to be causing clang-cmake-mips build bot to fail (timeout)

http://lab.llvm.org:8011/builders/clang-cmake-mips/builds/10299

llvm-svn: 251697
2015-10-30 11:13:07 +00:00
Sean Eveson 83390e45b3 [Analyzer] Widening loops which do not exit
Summary:
Dear All,

We have been looking at the following problem, where any code after the constant bound loop is not analyzed because of the limit on how many times the same block is visited, as described in bugzillas #7638 and #23438. This problem is of interest to us because we have identified significant bugs that the checkers are not locating. We have been discussing a solution involving ranges as a longer term project, but I would like to propose a patch to improve the current implementation.

Example issue:
```
for (int i = 0; i < 1000; ++i) {...something...}
int *p = 0;
*p = 0xDEADBEEF;
```

The proposal is to go through the first and last iterations of the loop. The patch creates an exploded node for the approximate last iteration of constant bound loops, before the max loop limit / block visit limit is reached. It does this by identifying the variable in the loop condition and finding the value which is “one away” from the loop being false. For example, if the condition is (x < 10), then an exploded node is created where the value of x is 9. Evaluating the loop body with x = 9 will then result in the analysis continuing after the loop, providing x is incremented.

The patch passes all the tests, with some modifications to coverage.c, in order to make the ‘function_which_gives_up’ continue to give up, since the changes allowed the analysis to progress past the loop.

This patch does introduce possible false positives, as a result of not knowing the state of variables which might be modified in the loop. I believe that, as a user, I would rather have false positives after loops than do no analysis at all. I understand this may not be the common opinion and am interested in hearing your views. There are also issues regarding break statements, which are not considered. A more advanced implementation of this approach might be able to consider other conditions in the loop, which would allow paths leading to breaks to be analyzed.

Lastly, I have performed a study on large code bases and I think there is little benefit in having “max-loop” default to 4 with the patch. For variable bound loops this tends to result in duplicated analysis after the loop, and it makes little difference to any constant bound loop which will do more than a few iterations. It might be beneficial to lower the default to 2, especially for the shallow analysis setting.

Please let me know your opinions on this approach to processing constant bound loops and the patch itself.

Regards,

Sean Eveson
SN Systems - Sony Computer Entertainment Group

Reviewers: jordan_rose, krememek, xazax.hun, zaks.anna, dcoughlin

Subscribers: krememek, xazax.hun, cfe-commits

Differential Revision: http://reviews.llvm.org/D12358

llvm-svn: 251621
2015-10-29 10:04:41 +00:00