[PATCH] D82185: [Analyzer][WIP] Handle pointer implemented as iterators in iterator checkers
baloghadamsoftware marked an inline comment as done. baloghadamsoftware added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:256 + SVal SubVal = State->getSVal(UO->getSubExpr(), C.getLocationContext()); + SVal Result = State->getSVal(UO, C.getLocationContext()); + baloghadamsoftware wrote: > baloghadamsoftware wrote: > > This is the problematic point which is not working. I left the comments > > intentionally in the code. > > > > The problem is that in `postStmt` we are //after// the operation. Thus the > > value of the operand (`SubExpr`) is not `i` anymore, but the //former > > value// of `i` (a pointer to a symbolic region initially). Instead, the > > result is `i` in case of prefix operators, but also the former value in > > case of postfix operators. This is correct, of course, because here, after > > the call the value of `i` was changed, thus it is not equal to the > > parameter. However, we need the region of `i` here and/or the new value > > bound to it (e.g. the pointer to an element region which is usually the > > result of a `++` or `--` on a pointer to a symbolic region). How to reach > > that? Of course, in `preStmt` the operand is `i` as it should be. The same > > is true for binary operators `+=` and `-=`. > Of course, I know it is all wrong to increment the //former// value. This is > also not the goal, probably I cannot reuse `handleIncrement()` and the like > here. The question is how to get the region of the variable or even better > the //new// region (e.g. the pointer to element region) bound to it? In the > prefix case I have the variable, but is there a generic way to get the region > bound to it? By generic I mean that I hope that I do not have to branch on > all possible lvalue expressions. Wait! I know the solution! I will try it tomorrow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82185/new/ https://reviews.llvm.org/D82185 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D82185: [Analyzer][WIP] Handle pointer implemented as iterators in iterator checkers
baloghadamsoftware marked an inline comment as done. baloghadamsoftware added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:256 + SVal SubVal = State->getSVal(UO->getSubExpr(), C.getLocationContext()); + SVal Result = State->getSVal(UO, C.getLocationContext()); + baloghadamsoftware wrote: > This is the problematic point which is not working. I left the comments > intentionally in the code. > > The problem is that in `postStmt` we are //after// the operation. Thus the > value of the operand (`SubExpr`) is not `i` anymore, but the //former value// > of `i` (a pointer to a symbolic region initially). Instead, the result is `i` > in case of prefix operators, but also the former value in case of postfix > operators. This is correct, of course, because here, after the call the value > of `i` was changed, thus it is not equal to the parameter. However, we need > the region of `i` here and/or the new value bound to it (e.g. the pointer to > an element region which is usually the result of a `++` or `--` on a pointer > to a symbolic region). How to reach that? Of course, in `preStmt` the operand > is `i` as it should be. The same is true for binary operators `+=` and `-=`. Of course, I know it is all wrong to increment the //former// value. This is also not the goal, probably I cannot reuse `handleIncrement()` and the like here. The question is how to get the region of the variable or even better the //new// region (e.g. the pointer to element region) bound to it? In the prefix case I have the variable, but is there a generic way to get the region bound to it? By generic I mean that I hope that I do not have to branch on all possible lvalue expressions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82185/new/ https://reviews.llvm.org/D82185 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D82185: [Analyzer][WIP] Handle pointer implemented as iterators in iterator checkers
baloghadamsoftware marked an inline comment as done. baloghadamsoftware added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:256 + SVal SubVal = State->getSVal(UO->getSubExpr(), C.getLocationContext()); + SVal Result = State->getSVal(UO, C.getLocationContext()); + This is the problematic point which is not working. I left the comments intentionally in the code. The problem is that in `postStmt` we are //after// the operation. Thus the value of the operand (`SubExpr`) is not `i` anymore, but the //former value// of `i` (a pointer to a symbolic region initially). Instead, the result is `i` in case of prefix operators, but also the former value in case of postfix operators. This is correct, of course, because here, after the call the value of `i` was changed, thus it is not equal to the parameter. However, we need the region of `i` here and/or the new value bound to it (e.g. the pointer to an element region which is usually the result of a `++` or `--` on a pointer to a symbolic region). How to reach that? Of course, in `preStmt` the operand is `i` as it should be. The same is true for binary operators `+=` and `-=`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82185/new/ https://reviews.llvm.org/D82185 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D82185: [Analyzer][WIP] Handle pointer implemented as iterators in iterator checkers
baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, Szelethus, martong, gamesh411.
baloghadamsoftware added a project: clang.
Herald added subscribers: ASDenysPetrov, steakhal, Charusso, dkrupp,
donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, whisperity.
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:256
+ SVal SubVal = State->getSVal(UO->getSubExpr(), C.getLocationContext());
+ SVal Result = State->getSVal(UO, C.getLocationContext());
+
This is the problematic point which is not working. I left the comments
intentionally in the code.
The problem is that in `postStmt` we are //after// the operation. Thus the
value of the operand (`SubExpr`) is not `i` anymore, but the //former value//
of `i` (a pointer to a symbolic region initially). Instead, the result is `i`
in case of prefix operators, but also the former value in case of postfix
operators. This is correct, of course, because here, after the call the value
of `i` was changed, thus it is not equal to the parameter. However, we need the
region of `i` here and/or the new value bound to it (e.g. the pointer to an
element region which is usually the result of a `++` or `--` on a pointer to a
symbolic region). How to reach that? Of course, in `preStmt` the operand is `i`
as it should be. The same is true for binary operators `+=` and `-=`.
Iterators are an abstraction of pointers and in some data structures iterators
may be implemented by pointers. This patch adds support for iterators
implemented as pointers in all the iterator checkers (including iterator
modeling).
This patch is work in progress, modeling of lvalue operations do not work yet.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D82185
Files:
clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp
clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
clang/lib/StaticAnalyzer/Checkers/Iterator.h
clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp
clang/test/Analysis/invalidated-iterator.cpp
clang/test/Analysis/iterator-modeling.cpp
clang/test/Analysis/iterator-range.cpp
clang/test/Analysis/mismatched-iterator.cpp
Index: clang/test/Analysis/mismatched-iterator.cpp
===
--- clang/test/Analysis/mismatched-iterator.cpp
+++ clang/test/Analysis/mismatched-iterator.cpp
@@ -118,3 +118,15 @@
if (V1.cbegin() == V2.cbegin()) {} //no-warning
}
+
+template
+struct cont_with_ptr_iterator {
+ T *begin() const;
+ T *end() const;
+};
+
+void comparison_ptr_iterator(cont_with_ptr_iterator ,
+ cont_with_ptr_iterator ) {
+ if (C1.begin() != C2.end()) {} // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
+
Index: clang/test/Analysis/iterator-range.cpp
===
--- clang/test/Analysis/iterator-range.cpp
+++ clang/test/Analysis/iterator-range.cpp
@@ -854,3 +854,84 @@
*i; // expected-warning{{Past-the-end iterator dereferenced}}
// expected-note@-1{{Past-the-end iterator dereferenced}}
}
+
+template
+struct cont_with_ptr_iterator {
+ T* begin() const;
+ T* end() const;
+};
+
+void deref_end_ptr_iterator(const cont_with_ptr_iterator ) {
+ auto i = c.end();
+ (void) *i; // expected-warning{{Past-the-end iterator dereferenced}}
+ // expected-note@-1{{Past-the-end iterator dereferenced}}
+}
+
+void array_deref_end_ptr_iterator(const cont_with_ptr_iterator ) {
+ auto i = c.end();
+ (void) i[0]; // expected-warning{{Past-the-end iterator dereferenced}}
+ // expected-note@-1{{Past-the-end iterator dereferenced}}
+}
+
+void arrow_deref_end_ptr_iterator(const cont_with_ptr_iterator ) {
+ auto i = c.end();
+ (void) i->n; // expected-warning{{Past-the-end iterator dereferenced}}
+ // expected-note@-1{{Past-the-end iterator dereferenced}}
+}
+
+void arrow_star_deref_end_ptr_iterator(const cont_with_ptr_iterator ,
+ int S::*p) {
+ auto i = c.end();
+ (void)(i->*p); // expected-warning{{Past-the-end iterator dereferenced}}
+ // expected-note@-1{{Past-the-end iterator dereferenced}}
+}
+
+void prefix_incr_end_ptr_iterator(const cont_with_ptr_iterator ) {
+ auto i = c.end();
+ ++i; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+ // expected-note@-1{{Iterator incremented behind the past-the-end iterator}}
+}
+
+void postfix_incr_end_ptr_iterator(const cont_with_ptr_iterator ) {
+ auto i = c.end();
+ i++; // expected-warning{{Iterator incremented behind the past-the-end iterator}}
+ //
