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