[PATCH] D53701: [Analyzer] Record and process comparison of symbols instead of iterator positions in interator checkers

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 30 14:04:15 PST 2018


NoQ added a comment.
Herald added a subscriber: gamesh411.

In D53701#1288682 <https://reviews.llvm.org/D53701#1288682>, @baloghadamsoftware wrote:

> I am almost sure it is implossible. It is not only about iterator-adapters in the classical sense but also any iterator that internally deals with other iterators.
>
> For example I created an iterator in the past that iterates over the elements of a list of lists. This iterator contained two iterators, one for the outer and one for the inner list. In this case determining whether two iterators are the same is non-trivial: for all in-range iterators both the outer and the inner iterators must be the same. However if the outer iterator is the past-end iterator of the outer list then the inner iterator is irrelevant. Thus the comparison operator of such iterator must check first whether any of the outer iterators is the past-end iterator and only compare the inner iterator none of them is. If both outer iterators are the past-end iterator then they are equal regardless of the inner iterators.
>
> Another example could be an iterator that somehow merges two lists internally using two different iterators. In this case only one of the inner iterators is relevant when comparing two merging iterators.
>
> So by dropping the inlining we lose some intrinsic knowledge of the analyzed code which leads the checker to wrong assumptions.


The nested iterator thing looks easy to detect heuristically. Just have a look if any of the fields within the object are of iterator type.

I think it's worth a try to do a total evalCall at first, and then disable evalCall (together with the attempt to model the iterator position) in an incrementally growing blacklist of cases (1. iterator adaptors, 2. ....) as we encounter problems. This essentially becomes part of the logic that decides whether an object is an iterator. Eg., if it's more like an adaptor than an actual iterator, let's treat it as if it's not an iterator, but inline instead, and hope that we model the underlying iterators correctly via evalCall.

This does look hacky, but it does look less hacky than trying to align two models of the iterator position. Or is it actually necessary to maintain the two models of the iterator position in order to avoid these false positives? If so, could you give an example?

> When I first started working on Clang Static Analyzer Anna told me the `-analyzer-eagerly-assume` should be the default.

Which is why i suggest a similar behavior here.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53701/new/

https://reviews.llvm.org/D53701





More information about the cfe-commits mailing list