[PATCH] D48460: [analyzer] Fix invalidation on C++ const methods.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.

`const` methods shouldn't invalidate the object unless mutable fields kick in.

They sometimes were invalidating the object when we accidentally failed to retrieve the record type to see if there are mutable fields in it.

We failed to retrieve it because this-expression is sometimes of pointer type and sometimes of object type, and we only handled the latter.


Repository:
  rC Clang

https://reviews.llvm.org/D48460

Files:
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/const-method-call.cpp


Index: test/Analysis/const-method-call.cpp
===================================================================
--- test/Analysis/const-method-call.cpp
+++ test/Analysis/const-method-call.cpp
@@ -6,6 +6,14 @@
   int x;
   void foo() const;
   void bar();
+
+  void testImplicitThisSyntax() {
+    x = 3;
+    foo();
+    clang_analyzer_eval(x == 3); // expected-warning{{TRUE}}
+    bar();
+    clang_analyzer_eval(x == 3); // expected-warning{{UNKNOWN}}
+  }
 };
 
 struct B {
@@ -108,6 +116,22 @@
   clang_analyzer_eval(t.in.x == 2); // expected-warning{{TRUE}}
 }
 
+void checkPointerTypedThisExpression(A *a) {
+  a->x = 3;
+  a->foo();
+  clang_analyzer_eval(a->x == 3); // expected-warning{{TRUE}}
+  a->bar();
+  clang_analyzer_eval(a->x == 3); // expected-warning{{UNKNOWN}}
+}
+
+void checkReferenceTypedThisExpression(A &a) {
+  a.x = 3;
+  a.foo();
+  clang_analyzer_eval(a.x == 3); // expected-warning{{TRUE}}
+  a.bar();
+  clang_analyzer_eval(a.x == 3); // expected-warning{{UNKNOWN}}
+}
+
 // --- Versions of the above tests where the const method is inherited --- //
 
 struct B1 {
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -535,8 +535,12 @@
     // 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())
+    QualType T = Ex->getType();
+    if (T->isPointerType()) // Arrow or implicit-this syntax?
+      T = T->getPointeeType();
+    const CXXRecordDecl *ParentRecord = T->getAsCXXRecordDecl();
+    assert(ParentRecord);
+    if (ParentRecord->hasMutableFields())
       return;
     // Preserve CXXThis.
     const MemRegion *ThisRegion = ThisVal.getAsRegion();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D48460.152391.patch
Type: text/x-patch
Size: 1997 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180621/69c4af53/attachment-0001.bin>