r330095 - [analyzer] Do not invalidate the `this` pointer.

[Henry Wong via cfe-commits]

Author: henrywong
Date: Sun Apr 15 03:34:06 2018
New Revision: 330095

URL: http://llvm.org/viewvc/llvm-project?rev=330095&view=rev
Log:
[analyzer] Do not invalidate the `this` pointer.

Summary:
`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.

Reviewers: NoQ, george.karpenkov, a.sidorin, seaneveson, szepet

Reviewed By: NoQ

Subscribers: xazax.hun, rnkovacs, cfe-commits, MTC

Differential Revision: https://reviews.llvm.org/D45491

Added:
    cfe/trunk/test/Analysis/this-pointer.cpp
Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp?rev=330095&r1=330094&r2=330095&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp Sun Apr 15 03:34:06 2018
@@ -59,6 +59,18 @@ ProgramStateRef getWidenedLoopState(Prog
     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);

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=330095&r1=330094&r2=330095&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Sun Apr 15 03:34:06 2018
@@ -2050,6 +2050,9 @@ RegionStoreManager::bind(RegionBindingsC
     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);

Added: cfe/trunk/test/Analysis/this-pointer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/this-pointer.cpp?rev=330095&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/this-pointer.cpp (added)
+++ cfe/trunk/test/Analysis/this-pointer.cpp Sun Apr 15 03:34:06 2018
@@ -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