MTC updated this revision to Diff 142026.
MTC marked 2 inline comments as done.
MTC added a comment.
- Move the `CXXThisRegion`'s check to `LoopWidening.cpp`
- Use `isa<CXXThisRegion>(R)` instead of `CXXThisRegion::classof(R)`.
Repository:
rC Clang
https://reviews.llvm.org/D45491
Files:
lib/StaticAnalyzer/Core/LoopWidening.cpp
lib/StaticAnalyzer/Core/RegionStore.cpp
test/Analysis/this-pointer.cpp
Index: test/Analysis/this-pointer.cpp
===================================================================
--- /dev/null
+++ test/Analysis/this-pointer.cpp
@@ -0,0 +1,88 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config widen-loops=true -analyzer-disable-retry-exhausted -verify %s
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_dump(int);
+
+// 'this' pointer is not an lvalue, we should not invalidate it.
+namespace this_pointer_after_loop_widen {
+class A {
+public:
+ A() {
+ int count = 10;
+ do {
+ } while (count--);
+ }
+};
+
+void goo(A a);
+void test_temporary_object() {
+ goo(A()); // no-crash
+}
+
+struct B {
+ int mem;
+ B() : mem(0) {
+ for (int i = 0; i < 10; ++i) {
+ }
+ mem = 0;
+ }
+};
+
+void test_ctor() {
+ B b;
+ clang_analyzer_eval(b.mem == 0); // expected-warning{{TRUE}}
+}
+
+struct C {
+ int mem;
+ C() : mem(0) {}
+ void set() {
+ for (int i = 0; i < 10; ++i) {
+ }
+ mem = 10;
+ }
+};
+
+void test_method() {
+ C c;
+ clang_analyzer_eval(c.mem == 0); // expected-warning{{TRUE}}
+ c.set();
+ clang_analyzer_eval(c.mem == 10); // expected-warning{{TRUE}}
+}
+
+struct D {
+ int mem;
+ D() : mem(0) {}
+ void set() {
+ for (int i = 0; i < 10; ++i) {
+ }
+ mem = 10;
+ }
+};
+
+void test_new() {
+ D *d = new D;
+ clang_analyzer_eval(d->mem == 0); // expected-warning{{TRUE}}
+ d->set();
+ clang_analyzer_eval(d->mem == 10); // expected-warning{{TRUE}}
+}
+
+struct E {
+ int mem;
+ E() : mem(0) {}
+ void set() {
+ for (int i = 0; i < 10; ++i) {
+ }
+ setAux();
+ }
+ void setAux() {
+ this->mem = 10;
+ }
+};
+
+void test_chained_method_call() {
+ E e;
+ e.set();
+ clang_analyzer_eval(e.mem == 10); // expected-warning{{TRUE}}
+}
+} // namespace this_pointer_after_loop_widen
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2050,6 +2050,9 @@
R = GetElementZeroRegion(SR, T);
}
+ assert((!isa<CXXThisRegion>(R) || !B.lookup(R)) &&
+ "'this' pointer is not an l-value and is not assignable");
+
// Clear out bindings that may overlap with this binding.
RegionBindingsRef NewB = removeSubRegionBindings(B, cast<SubRegion>(R));
return NewB.addBinding(BindingKey::Make(R, BindingKey::Direct), V);
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===================================================================
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -59,6 +59,18 @@
ITraits.setTrait(Region,
RegionAndSymbolInvalidationTraits::TK_EntireMemSpace);
}
+
+ // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
+ // is located in a method, constructor or destructor, the value of 'this'
+ // pointer shoule remain unchanged.
+ if (const CXXMethodDecl *CXXMD = dyn_cast<CXXMethodDecl>(STC->getDecl())) {
+ const CXXThisRegion *ThisR = MRMgr.getCXXThisRegion(
+ CXXMD->getThisType(STC->getAnalysisDeclContext()->getASTContext()),
+ STC);
+ ITraits.setTrait(ThisR,
+ RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+ }
+
return PrevState->invalidateRegions(Regions, getLoopCondition(LoopStmt),
BlockCount, LCtx, true, nullptr, nullptr,
&ITraits);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
