[analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods...

Prevent invalidation of `this' when a method is const; fixing PR 21606.

A patch by Sean Eveson!

Differential Revision: http://reviews.llvm.org/D13099

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@250237 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index 63b8631665ac756c059335b0d128ff6e25ac51b9..cf888b5040e5f24a6f3582d03b2a98b087838131 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -164,7 +164,8 @@ protected:
 
   /// \brief Used to specify non-argument regions that will be invalidated as a
   /// result of this call.
-  virtual void getExtraInvalidatedValues(ValueList &Values) const {}
+  virtual void getExtraInvalidatedValues(ValueList &Values,
+                 RegionAndSymbolInvalidationTraits *ETraits) const {}
 
 public:
   virtual ~CallEvent() {}
@@ -472,7 +473,8 @@ protected:
   BlockCall(const BlockCall &Other) : CallEvent(Other) {}
   void cloneTo(void *Dest) const override { new (Dest) BlockCall(*this); }
 
-  void getExtraInvalidatedValues(ValueList &Values) const override;
+  void getExtraInvalidatedValues(ValueList &Values,
+         RegionAndSymbolInvalidationTraits *ETraits) const override;
 
 public:
   virtual const CallExpr *getOriginExpr() const {
@@ -521,7 +523,8 @@ public:
 /// it is written.
 class CXXInstanceCall : public AnyFunctionCall {
 protected:
-  void getExtraInvalidatedValues(ValueList &Values) const override;
+  void getExtraInvalidatedValues(ValueList &Values, 
+         RegionAndSymbolInvalidationTraits *ETraits) const override;
 
   CXXInstanceCall(const CallExpr *CE, ProgramStateRef St,
                   const LocationContext *LCtx)
@@ -704,7 +707,8 @@ protected:
   CXXConstructorCall(const CXXConstructorCall &Other) : AnyFunctionCall(Other){}
   void cloneTo(void *Dest) const override { new (Dest) CXXConstructorCall(*this); }
 
-  void getExtraInvalidatedValues(ValueList &Values) const override;
+  void getExtraInvalidatedValues(ValueList &Values,
+         RegionAndSymbolInvalidationTraits *ETraits) const override;
 
 public:
   virtual const CXXConstructExpr *getOriginExpr() const {
@@ -803,7 +807,8 @@ protected:
   ObjCMethodCall(const ObjCMethodCall &Other) : CallEvent(Other) {}
   void cloneTo(void *Dest) const override { new (Dest) ObjCMethodCall(*this); }
 
-  void getExtraInvalidatedValues(ValueList &Values) const override;
+  void getExtraInvalidatedValues(ValueList &Values,
+         RegionAndSymbolInvalidationTraits *ETraits) const override;
 
   /// Check if the selector may have multiple definitions (may have overrides).
   virtual bool canBeOverridenInSubclass(ObjCInterfaceDecl *IDecl,

diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp
index 5b6a8258d7adfcb22a07db269de96c611d75e4f8..a287569723133e33902653b2fb3c08845f554fc6 100644 (file)
--- a/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -148,7 +148,7 @@ ProgramStateRef CallEvent::invalidateRegions(unsigned BlockCount,
   SmallVector<SVal, 8> ValuesToInvalidate;
   RegionAndSymbolInvalidationTraits ETraits;
 
-  getExtraInvalidatedValues(ValuesToInvalidate);
+  getExtraInvalidatedValues(ValuesToInvalidate, &ETraits);
 
   // Indexes of arguments whose values will be preserved by the call.
   llvm::SmallSet<unsigned, 4> PreserveArgs;
@@ -403,8 +403,28 @@ const FunctionDecl *CXXInstanceCall::getDecl() const {
   return getSVal(CE->getCallee()).getAsFunctionDecl();
 }
 
-void CXXInstanceCall::getExtraInvalidatedValues(ValueList &Values) const {
-  Values.push_back(getCXXThisVal());
+void CXXInstanceCall::getExtraInvalidatedValues(ValueList &Values,
+                        RegionAndSymbolInvalidationTraits *ETraits) const {
+  SVal ThisVal = getCXXThisVal();
+  Values.push_back(ThisVal);
+
+  // Don't invalidate if the method is const and there are no mutable fields
+  if (const CXXMethodDecl *D = cast_or_null<CXXMethodDecl>(getDecl())) {
+    if (!D->isConst())
+      return;
+    // Get the record decl for the class of 'This'. D->getParent() may return a
+    // base class decl, rather than the class of the instance which needs to be
+    // checked for mutable fields.
+    const Expr *Ex = getCXXThisExpr()->ignoreParenBaseCasts();
+    const CXXRecordDecl *ParentRecord = Ex->getType()->getAsCXXRecordDecl();
+    if (!ParentRecord || ParentRecord->hasMutableFields())
+      return;
+    // Preserve CXXThis.
+    const MemRegion *ThisRegion = ThisVal.getAsRegion();
+    assert(ThisRegion && "ThisValue was not a memory region");
+    ETraits->setTrait(ThisRegion->getBaseRegion(),
+      RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+  }
 }
 
 SVal CXXInstanceCall::getCXXThisVal() const {
@@ -550,7 +570,8 @@ ArrayRef<ParmVarDecl*> BlockCall::parameters() const {
   return D->parameters();
 }
 
-void BlockCall::getExtraInvalidatedValues(ValueList &Values) const {
+void BlockCall::getExtraInvalidatedValues(ValueList &Values,
+                  RegionAndSymbolInvalidationTraits *ETraits) const {
   // FIXME: This also needs to invalidate captured globals.
   if (const MemRegion *R = getBlockRegion())
     Values.push_back(loc::MemRegionVal(R));
@@ -571,7 +592,8 @@ SVal CXXConstructorCall::getCXXThisVal() const {
   return UnknownVal();
 }
 
-void CXXConstructorCall::getExtraInvalidatedValues(ValueList &Values) const {
+void CXXConstructorCall::getExtraInvalidatedValues(ValueList &Values,
+                           RegionAndSymbolInvalidationTraits *ETraits) const {
   if (Data)
     Values.push_back(loc::MemRegionVal(static_cast<const MemRegion *>(Data)));
 }
@@ -613,7 +635,8 @@ ArrayRef<ParmVarDecl*> ObjCMethodCall::parameters() const {
 }
 
 void
-ObjCMethodCall::getExtraInvalidatedValues(ValueList &Values) const {
+ObjCMethodCall::getExtraInvalidatedValues(ValueList &Values,
+                  RegionAndSymbolInvalidationTraits *ETraits) const {
   Values.push_back(getReceiverSVal());
 }
 

diff --git a/test/Analysis/const-method-call.cpp b/test/Analysis/const-method-call.cpp
new file mode 100644 (file)
index 0000000..25909f8
--- /dev/null
+++ b/test/Analysis/const-method-call.cpp
@@ -0,0 +1,230 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(bool);
+
+struct A {
+  int x;
+  void foo() const;
+  void bar();
+};
+
+struct B {
+  mutable int mut;
+  void foo() const;
+};
+
+struct C {
+  int *p;
+  void foo() const;
+};
+
+struct MutBase {
+  mutable int b_mut;
+};
+
+struct MutDerived : MutBase {
+  void foo() const;
+};
+
+struct PBase {
+  int *p;
+};
+
+struct PDerived : PBase {
+  void foo() const;
+};
+
+struct Inner {
+  int x;
+  int *p;
+  void bar() const;
+};
+
+struct Outer {
+  int x;
+  Inner in;
+  void foo() const;
+};
+
+void checkThatConstMethodWithoutDefinitionDoesNotInvalidateObject() {
+  A t;
+  t.x = 3;
+  t.foo();
+  clang_analyzer_eval(t.x == 3); // expected-warning{{TRUE}}
+  // Test non-const does invalidate
+  t.bar();
+  clang_analyzer_eval(t.x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateMutableFields() {
+  B t;
+  t.mut = 4;
+  t.foo();
+  clang_analyzer_eval(t.mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidatePointedAtMemory() {
+  int x = 1;
+  C t;
+  t.p = &x;
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.p == &x); // expected-warning{{TRUE}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedMutableFields() {
+  MutDerived t;
+  t.b_mut = 4;
+  t.foo();
+  clang_analyzer_eval(t.b_mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedPointedAtMemory() {
+  int x = 1;
+  PDerived t;
+  t.p = &x;
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.p == &x); // expected-warning{{TRUE}}
+}
+
+void checkThatConstMethodDoesInvalidateContainedPointedAtMemory() {
+  int x = 1;
+  Outer t;
+  t.x = 2;
+  t.in.p = &x;
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.p == &x); // expected-warning{{TRUE}}
+}
+
+void checkThatContainedConstMethodDoesNotInvalidateObjects() {
+  Outer t;
+  t.x = 1;
+  t.in.x = 2;
+  t.in.bar();
+  clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.x == 2); // expected-warning{{TRUE}}
+}
+
+// --- Versions of the above tests where the const method is inherited --- //
+
+struct B1 {
+  void foo() const;
+};
+
+struct D1 : public B1 {
+  int x;
+};
+
+struct D2 : public B1 {
+  mutable int mut;
+};
+
+struct D3 : public B1 {
+  int *p;
+};
+
+struct DInner : public B1 {
+  int x;
+  int *p;
+};
+
+struct DOuter : public B1 {
+  int x;
+  DInner in;
+};
+
+void checkThatInheritedConstMethodDoesNotInvalidateObject() {
+  D1 t;
+  t.x = 1;
+  t.foo();
+  clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidateMutableFields() {
+  D2 t;
+  t.mut = 1;
+  t.foo();
+  clang_analyzer_eval(t.mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidatePointedAtMemory() {
+  int x = 1;
+  D3 t;
+  t.p = &x;
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.p == &x); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidateContainedPointedAtMemory() {
+  int x = 1;
+  DOuter t;
+  t.x = 2;
+  t.in.x = 3;
+  t.in.p = &x;
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.x == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.p == &x); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedContainedConstMethodDoesNotInvalidateObjects() {
+  DOuter t;
+  t.x = 1;
+  t.in.x = 2;
+  t.in.foo();
+  clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.x == 2); // expected-warning{{TRUE}}
+}
+
+// --- PR21606 --- //
+
+struct s1 {
+    void g(const int *i) const;
+};
+
+struct s2 {
+    void f(int *i) {
+        m_i = i;
+        m_s.g(m_i);
+        if (m_i)
+            *i = 42; // no-warning
+    }
+
+    int *m_i;
+    s1 m_s;
+};
+
+void PR21606()
+{
+    s2().f(0);
+}
+
+// FIXME
+// When there is a circular reference to an object and a const method is called
+// the object is not invalidated because TK_PreserveContents has already been
+// set.
+struct Outer2;
+
+struct InnerWithRef {
+  Outer2 *ref;
+};
+
+struct Outer2 {
+  int x;
+  InnerWithRef in;
+  void foo() const;
+};
+
+void checkThatConstMethodCallDoesInvalidateObjectForCircularReferences() {
+  Outer2 t;
+  t.x = 1;
+  t.in.ref = &t;
+  t.foo();
+  // FIXME: Should be UNKNOWN.
+  clang_analyzer_eval(t.x); // expected-warning{{TRUE}}
+}