baloghadamsoftware updated this revision to Diff 176374.
baloghadamsoftware added a comment.
Now I think I understand the terminology and the concept so I could add Doxygen
comment. I also refactored the code as you suggested, original code was based
on `getBaseRegion()`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54466/new/
https://reviews.llvm.org/D54466
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
lib/StaticAnalyzer/Core/MemRegion.cpp
test/Analysis/iterator-range.cpp
Index: test/Analysis/iterator-range.cpp
===================================================================
--- test/Analysis/iterator-range.cpp
+++ test/Analysis/iterator-range.cpp
@@ -200,3 +200,40 @@
++i0;
*++i0; // expected-warning{{Iterator accessed outside of its range}}
}
+
+struct simple_iterator_base {
+ simple_iterator_base();
+ simple_iterator_base(const simple_iterator_base& rhs);
+ simple_iterator_base &operator=(const simple_iterator_base& rhs);
+ virtual ~simple_iterator_base();
+ bool friend operator==(const simple_iterator_base &lhs,
+ const simple_iterator_base &rhs);
+ bool friend operator!=(const simple_iterator_base &lhs,
+ const simple_iterator_base &rhs);
+private:
+ int *ptr;
+};
+
+struct simple_derived_iterator: public simple_iterator_base {
+ int& operator*();
+ int* operator->();
+ simple_iterator_base &operator++();
+ simple_iterator_base operator++(int);
+ simple_iterator_base &operator--();
+ simple_iterator_base operator--(int);
+};
+
+struct simple_container {
+ typedef simple_derived_iterator iterator;
+
+ iterator begin();
+ iterator end();
+};
+
+void good_derived(simple_container c) {
+ auto i0 = c.end();
+ if (i0 != c.end()) {
+ clang_analyzer_warnIfReached();
+ *i0; // no-warning
+ }
+}
Index: lib/StaticAnalyzer/Core/MemRegion.cpp
===================================================================
--- lib/StaticAnalyzer/Core/MemRegion.cpp
+++ lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1175,6 +1175,15 @@
return R;
}
+// getgetMostDerivedObjectRegion gets the region of the root class of a C++
+// class hierarchy.
+const MemRegion *MemRegion::getMostDerivedObjectRegion() const {
+ const MemRegion *R = this;
+ while (const auto *BR = dyn_cast<CXXBaseObjectRegion>(R))
+ R = BR->getSuperRegion();
+ return R;
+}
+
bool MemRegion::isSubRegionOf(const MemRegion *) const {
return false;
}
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -1089,9 +1089,7 @@
void IteratorChecker::verifyMatch(CheckerContext &C, const SVal &Iter,
const MemRegion *Cont) const {
// Verify match between a container and the container of an iterator
- while (const auto *CBOR = Cont->getAs<CXXBaseObjectRegion>()) {
- Cont = CBOR->getSuperRegion();
- }
+ Cont = Cont->getMostDerivedObjectRegion();
auto State = C.getState();
const auto *Pos = getIteratorPosition(State, Iter);
@@ -1125,9 +1123,7 @@
if (!ContReg)
return;
- while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
- ContReg = CBOR->getSuperRegion();
- }
+ ContReg = ContReg->getMostDerivedObjectRegion();
// If the container already has a begin symbol then use it. Otherwise first
// create a new one.
@@ -1151,9 +1147,7 @@
if (!ContReg)
return;
- while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
- ContReg = CBOR->getSuperRegion();
- }
+ ContReg = ContReg->getMostDerivedObjectRegion();
// If the container already has an end symbol then use it. Otherwise first
// create a new one.
@@ -1174,9 +1168,7 @@
void IteratorChecker::assignToContainer(CheckerContext &C, const Expr *CE,
const SVal &RetVal,
const MemRegion *Cont) const {
- while (const auto *CBOR = Cont->getAs<CXXBaseObjectRegion>()) {
- Cont = CBOR->getSuperRegion();
- }
+ Cont = Cont->getMostDerivedObjectRegion();
auto State = C.getState();
auto &SymMgr = C.getSymbolManager();
@@ -1194,9 +1186,7 @@
if (!ContReg)
return;
- while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
- ContReg = CBOR->getSuperRegion();
- }
+ ContReg = ContReg->getMostDerivedObjectRegion();
// Assignment of a new value to a container always invalidates all its
// iterators
@@ -1211,9 +1201,7 @@
if (!OldCont.isUndef()) {
const auto *OldContReg = OldCont.getAsRegion();
if (OldContReg) {
- while (const auto *CBOR = OldContReg->getAs<CXXBaseObjectRegion>()) {
- OldContReg = CBOR->getSuperRegion();
- }
+ OldContReg = OldContReg->getMostDerivedObjectRegion();
const auto OldCData = getContainerData(State, OldContReg);
if (OldCData) {
if (const auto OldEndSym = OldCData->getEnd()) {
@@ -1273,9 +1261,7 @@
if (!ContReg)
return;
- while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
- ContReg = CBOR->getSuperRegion();
- }
+ ContReg = ContReg->getMostDerivedObjectRegion();
// The clear() operation invalidates all the iterators, except the past-end
// iterators of list-like containers
@@ -1302,9 +1288,7 @@
if (!ContReg)
return;
- while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
- ContReg = CBOR->getSuperRegion();
- }
+ ContReg = ContReg->getMostDerivedObjectRegion();
// For deque-like containers invalidate all iterator positions
auto State = C.getState();
@@ -1341,9 +1325,7 @@
if (!ContReg)
return;
- while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
- ContReg = CBOR->getSuperRegion();
- }
+ ContReg = ContReg->getMostDerivedObjectRegion();
auto State = C.getState();
const auto CData = getContainerData(State, ContReg);
@@ -1381,9 +1363,7 @@
if (!ContReg)
return;
- while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
- ContReg = CBOR->getSuperRegion();
- }
+ ContReg = ContReg->getMostDerivedObjectRegion();
// For deque-like containers invalidate all iterator positions
auto State = C.getState();
@@ -1416,9 +1396,7 @@
if (!ContReg)
return;
- while (const auto *CBOR = ContReg->getAs<CXXBaseObjectRegion>()) {
- ContReg = CBOR->getSuperRegion();
- }
+ ContReg = ContReg->getMostDerivedObjectRegion();
auto State = C.getState();
const auto CData = getContainerData(State, ContReg);
@@ -2015,7 +1993,8 @@
const IteratorPosition *getIteratorPosition(ProgramStateRef State,
const SVal &Val) {
- if (const auto Reg = Val.getAsRegion()) {
+ if (auto Reg = Val.getAsRegion()) {
+ Reg = Reg->getMostDerivedObjectRegion();
return State->get<IteratorRegionMap>(Reg);
} else if (const auto Sym = Val.getAsSymbol()) {
return State->get<IteratorSymbolMap>(Sym);
@@ -2028,7 +2007,8 @@
const IteratorPosition *getIteratorPosition(ProgramStateRef State,
RegionOrSymbol RegOrSym) {
if (RegOrSym.is<const MemRegion *>()) {
- return State->get<IteratorRegionMap>(RegOrSym.get<const MemRegion *>());
+ auto Reg = RegOrSym.get<const MemRegion *>()->getMostDerivedObjectRegion();
+ return State->get<IteratorRegionMap>(Reg);
} else if (RegOrSym.is<SymbolRef>()) {
return State->get<IteratorSymbolMap>(RegOrSym.get<SymbolRef>());
}
@@ -2037,7 +2017,8 @@
ProgramStateRef setIteratorPosition(ProgramStateRef State, const SVal &Val,
const IteratorPosition &Pos) {
- if (const auto Reg = Val.getAsRegion()) {
+ if (auto Reg = Val.getAsRegion()) {
+ Reg = Reg->getMostDerivedObjectRegion();
return State->set<IteratorRegionMap>(Reg, Pos);
} else if (const auto Sym = Val.getAsSymbol()) {
return State->set<IteratorSymbolMap>(Sym, Pos);
@@ -2051,8 +2032,8 @@
RegionOrSymbol RegOrSym,
const IteratorPosition &Pos) {
if (RegOrSym.is<const MemRegion *>()) {
- return State->set<IteratorRegionMap>(RegOrSym.get<const MemRegion *>(),
- Pos);
+ auto Reg = RegOrSym.get<const MemRegion *>()->getMostDerivedObjectRegion();
+ return State->set<IteratorRegionMap>(Reg, Pos);
} else if (RegOrSym.is<SymbolRef>()) {
return State->set<IteratorSymbolMap>(RegOrSym.get<SymbolRef>(), Pos);
}
@@ -2060,7 +2041,8 @@
}
ProgramStateRef removeIteratorPosition(ProgramStateRef State, const SVal &Val) {
- if (const auto Reg = Val.getAsRegion()) {
+ if (auto Reg = Val.getAsRegion()) {
+ Reg = Reg->getMostDerivedObjectRegion();
return State->remove<IteratorRegionMap>(Reg);
} else if (const auto Sym = Val.getAsSymbol()) {
return State->remove<IteratorSymbolMap>(Sym);
Index: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -118,6 +118,10 @@
const MemRegion *getBaseRegion() const;
+ /// Recursively retrieve the region of the most derived class instance of
+ /// regions of C++ base class instances.
+ const MemRegion *getMostDerivedObjectRegion() const;
+
/// Check if the region is a subregion of the given region.
/// Each region is a subregion of itself.
virtual bool isSubRegionOf(const MemRegion *R) const;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
