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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits