seaneveson created this revision.
seaneveson added a subscriber: cfe-commits.
Dear All,
I would like to propose a patch that prevents the invalidation of ‘this’ when a
method is const; fixing the test case given below, taken from PR 21606
(https://llvm.org/bugs/show_bug.cgi?id=21606).
```
struct s1 {
void g(const int *i) const;
};
struct s2 {
void f(int *i) {
m_i = i;
m_s.g(m_i); // Diagnostic goes away if you remove this line.
if (m_i)
*i = 42;
}
int *m_i;
s1 m_s;
};
int main() {
s2().f(0);
}
```
Mutable members of the object are a special case and are still invalidated; if
a mutable member is invalidated the entire object will be invalidated because
invalidateRegions invalidates the base region.
Whilst the patch fixes the test case from PR 21606, the same false-positive
occurs when the method ‘s1::g’ isn’t const; i.e. when ‘s2::f’ is called,
subsequently calling ‘s1::g’, the memory region for the instance of s1 is
(correctly) invalidated. However, the containing memory region (the instance
of s2) is also invalidated, which I think is overly conservative.
Why is the base region (in this case: S2) invalidated? Would it be acceptable
to change invalidation to modify the given region and not the base region when
# invalidating only the mutable members for a const method call?
# invalidating an object as a result of conservative ‘method call’
evaluations?
Regards,
Sean Eveson
SN Systems - Sony Computer Entertainment Group
http://reviews.llvm.org/D13099
Files:
lib/StaticAnalyzer/Core/CallEvent.cpp
test/Analysis/PR21606.cpp
test/Analysis/method-call.cpp
Index: test/Analysis/method-call.cpp
===================================================================
--- test/Analysis/method-call.cpp
+++ test/Analysis/method-call.cpp
@@ -13,6 +13,17 @@
int x;
};
+struct C {
+ int x;
+ void foo() const;
+ void bar();
+};
+
+struct D {
+ mutable int x;
+ void foo() const;
+};
+
void testNullObject(A *a) {
clang_analyzer_eval(a); // expected-warning{{UNKNOWN}}
(void)a->getx(); // assume we know what we're doing
@@ -45,3 +56,21 @@
B t2(t);
clang_analyzer_eval(t.x == 0); // expected-warning{{TRUE}}
}
+
+void checkThatConstMethodWithoutDefinitionDoesNotInvalidateObject() {
+ C 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() {
+ D t;
+ t.x = 3;
+ t.foo();
+ clang_analyzer_eval(t.x); // expected-warning{{UNKNOWN}}
+}
+
Index: test/Analysis/PR21606.cpp
===================================================================
--- test/Analysis/PR21606.cpp
+++ test/Analysis/PR21606.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core
+// 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;
+};
+
+int main()
+{
+ s2().f(0);
+}
\ No newline at end of file
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===================================================================
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -404,6 +404,24 @@
}
void CXXInstanceCall::getExtraInvalidatedValues(ValueList &Values) const {
+ // Check if this is a call to a const method.
+ if (const CXXMethodDecl *D = cast_or_null<CXXMethodDecl>(getDecl())) {
+ if(D->getCanonicalDecl()->isConst()) {
+ // Check if the containing class/struct has mutable members
+ const MemRegion *ThisRegion = getCXXThisVal().getAsRegion();
+ if (ThisRegion) {
+ MemRegionManager *MemMgr = ThisRegion->getMemRegionManager();
+ const CXXRecordDecl *Parent = D->getParent();
+ for (const auto *I : Parent->fields()) {
+ if (I->isMutable()) {
+ const FieldRegion *FR = MemMgr->getFieldRegion(I, ThisRegion);
+ Values.push_back(loc::MemRegionVal(FR));
+ }
+ }
+ }
+ return;
+ }
+ }
Values.push_back(getCXXThisVal());
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
