[PATCH] D81718: [Analyzer][NFC] Add methods `getReturnObject()` and `getArgObject()` to `CallEvent`

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 16 02:43:37 PDT 2020


NoQ added a comment.

> Let me explain! An iterator may be implemented by multiple ways. It can be a simple pointer...

Let's cover this with tests then, for once.

Hint: they don't work, for the exact reason i described above, and your last update to this patch is insufficient to fix that.

`// system-header-simulator-cxx.h:`

  ...
  222   class vector {
  223     T *_start;
  224     T *_finish;
  225     T *_end_of_storage;
  226
  227   public:
  228     typedef T value_type;
  229     typedef size_t size_type;
  230     typedef T* iterator;              // <===
  231     typedef const T * const_iterator; // <===
  ...

`// iterator-range.cpp:`

  ...
  11 void deref_end(const std::vector<int> &V) {
  12   auto i = V.end();
  13   *i; // expected-warning{{Past-the-end iterator dereferenced}}
  14       // expected-note at -1{{Past-the-end iterator dereferenced}}
  15 }
  ...

And indeed it doesn't emit the warning.

F12170968: Screen Shot 2020-06-16 at 1.33.40 AM.png <https://reviews.llvm.org/F12170968>

So it still thinks that the iterator is an object, whereas it should be tracking the iterator as symbol instead. As expected, because your code fails to discriminate between these two cases. You can't reorder checks in `{set,get}IteratorPosition` to fix that (first check for symbol, then check for region) because that'd break object iterators that reside in symbolic regions; you need to repeat the check on the AST instead. Which makes the reuse of that check proposed in this patch worthless.


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

https://reviews.llvm.org/D81718





More information about the cfe-commits mailing list