MTC created this revision.
MTC added reviewers: NoQ, george.karpenkov, a.sidorin, seaneveson, szepet.
Herald added subscribers: cfe-commits, rnkovacs, xazax.hun.
MTC edited the summary of this revision.
`this` pointer is not an l-value, although we have modeled `CXXThisRegion` for
`this` pointer, we can only bind it once, which is when we start to inline
method. And this patch fixes https://bugs.llvm.org/show_bug.cgi?id=35506.
In addition, I didn't find any other cases other than loop-widen that could
invalidate `this` pointer.
Repository:
rC Clang
https://reviews.llvm.org/D45491
Files:
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,87 @@
+// 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);
+
+// '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 func1() {
+ goo(A()); // no-crash
+}
+
+struct B {
+ int mem;
+ B() : mem(0) {
+ for (int i = 0; i < 10; ++i) {
+ }
+ mem = 0;
+ }
+};
+
+void func2() {
+ 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 func3() {
+ 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 func4() {
+ 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 func6() {
+ 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
@@ -1021,6 +1021,10 @@
void invalidateRegionsWorker::VisitCluster(const MemRegion *baseR,
const ClusterBindings *C) {
+ // 'this' pointer is not an lvalue, we should not invalidate it.
+ if (CXXThisRegion::classof(baseR))
+ return;
+
bool PreserveRegionsContents =
ITraits.hasTrait(baseR,
RegionAndSymbolInvalidationTraits::TK_PreserveContents);
@@ -2050,8 +2054,12 @@
R = GetElementZeroRegion(SR, T);
}
+ assert((!CXXThisRegion::classof(R) ||
+ CXXThisRegion::classof(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));
+ RegionBindingsRef NewB = removeSubRegionBindings(B, cast<SubRegion>(R));
return NewB.addBinding(BindingKey::Make(R, BindingKey::Direct), V);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
