[cfe-commits] r163220 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h lib/StaticAnalyzer/Core/CallEvent.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp test/Analysis/array-struct-region.cpp test/Analysis/reference.cpp
Jordan Rose
jordan_rose at apple.com
Wed Sep 5 10:11:26 PDT 2012
Author: jrose
Date: Wed Sep 5 12:11:26 2012
New Revision: 163220
URL: http://llvm.org/viewvc/llvm-project?rev=163220&view=rev
Log:
[analyzer] Be more forgiving about calling methods on struct rvalues.
The problem is that the value of 'this' in a C++ member function call
should always be a region (or NULL). However, if the object is an rvalue,
it has no associated region (only a conjured symbol or LazyCompoundVal).
For now, we handle this in two ways:
1) Actually respect MaterializeTemporaryExpr. Before, it was relying on
CXXConstructExpr to create temporary regions for all struct values.
Now it just does the right thing: if the value is not in a temporary
region, create one.
2) Have CallEvent recognize the case where its 'this' pointer is a
non-region, and just return UnknownVal to keep from confusing clients.
The long-term problem is being tracked internally in <rdar://problem/12137950>,
but this makes many test cases pass.
Added:
cfe/trunk/test/Analysis/array-struct-region.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
cfe/trunk/test/Analysis/reference.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h?rev=163220&r1=163219&r2=163220&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h Wed Sep 5 12:11:26 2012
@@ -504,13 +504,7 @@
virtual const Expr *getCXXThisExpr() const { return 0; }
/// \brief Returns the value of the implicit 'this' object.
- virtual SVal getCXXThisVal() const {
- const Expr *Base = getCXXThisExpr();
- // FIXME: This doesn't handle an overloaded ->* operator.
- if (!Base)
- return UnknownVal();
- return getSVal(Base);
- }
+ virtual SVal getCXXThisVal() const;
virtual const FunctionDecl *getDecl() const;
Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=163220&r1=163219&r2=163220&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Wed Sep 5 12:11:26 2012
@@ -384,6 +384,25 @@
Regions.push_back(R);
}
+SVal CXXInstanceCall::getCXXThisVal() const {
+ const Expr *Base = getCXXThisExpr();
+ // FIXME: This doesn't handle an overloaded ->* operator.
+ if (!Base)
+ return UnknownVal();
+
+ SVal ThisVal = getSVal(Base);
+
+ // FIXME: This is only necessary because we can call member functions on
+ // struct rvalues, which do not have regions we can use for a 'this' pointer.
+ // Ideally this should eventually be changed to an assert, i.e. all
+ // non-Unknown, non-null 'this' values should be loc::MemRegionVals.
+ if (isa<DefinedSVal>(ThisVal))
+ if (!ThisVal.getAsRegion() && !ThisVal.isConstant())
+ return UnknownVal();
+
+ return ThisVal;
+}
+
RuntimeDefinition CXXInstanceCall::getRuntimeDefinition() const {
// Do we have a decl at all?
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=163220&r1=163219&r2=163220&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Wed Sep 5 12:11:26 2012
@@ -34,9 +34,10 @@
// the expression to the location of the object.
SVal V = state->getSVal(tempExpr, LCtx);
- // If the object is a record, the constructor will have already created
- // a temporary object region. If it is not, we need to copy the value over.
- if (!ME->getType()->isRecordType()) {
+ // If the value is already a CXXTempObjectRegion, it is fine as it is.
+ // Otherwise, create a new CXXTempObjectRegion, and copy the value into it.
+ const MemRegion *MR = V.getAsRegion();
+ if (!MR || !isa<CXXTempObjectRegion>(MR)) {
const MemRegion *R =
svalBuilder.getRegionManager().getCXXTempObjectRegion(ME, LCtx);
Added: cfe/trunk/test/Analysis/array-struct-region.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/array-struct-region.cpp?rev=163220&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/array-struct-region.cpp (added)
+++ cfe/trunk/test/Analysis/array-struct-region.cpp Wed Sep 5 12:11:26 2012
@@ -0,0 +1,87 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ -analyzer-config c++-inlining=constructors %s
+
+void clang_analyzer_eval(int);
+
+struct S {
+ int field;
+
+#if __cplusplus
+ const struct S *getThis() const { return this; }
+#endif
+};
+
+struct S getS();
+
+
+void testAssignment() {
+ struct S s = getS();
+
+ if (s.field != 42) return;
+ clang_analyzer_eval(s.field == 42); // expected-warning{{TRUE}}
+
+ s.field = 0;
+ clang_analyzer_eval(s.field == 0); // expected-warning{{TRUE}}
+
+#if __cplusplus
+ clang_analyzer_eval(s.getThis() == &s); // expected-warning{{TRUE}}
+#endif
+}
+
+
+void testImmediateUse() {
+ int x = getS().field;
+
+ if (x != 42) return;
+ clang_analyzer_eval(x == 42); // expected-warning{{TRUE}}
+
+#if __cplusplus
+ clang_analyzer_eval((void *)getS().getThis() == (void *)&x); // expected-warning{{FALSE}}
+#endif
+}
+
+int getConstrainedField(struct S s) {
+ if (s.field != 42) return 42;
+ return s.field;
+}
+
+int getAssignedField(struct S s) {
+ s.field = 42;
+ return s.field;
+}
+
+void testArgument() {
+ clang_analyzer_eval(getConstrainedField(getS()) == 42); // expected-warning{{TRUE}}
+ clang_analyzer_eval(getAssignedField(getS()) == 42); // expected-warning{{TRUE}}
+}
+
+
+//--------------------
+// C++-only tests
+//--------------------
+
+#if __cplusplus
+void testReferenceAssignment() {
+ const S &s = getS();
+
+ if (s.field != 42) return;
+ clang_analyzer_eval(s.field == 42); // expected-warning{{TRUE}}
+
+ clang_analyzer_eval(s.getThis() == &s); // expected-warning{{TRUE}}
+}
+
+
+int getConstrainedFieldRef(const S &s) {
+ if (s.field != 42) return 42;
+ return s.field;
+}
+
+bool checkThis(const S &s) {
+ return s.getThis() == &s;
+}
+
+void testReferenceArgument() {
+ clang_analyzer_eval(getConstrainedFieldRef(getS()) == 42); // expected-warning{{TRUE}}
+ clang_analyzer_eval(checkThis(getS())); // expected-warning{{TRUE}}
+}
+#endif
Modified: cfe/trunk/test/Analysis/reference.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/reference.cpp?rev=163220&r1=163219&r2=163220&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/reference.cpp (original)
+++ cfe/trunk/test/Analysis/reference.cpp Wed Sep 5 12:11:26 2012
@@ -116,8 +116,13 @@
struct S { int &x; };
- extern S *getS();
- clang_analyzer_eval(&getS()->x != 0); // expected-warning{{TRUE}}
+ // FIXME: Should be TRUE. Fields of return-by-value structs are not yet
+ // symbolicated. Tracked by <rdar://problem/12137950>.
+ extern S getS();
+ clang_analyzer_eval(&getS().x != 0); // expected-warning{{UNKNOWN}}
+
+ extern S *getSP();
+ clang_analyzer_eval(&getSP()->x != 0); // expected-warning{{TRUE}}
}
@@ -150,10 +155,3 @@
return *x; // should warn here!
}
}
-
-void testReferenceFieldAddress() {
- struct S { int &x; };
-
- extern S getS();
- clang_analyzer_eval(&getS().x != 0); // expected-warning{{UNKNOWN}}
-}
More information about the cfe-commits
mailing list