[analyzer] Fix invalidation on C++ const methods.

[NoQ]

Summary

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.

[baloghadamsoftware]

I find it a strange behavior that this-expression is sometimes a pointer, sometimes a record declaration. If references are resolved, why are pointers not?

This is an important fix, but I wonder how many other places are in the code where we do not handle this-expressions by pointers.

[NoQ]

Well, i guess that's just one of those mildly infuriating aspects of the AST and/or the standard :)

If references are resolved, why are pointers not?

It's kinda understandable. References are simple aliases to glvalues. When you use a reference, you simply get *the* original glvalue, which is exactly what the AST gives you: a glvalue expression of object type. Here's what Expr::setType() says:

130   void setType(QualType t) {
131     // In C++, the type of an expression is always adjusted so that it
132     // will not have reference type (C++ [expr]p6). Use
133     // QualType::getNonReferenceType() to retrieve the non-reference
134     // type. Additionally, inspect Expr::isLvalue to determine whether
135     // an expression that is adjusted in this manner should be
136     // considered an lvalue.
137     assert((t.isNull() || !t->isReferenceType()) &&
138            "Expressions can't have reference type");
139
140     TR = t;
141   }

---

This code is still imperfect because it doesn't take dynamic type of the region into account, which may be more specific than the expression's type. I guess i'll add a FIXME.

[xazax.hun]

LGTM!

lib/StaticAnalyzer/Core/CallEvent.cpp

void CXXInstanceCall::getExtraInvalidatedValues(
 
   // Don't invalidate if the method is const and there are no mutable fields.
   if (const auto *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.
+    // TODO: We might as well look at the dynamic type of the object.
     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();
     if (!ThisRegion)
       return;
 
     ETraits->setTrait(ThisRegion->getBaseRegion(),
                       RegionAndSymbolInvalidationTraits::TK_PreserveContents);

test/Analysis/const-method-call.cpp

 // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
 
 void clang_analyzer_eval(bool);
 
 struct A {
   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 {
   mutable int mut;
   void foo() const;
 };
 
 struct C {
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}}
 }
 
+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 {
   void foo() const;
 };
 
 struct D1 : public B1 {
   int x;