[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers
This revision was automatically updated to reflect the committed changes.
Closed by commit rC341791: [Analyzer] Iterator Checker - Part 5: Move
Assignment of Containers (authored by baloghadamsoftware, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D32859?vs=163725=164632#toc
Repository:
rC Clang
https://reviews.llvm.org/D32859
Files:
lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -114,6 +114,10 @@
return IteratorPosition(Cont, Valid, NewOf);
}
+ IteratorPosition reAssign(const MemRegion *NewCont) const {
+return IteratorPosition(NewCont, Valid, Offset);
+ }
+
bool operator==(const IteratorPosition ) const {
return Cont == X.Cont && Valid == X.Valid && Offset == X.Offset;
}
@@ -218,7 +222,9 @@
const SVal ) const;
void assignToContainer(CheckerContext , const Expr *CE, const SVal ,
const MemRegion *Cont) const;
- void handleAssign(CheckerContext , const SVal ) const;
+ void handleAssign(CheckerContext , const SVal ,
+const Expr *CE = nullptr,
+const SVal = UndefinedVal()) const;
void verifyRandomIncrOrDecr(CheckerContext , OverloadedOperatorKind Op,
const SVal , const SVal ,
const SVal ) const;
@@ -315,6 +321,17 @@
bool Equal);
ProgramStateRef invalidateAllIteratorPositions(ProgramStateRef State,
const MemRegion *Cont);
+ProgramStateRef reassignAllIteratorPositions(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont);
+ProgramStateRef reassignAllIteratorPositionsUnless(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont,
+ SymbolRef Offset,
+ BinaryOperator::Opcode Opc);
+ProgramStateRef rebaseSymbolInIteratorPositionsIf(
+ProgramStateRef State, SValBuilder , SymbolRef OldSym,
+SymbolRef NewSym, SymbolRef CondSym, BinaryOperator::Opcode Opc);
const ContainerData *getContainerData(ProgramStateRef State,
const MemRegion *Cont);
ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
@@ -454,7 +471,12 @@
const auto Op = Func->getOverloadedOperator();
if (isAssignmentOperator(Op)) {
const auto *InstCall = dyn_cast();
- handleAssign(C, InstCall->getCXXThisVal());
+ if (Func->getParamDecl(0)->getType()->isRValueReferenceType()) {
+handleAssign(C, InstCall->getCXXThisVal(), Call.getOriginExpr(),
+ Call.getArgSVal(0));
+ } else {
+handleAssign(C, InstCall->getCXXThisVal());
+ }
} else if (isSimpleComparisonOperator(Op)) {
if (const auto *InstCall = dyn_cast()) {
handleComparison(C, Call.getReturnValue(), InstCall->getCXXThisVal(),
@@ -503,9 +525,6 @@
return;
auto State = C.getState();
-// Already bound to container?
-if (getIteratorPosition(State, Call.getReturnValue()))
- return;
if (const auto *InstCall = dyn_cast()) {
if (isBeginCall(Func)) {
@@ -520,6 +539,10 @@
}
}
+// Already bound to container?
+if (getIteratorPosition(State, Call.getReturnValue()))
+ return;
+
// Copy-like and move constructors
if (isa() && Call.getNumArgs() == 1) {
if (const auto *Pos = getIteratorPosition(State, Call.getArgSVal(0))) {
@@ -981,7 +1004,8 @@
C.addTransition(State);
}
-void IteratorChecker::handleAssign(CheckerContext , const SVal ) const {
+void IteratorChecker::handleAssign(CheckerContext , const SVal ,
+ const Expr *CE, const SVal ) const {
const auto *ContReg = Cont.getAsRegion();
if (!ContReg)
return;
@@ -998,6 +1022,65 @@
State = invalidateAllIteratorPositions(State, ContReg);
}
+ // In case of move, iterators of the old container (except the past-end
+ // iterators) remain valid but refer to the new container
+ if (!OldCont.isUndef()) {
+const auto *OldContReg = OldCont.getAsRegion();
+if (OldContReg) {
+ while (const auto *CBOR = OldContReg->getAs()) {
+OldContReg = CBOR->getSuperRegion();
+ }
+ const auto OldCData = getContainerData(State, OldContReg);
+ if (OldCData) {
+if (const auto OldEndSym = OldCData->getEnd()) {
+ // If we already assigned an "end" symbol to the old conainer, then
+ //
[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers
baloghadamsoftware updated this revision to Diff 163725.
baloghadamsoftware added a comment.
) added.
https://reviews.llvm.org/D32859
Files:
lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
test/Analysis/mismatched-iterator.cpp
Index: test/Analysis/mismatched-iterator.cpp
===
--- test/Analysis/mismatched-iterator.cpp
+++ test/Analysis/mismatched-iterator.cpp
@@ -15,11 +15,48 @@
std::copy(v1.cbegin(), v1.cend(), v2.begin()); // no-warning
}
+void good_move_find1(std::vector , std::vector , int n) {
+ auto i0 = v2.cbegin();
+ v1 = std::move(v2);
+ std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find2(std::vector , std::vector , int n) {
+ auto i0 = --v2.cend();
+ v1 = std::move(v2);
+ std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find3(std::vector , std::vector , int n) {
+ auto i0 = v2.cend();
+ v1 = std::move(v2);
+ v2.push_back(n);
+ std::find(v2.cbegin(), i0, n); // no-warning
+}
+
void bad_find(std::vector , std::vector , int n) {
std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
}
void bad_find_first_of(std::vector , std::vector ) {
std::find_first_of(v1.cbegin(), v2.cend(), v2.cbegin(), v2.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v1.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
}
+
+void bad_move_find1(std::vector , std::vector , int n) {
+ auto i0 = v2.cbegin();
+ v1 = std::move(v2);
+ std::find(i0, v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
+
+void bad_move_find2(std::vector , std::vector , int n) {
+ auto i0 = --v2.cend();
+ v1 = std::move(v2);
+ std::find(i0, v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
+
+void bad_move_find3(std::vector , std::vector , int n) {
+ auto i0 = v2.cend();
+ v1 = std::move(v2);
+ std::find(v1.cbegin(), i0, n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -114,6 +114,10 @@
return IteratorPosition(Cont, Valid, NewOf);
}
+ IteratorPosition reAssign(const MemRegion *NewCont) const {
+return IteratorPosition(NewCont, Valid, Offset);
+ }
+
bool operator==(const IteratorPosition ) const {
return Cont == X.Cont && Valid == X.Valid && Offset == X.Offset;
}
@@ -218,7 +222,9 @@
const SVal ) const;
void assignToContainer(CheckerContext , const Expr *CE, const SVal ,
const MemRegion *Cont) const;
- void handleAssign(CheckerContext , const SVal ) const;
+ void handleAssign(CheckerContext , const SVal ,
+const Expr *CE = nullptr,
+const SVal = UndefinedVal()) const;
void verifyRandomIncrOrDecr(CheckerContext , OverloadedOperatorKind Op,
const SVal , const SVal ,
const SVal ) const;
@@ -315,6 +321,17 @@
bool Equal);
ProgramStateRef invalidateAllIteratorPositions(ProgramStateRef State,
const MemRegion *Cont);
+ProgramStateRef reassignAllIteratorPositions(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont);
+ProgramStateRef reassignAllIteratorPositionsUnless(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont,
+ SymbolRef Offset,
+ BinaryOperator::Opcode Opc);
+ProgramStateRef rebaseSymbolInIteratorPositionsIf(
+ProgramStateRef State, SValBuilder , SymbolRef OldSym,
+SymbolRef NewSym, SymbolRef CondSym, BinaryOperator::Opcode Opc);
const ContainerData *getContainerData(ProgramStateRef State,
const MemRegion *Cont);
ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
@@ -454,7 +471,12 @@
const auto Op = Func->getOverloadedOperator();
if (isAssignmentOperator(Op)) {
const auto *InstCall = dyn_cast();
- handleAssign(C, InstCall->getCXXThisVal());
+ if (Func->getParamDecl(0)->getType()->isRValueReferenceType()) {
+handleAssign(C,
[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers
baloghadamsoftware updated this revision to Diff 163724.
baloghadamsoftware added a comment.
Comments added, functions renamed.
https://reviews.llvm.org/D32859
Files:
lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
test/Analysis/mismatched-iterator.cpp
Index: test/Analysis/mismatched-iterator.cpp
===
--- test/Analysis/mismatched-iterator.cpp
+++ test/Analysis/mismatched-iterator.cpp
@@ -15,11 +15,48 @@
std::copy(v1.cbegin(), v1.cend(), v2.begin()); // no-warning
}
+void good_move_find1(std::vector , std::vector , int n) {
+ auto i0 = v2.cbegin();
+ v1 = std::move(v2);
+ std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find2(std::vector , std::vector , int n) {
+ auto i0 = --v2.cend();
+ v1 = std::move(v2);
+ std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find3(std::vector , std::vector , int n) {
+ auto i0 = v2.cend();
+ v1 = std::move(v2);
+ v2.push_back(n);
+ std::find(v2.cbegin(), i0, n); // no-warning
+}
+
void bad_find(std::vector , std::vector , int n) {
std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
}
void bad_find_first_of(std::vector , std::vector ) {
std::find_first_of(v1.cbegin(), v2.cend(), v2.cbegin(), v2.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v1.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
}
+
+void bad_move_find1(std::vector , std::vector , int n) {
+ auto i0 = v2.cbegin();
+ v1 = std::move(v2);
+ std::find(i0, v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
+
+void bad_move_find2(std::vector , std::vector , int n) {
+ auto i0 = --v2.cend();
+ v1 = std::move(v2);
+ std::find(i0, v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
+
+void bad_move_find3(std::vector , std::vector , int n) {
+ auto i0 = v2.cend();
+ v1 = std::move(v2);
+ std::find(v1.cbegin(), i0, n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -114,6 +114,10 @@
return IteratorPosition(Cont, Valid, NewOf);
}
+ IteratorPosition reAssign(const MemRegion *NewCont) const {
+return IteratorPosition(NewCont, Valid, Offset);
+ }
+
bool operator==(const IteratorPosition ) const {
return Cont == X.Cont && Valid == X.Valid && Offset == X.Offset;
}
@@ -218,7 +222,9 @@
const SVal ) const;
void assignToContainer(CheckerContext , const Expr *CE, const SVal ,
const MemRegion *Cont) const;
- void handleAssign(CheckerContext , const SVal ) const;
+ void handleAssign(CheckerContext , const SVal ,
+const Expr *CE = nullptr,
+const SVal = UndefinedVal()) const;
void verifyRandomIncrOrDecr(CheckerContext , OverloadedOperatorKind Op,
const SVal , const SVal ,
const SVal ) const;
@@ -315,6 +321,17 @@
bool Equal);
ProgramStateRef invalidateAllIteratorPositions(ProgramStateRef State,
const MemRegion *Cont);
+ProgramStateRef reassignAllIteratorPositions(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont);
+ProgramStateRef reassignAllIteratorPositionsUnless(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont,
+ SymbolRef Offset,
+ BinaryOperator::Opcode Opc);
+ProgramStateRef rebaseSymbolInIteratorPositionsIf(
+ProgramStateRef State, SValBuilder , SymbolRef OldSym,
+SymbolRef NewSym, SymbolRef CondSym, BinaryOperator::Opcode Opc);
const ContainerData *getContainerData(ProgramStateRef State,
const MemRegion *Cont);
ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
@@ -454,7 +471,12 @@
const auto Op = Func->getOverloadedOperator();
if (isAssignmentOperator(Op)) {
const auto *InstCall = dyn_cast();
- handleAssign(C, InstCall->getCXXThisVal());
+ if (Func->getParamDecl(0)->getType()->isRValueReferenceType()) {
+
[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Herald added a subscriber: Szelethus. I think this patch is in good shape. In https://reviews.llvm.org/D32859#1187551, @baloghadamsoftware wrote: > I do not see which lines exactly you commented This was about the `replaceSymbol()` sub. You can traverse history using the History tab, or if there's still a greyed out comment you can push the |<< button on it to jump to the respective historical diff. In any case, i think in current form `replaceSymbol()` is much easier to understand, but still deserves a short comment, maybe a small example, maybe a better name (the word "rebase" comes to mind). https://reviews.llvm.org/D32859 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers
baloghadamsoftware added a comment. In https://reviews.llvm.org/D32859##inline-360206, @NoQ wrote: > I do not immediately understand what is this useful for. At least tests don't > look like they make use of these offset manipulations(?) > > Without full understanding, i wonder: when we overwrite one container with > another, why don't we just overwrite all symbols associated with it, instead > of creating a mixture of old and new symbols? > > Or maybe this is an accidental part of another patch, that has something to > do with resizes? I do not see which lines exactly you commented but I suppose it is about not reassigning all iterator positions to the new container upon moving. According to [[ C++ Reference | en.cppreference.com/w/cpp/container/vector/operator%3D ]] the past-end iterators are not moved to the new container. https://reviews.llvm.org/D32859 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers
whisperity added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1061 + // first reassign all iterator positions to the new container which + // are not past the container (thus not greater or equal to the + // current "end" symbol. `(` is not closed https://reviews.llvm.org/D32859 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers
baloghadamsoftware updated this revision to Diff 153482.
baloghadamsoftware added a comment.
Herald added a subscriber: mikhail.ramalho.
Rebased to https://reviews.llvm.org/rL335835.
https://reviews.llvm.org/D32859
Files:
lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
test/Analysis/mismatched-iterator.cpp
Index: test/Analysis/mismatched-iterator.cpp
===
--- test/Analysis/mismatched-iterator.cpp
+++ test/Analysis/mismatched-iterator.cpp
@@ -15,11 +15,48 @@
std::copy(v1.cbegin(), v1.cend(), v2.begin()); // no-warning
}
+void good_move_find1(std::vector , std::vector , int n) {
+ auto i0 = v2.cbegin();
+ v1 = std::move(v2);
+ std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find2(std::vector , std::vector , int n) {
+ auto i0 = --v2.cend();
+ v1 = std::move(v2);
+ std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find3(std::vector , std::vector , int n) {
+ auto i0 = v2.cend();
+ v1 = std::move(v2);
+ v2.push_back(n);
+ std::find(v2.cbegin(), i0, n); // no-warning
+}
+
void bad_find(std::vector , std::vector , int n) {
std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterator access mismatched}}
}
void bad_find_first_of(std::vector , std::vector ) {
std::find_first_of(v1.cbegin(), v2.cend(), v2.cbegin(), v2.cend()); // expected-warning{{Iterator access mismatched}}
std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v1.cend()); // expected-warning{{Iterator access mismatched}}
}
+
+void bad_move_find1(std::vector , std::vector , int n) {
+ auto i0 = v2.cbegin();
+ v1 = std::move(v2);
+ std::find(i0, v2.cend(), n); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_move_find2(std::vector , std::vector , int n) {
+ auto i0 = --v2.cend();
+ v1 = std::move(v2);
+ std::find(i0, v2.cend(), n); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_move_find3(std::vector , std::vector , int n) {
+ auto i0 = v2.cend();
+ v1 = std::move(v2);
+ std::find(v1.cbegin(), i0, n); // expected-warning{{Iterator access mismatched}}
+}
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -114,6 +114,10 @@
return IteratorPosition(Cont, Valid, NewOf);
}
+ IteratorPosition reAssign(const MemRegion *NewCont) const {
+return IteratorPosition(NewCont, Valid, Offset);
+ }
+
bool operator==(const IteratorPosition ) const {
return Cont == X.Cont && Valid == X.Valid && Offset == X.Offset;
}
@@ -219,7 +223,9 @@
const SVal ) const;
void assignToContainer(CheckerContext , const Expr *CE, const SVal ,
const MemRegion *Cont) const;
- void handleAssign(CheckerContext , const SVal ) const;
+ void handleAssign(CheckerContext , const SVal ,
+const Expr *CE = nullptr,
+const SVal = UndefinedVal()) const;
void verifyRandomIncrOrDecr(CheckerContext , OverloadedOperatorKind Op,
const SVal , const SVal ,
const SVal ) const;
@@ -317,6 +323,17 @@
bool Equal);
ProgramStateRef invalidateAllIteratorPositions(ProgramStateRef State,
const MemRegion *Cont);
+ProgramStateRef reassignAllIteratorPositions(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont);
+ProgramStateRef reassignAllIteratorPositionsUnless(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont,
+ SymbolRef Offset,
+ BinaryOperator::Opcode Opc);
+ProgramStateRef replaceSymbolInIteratorPositionsIf(
+ProgramStateRef State, SValBuilder , SymbolRef OldSym,
+SymbolRef NewSym, SymbolRef CondSym, BinaryOperator::Opcode Opc);
const ContainerData *getContainerData(ProgramStateRef State,
const MemRegion *Cont);
ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
@@ -453,7 +470,12 @@
const auto Op = Func->getOverloadedOperator();
if (isAssignmentOperator(Op)) {
const auto *InstCall = dyn_cast();
- handleAssign(C, InstCall->getCXXThisVal());
+ if (Func->getParamDecl(0)->getType()->isRValueReferenceType()) {
+handleAssign(C, InstCall->getCXXThisVal(), Call.getOriginExpr(),
+ Call.getArgSVal(0));
+ } else {
+handleAssign(C, InstCall->getCXXThisVal());
+ }
} else if (isSimpleComparisonOperator(Op)) {
if
[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers
baloghadamsoftware updated this revision to Diff 144106.
baloghadamsoftware added a comment.
Herald added a reviewer: george.karpenkov.
One test failed after rebased to the current master branch. Depending on the
internal implementation of the iterator and the move operation of the container
it could happen that after moving a container the begin() or end() of the
target of the move returns an already existing iterator position to the source
of the move which is wrong. Therefore we must first check for begin() and end()
and only then check for the existence of the iterator position of the return
value of a function returning an iterator.
https://reviews.llvm.org/D32859
Files:
lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
test/Analysis/mismatched-iterator.cpp
Index: test/Analysis/mismatched-iterator.cpp
===
--- test/Analysis/mismatched-iterator.cpp
+++ test/Analysis/mismatched-iterator.cpp
@@ -15,11 +15,48 @@
std::copy(v1.cbegin(), v1.cend(), v2.begin()); // no-warning
}
+void good_move_find1(std::vector , std::vector , int n) {
+ auto i0 = v2.cbegin();
+ v1 = std::move(v2);
+ std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find2(std::vector , std::vector , int n) {
+ auto i0 = --v2.cend();
+ v1 = std::move(v2);
+ std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find3(std::vector , std::vector , int n) {
+ auto i0 = v2.cend();
+ v1 = std::move(v2);
+ v2.push_back(n);
+ std::find(v2.cbegin(), i0, n); // no-warning
+}
+
void bad_find(std::vector , std::vector , int n) {
std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterator access mismatched}}
}
void bad_find_first_of(std::vector , std::vector ) {
std::find_first_of(v1.cbegin(), v2.cend(), v2.cbegin(), v2.cend()); // expected-warning{{Iterator access mismatched}}
std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v1.cend()); // expected-warning{{Iterator access mismatched}}
}
+
+void bad_move_find1(std::vector , std::vector , int n) {
+ auto i0 = v2.cbegin();
+ v1 = std::move(v2);
+ std::find(i0, v2.cend(), n); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_move_find2(std::vector , std::vector , int n) {
+ auto i0 = --v2.cend();
+ v1 = std::move(v2);
+ std::find(i0, v2.cend(), n); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_move_find3(std::vector , std::vector , int n) {
+ auto i0 = v2.cend();
+ v1 = std::move(v2);
+ std::find(v1.cbegin(), i0, n); // expected-warning{{Iterator access mismatched}}
+}
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -114,6 +114,10 @@
return IteratorPosition(Cont, Valid, NewOf);
}
+ IteratorPosition reAssign(const MemRegion *NewCont) const {
+return IteratorPosition(NewCont, Valid, Offset);
+ }
+
bool operator==(const IteratorPosition ) const {
return Cont == X.Cont && Valid == X.Valid && Offset == X.Offset;
}
@@ -219,7 +223,9 @@
const SVal ) const;
void assignToContainer(CheckerContext , const Expr *CE, const SVal ,
const MemRegion *Cont) const;
- void handleAssign(CheckerContext , const SVal ) const;
+ void handleAssign(CheckerContext , const SVal ,
+const Expr *CE = nullptr,
+const SVal = UndefinedVal()) const;
void verifyRandomIncrOrDecr(CheckerContext , OverloadedOperatorKind Op,
const SVal , const SVal ,
const SVal ) const;
@@ -317,6 +323,17 @@
bool Equal);
ProgramStateRef invalidateAllIteratorPositions(ProgramStateRef State,
const MemRegion *Cont);
+ProgramStateRef reassignAllIteratorPositions(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont);
+ProgramStateRef reassignAllIteratorPositionsUnless(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont,
+ SymbolRef Offset,
+ BinaryOperator::Opcode Opc);
+ProgramStateRef replaceSymbolInIteratorPositionsIf(
+ProgramStateRef State, SValBuilder , SymbolRef OldSym,
+SymbolRef NewSym, SymbolRef CondSym, BinaryOperator::Opcode Opc);
const ContainerData *getContainerData(ProgramStateRef State,
const MemRegion *Cont);
ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
@@ -453,7 +470,12 @@
const auto Op =
[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers
baloghadamsoftware updated this revision to Diff 130185.
baloghadamsoftware added a comment.
Rebased.
https://reviews.llvm.org/D32859
Files:
lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
test/Analysis/mismatched-iterator.cpp
Index: test/Analysis/mismatched-iterator.cpp
===
--- test/Analysis/mismatched-iterator.cpp
+++ test/Analysis/mismatched-iterator.cpp
@@ -15,11 +15,49 @@
std::copy(v1.cbegin(), v1.cend(), v2.begin()); // no-warning
}
+void good_move_find1(std::vector , std::vector , int n) {
+ auto i0 = v2.cbegin();
+ v1 = std::move(v2);
+ std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find2(std::vector , std::vector , int n) {
+ auto i0 = --v2.cend();
+ v1 = std::move(v2);
+ std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find3(std::vector , std::vector , int n) {
+ auto i0 = v2.cend();
+ v1 = std::move(v2);
+ v2.push_back(n);
+ std::find(v2.cbegin(), i0, n); // no-warning
+}
+
void bad_find(std::vector , std::vector , int n) {
std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterator access mismatched}}
}
void bad_find_first_of(std::vector , std::vector ) {
std::find_first_of(v1.cbegin(), v2.cend(), v2.cbegin(), v2.cend()); // expected-warning{{Iterator access mismatched}}
std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v1.cend()); // expected-warning{{Iterator access mismatched}}
}
+
+void bad_move_find1(std::vector , std::vector , int n) {
+ auto i0 = v2.cbegin();
+ v1 = std::move(v2);
+ std::find(i0, v2.cend(), n); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_move_find2(std::vector , std::vector , int n) {
+ auto i0 = --v2.cend();
+ v1 = std::move(v2);
+ std::find(i0, v2.cend(), n); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_move_find3(std::vector , std::vector , int n) {
+ auto i0 = v2.cend();
+ v1 = std::move(v2);
+ std::find(v1.cbegin(), i0, n); // expected-warning{{Iterator access mismatched}}
+}
+
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -114,6 +114,10 @@
return IteratorPosition(Cont, Valid, NewOf);
}
+ IteratorPosition reAssign(const MemRegion *NewCont) const {
+return IteratorPosition(NewCont, Valid, Offset);
+ }
+
bool operator==(const IteratorPosition ) const {
return Cont == X.Cont && Valid == X.Valid && Offset == X.Offset;
}
@@ -219,7 +223,9 @@
const SVal ) const;
void assignToContainer(CheckerContext , const Expr *CE, const SVal ,
const MemRegion *Cont) const;
- void handleAssign(CheckerContext , const SVal ) const;
+ void handleAssign(CheckerContext , const SVal ,
+const Expr *CE = nullptr,
+const SVal = UndefinedVal()) const;
void verifyRandomIncrOrDecr(CheckerContext , OverloadedOperatorKind Op,
const SVal , const SVal ,
const SVal ) const;
@@ -317,6 +323,17 @@
bool Equal);
ProgramStateRef invalidateAllIteratorPositions(ProgramStateRef State,
const MemRegion *Cont);
+ProgramStateRef reassignAllIteratorPositions(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont);
+ProgramStateRef reassignAllIteratorPositionsUnless(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont,
+ SymbolRef Offset,
+ BinaryOperator::Opcode Opc);
+ProgramStateRef replaceSymbolInIteratorPositionsIf(
+ProgramStateRef State, SValBuilder , SymbolRef OldSym,
+SymbolRef NewSym, SymbolRef CondSym, BinaryOperator::Opcode Opc);
const ContainerData *getContainerData(ProgramStateRef State,
const MemRegion *Cont);
ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
@@ -453,7 +470,12 @@
const auto Op = Func->getOverloadedOperator();
if (isAssignmentOperator(Op)) {
const auto *InstCall = dyn_cast();
- handleAssign(C, InstCall->getCXXThisVal());
+ if (Func->getParamDecl(0)->getType()->isRValueReferenceType()) {
+handleAssign(C, InstCall->getCXXThisVal(), Call.getOriginExpr(),
+ Call.getArgSVal(0));
+ } else {
+handleAssign(C, InstCall->getCXXThisVal());
+ }
} else if (isSimpleComparisonOperator(Op)) {
if (const auto *InstCall = dyn_cast()) {
handleComparison(C,
[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers
baloghadamsoftware updated this revision to Diff 129960.
baloghadamsoftware added a comment.
Rebased to current part 4. New tests added. Comments added.
https://reviews.llvm.org/D32859
Files:
lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
test/Analysis/mismatched-iterator.cpp
Index: test/Analysis/mismatched-iterator.cpp
===
--- test/Analysis/mismatched-iterator.cpp
+++ test/Analysis/mismatched-iterator.cpp
@@ -15,11 +15,49 @@
std::copy(v1.cbegin(), v1.cend(), v2.begin()); // no-warning
}
+void good_move_find1(std::vector , std::vector , int n) {
+ auto i0 = v2.cbegin();
+ v1 = std::move(v2);
+ std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find2(std::vector , std::vector , int n) {
+ auto i0 = --v2.cend();
+ v1 = std::move(v2);
+ std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find3(std::vector , std::vector , int n) {
+ auto i0 = v2.cend();
+ v1 = std::move(v2);
+ v2.push_back(n);
+ std::find(v2.cbegin(), i0, n); // no-warning
+}
+
void bad_find(std::vector , std::vector , int n) {
std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterator access mismatched}}
}
void bad_find_first_of(std::vector , std::vector ) {
std::find_first_of(v1.cbegin(), v2.cend(), v2.cbegin(), v2.cend()); // expected-warning{{Iterator access mismatched}}
std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v1.cend()); // expected-warning{{Iterator access mismatched}}
}
+
+void bad_move_find1(std::vector , std::vector , int n) {
+ auto i0 = v2.cbegin();
+ v1 = std::move(v2);
+ std::find(i0, v2.cend(), n); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_move_find2(std::vector , std::vector , int n) {
+ auto i0 = --v2.cend();
+ v1 = std::move(v2);
+ std::find(i0, v2.cend(), n); // expected-warning{{Iterator access mismatched}}
+}
+
+void bad_move_find3(std::vector , std::vector , int n) {
+ auto i0 = v2.cend();
+ v1 = std::move(v2);
+ std::find(v1.cbegin(), i0, n); // expected-warning{{Iterator access mismatched}}
+}
+
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -114,6 +114,10 @@
return IteratorPosition(Cont, Valid, NewOf);
}
+ IteratorPosition reAssign(const MemRegion *NewCont) const {
+return IteratorPosition(NewCont, Valid, Offset);
+ }
+
bool operator==(const IteratorPosition ) const {
return Cont == X.Cont && Valid == X.Valid && Offset == X.Offset;
}
@@ -219,7 +223,9 @@
const SVal ) const;
void assignToContainer(CheckerContext , const Expr *CE, const SVal ,
const MemRegion *Cont) const;
- void handleAssign(CheckerContext , const SVal ) const;
+ void handleAssign(CheckerContext , const SVal ,
+const Expr *CE = nullptr,
+const SVal = UndefinedVal()) const;
void verifyRandomIncrOrDecr(CheckerContext , OverloadedOperatorKind Op,
const SVal , const SVal ,
const SVal ) const;
@@ -316,6 +322,17 @@
bool Equal);
ProgramStateRef invalidateAllIteratorPositions(ProgramStateRef State,
const MemRegion *Cont);
+ProgramStateRef reassignAllIteratorPositions(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont);
+ProgramStateRef reassignAllIteratorPositionsUnless(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont,
+ SymbolRef Offset,
+ BinaryOperator::Opcode Opc);
+ProgramStateRef replaceSymbolInIteratorPositionsIf(
+ProgramStateRef State, SValBuilder , SymbolRef OldSym,
+SymbolRef NewSym, SymbolRef CondSym, BinaryOperator::Opcode Opc);
const ContainerData *getContainerData(ProgramStateRef State,
const MemRegion *Cont);
ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
@@ -452,7 +469,12 @@
const auto Op = Func->getOverloadedOperator();
if (isAssignmentOperator(Op)) {
const auto *InstCall = dyn_cast();
- handleAssign(C, InstCall->getCXXThisVal());
+ if (Func->getParamDecl(0)->getType()->isRValueReferenceType()) {
+handleAssign(C, InstCall->getCXXThisVal(), Call.getOriginExpr(),
+ Call.getArgSVal(0));
+ } else {
+handleAssign(C, InstCall->getCXXThisVal());
+ }
} else if (isSimpleComparisonOperator(Op)) {
if (const auto *InstCall =
[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers
NoQ added inline comments.
Herald added subscribers: a.sidorin, rnkovacs, szepet.
Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1489-1511
+SymbolRef replaceSymbol(SymbolManager , SymbolRef OrigExpr,
+SymbolRef OldExpr, SymbolRef NewSym) {
+ // We have a symbolic expression in the format of A+n, where we want to
+ // replace A+m with B, so the result becomes B+k, where k==n-m.
+ SymbolRef OrigSym, OldSym;
+ llvm::APSInt OrigInt, OldInt;
+ std::tie(OrigSym, OrigInt) = decompose(OrigExpr);
I do not immediately understand what is this useful for. At least tests don't
look like they make use of these offset manipulations(?)
Without full understanding, i wonder: when we overwrite one container with
another, why don't we just overwrite //all// symbols associated with it,
instead of creating a mixture of old and new symbols?
Or maybe this is an accidental part of another patch, that has something to do
with resizes?
https://reviews.llvm.org/D32859
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers
baloghadamsoftware created this revision.
If a container is moved by its move assignment operator, according to the
standard all their iterators except the past-end iterators remain valid but
refer to the new container. This patch introduces support for this case in the
iterator checkers.
https://reviews.llvm.org/D32859
Files:
lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
test/Analysis/mismatched-iterator.cpp
Index: test/Analysis/mismatched-iterator.cpp
===
--- test/Analysis/mismatched-iterator.cpp
+++ test/Analysis/mismatched-iterator.cpp
@@ -15,11 +15,23 @@
std::copy(v1.cbegin(), v1.cend(), v2.begin()); // no-warning
}
+void good_move_find(std::vector , std::vector , int n) {
+ auto i0 = v2.cbegin();
+ v1 = std::move(v2);
+ std::find(i0, v1.cend(), n); // no-warning
+}
+
void bad_find(std::vector , std::vector , int n) {
std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterator access mismatched}}
}
void bad_find_first_of(std::vector , std::vector ) {
std::find_first_of(v1.cbegin(), v2.cend(), v2.cbegin(), v2.cend()); // expected-warning{{Iterator access mismatched}}
std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v1.cend()); // expected-warning{{Iterator access mismatched}}
}
+
+void bad_move_find(std::vector , std::vector , int n) {
+ auto i0 = v2.cbegin();
+ v1 = std::move(v2);
+ std::find(i0, v2.cend(), n); // expected-warning{{Iterator access mismatched}}
+}
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -51,6 +51,10 @@
return IteratorPosition(Cont, Valid, NewOf);
}
+ IteratorPosition reAssign(const MemRegion *NewCont) const {
+return IteratorPosition(NewCont, Valid, Offset);
+ }
+
bool operator==(const IteratorPosition ) const {
return Cont == X.Cont && Valid == X.Valid && Offset == X.Offset;
}
@@ -153,7 +157,9 @@
const SVal ) const;
void assignToContainer(CheckerContext , const Expr *CE, const SVal ,
const MemRegion *Cont) const;
- void handleAssign(CheckerContext , const SVal ) const;
+ void handleAssign(CheckerContext , const SVal ,
+const Expr *CE = nullptr,
+const SVal = UndefinedVal()) const;
void verifyRandomIncrOrDecr(CheckerContext , OverloadedOperatorKind Op,
const SVal , const SVal ,
const SVal ) const;
@@ -257,6 +263,17 @@
bool Equal);
ProgramStateRef invalidateAllIteratorPositions(ProgramStateRef State,
const MemRegion *Cont);
+ProgramStateRef reassignAllIteratorPositions(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont);
+ProgramStateRef reassignAllIteratorPositionsUnless(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont,
+ SymbolRef Offset,
+ BinaryOperator::Opcode Opc);
+ProgramStateRef replaceSymbolInIteratorPositionsIf(
+ProgramStateRef State, SymbolManager , SymbolRef OldSym,
+SymbolRef NewSym, SymbolRef CondSym, BinaryOperator::Opcode Opc);
const ContainerData *getContainerData(ProgramStateRef State,
const MemRegion *Cont);
ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
@@ -386,7 +403,12 @@
const auto Op = Func->getOverloadedOperator();
if (isAssignmentOperator(Op)) {
const auto *InstCall = dyn_cast();
- handleAssign(C, InstCall->getCXXThisVal());
+ if (Func->getParamDecl(0)->getType()->isRValueReferenceType()) {
+handleAssign(C, InstCall->getCXXThisVal(), Call.getOriginExpr(),
+ Call.getArgSVal(0));
+ } else {
+handleAssign(C, InstCall->getCXXThisVal());
+ }
} else if (isSimpleComparisonOperator(Op)) {
if (const auto *InstCall = dyn_cast()) {
handleComparison(C, Call.getReturnValue(), InstCall->getCXXThisVal(),
@@ -873,7 +895,8 @@
C.addTransition(State);
}
-void IteratorChecker::handleAssign(CheckerContext , const SVal ) const {
+void IteratorChecker::handleAssign(CheckerContext , const SVal ,
+ const Expr *CE, const SVal ) const {
const auto *ContReg = Cont.getAsRegion();
if (!ContReg)
return;
@@ -889,6 +912,51 @@
if (CData) {
State = invalidateAllIteratorPositions(State, ContReg);
}
+
+ // In case of move, iterators of the old container
