[cfe-commits] r158784 - in /cfe/trunk: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp test/Analysis/new-fail.cpp test/Analysis/new.cpp
Jordan Rose
jordan_rose at apple.com
Tue Jun 19 18:32:01 PDT 2012
Author: jrose
Date: Tue Jun 19 20:32:01 2012
New Revision: 158784
URL: http://llvm.org/viewvc/llvm-project?rev=158784&view=rev
Log:
[analyzer] Invalidate placement args; return the pointer given to placement new
The default global placement new just returns the pointer it is given.
Note that other custom 'new' implementations with placement args are not
guaranteed to do this.
In addition, we need to invalidate placement args, since they may be updated by
the allocator function. (Also, right now we don't properly handle the
constructor inside a CXXNewExpr, so we need to invalidate the placement args
just so that callers know something changed!)
This invalidation is not perfect because CallOrObjCMessage doesn't support
CXXNewExpr, and all of our invalidation callbacks expect that if there's no
CallOrObjCMessage, the invalidation is happening manually (e.g. by a direct
assignment) and shouldn't affect checker-specific metadata (like malloc state);
hence the malloc test case in new-fail.cpp. But region values are now
properly invalidated, at least.
The long-term solution to this problem is to rework CallOrObjCMessage into
something more general, rather than the morass of branches it is today.
<rdar://problem/11679031>
Added:
cfe/trunk/test/Analysis/new-fail.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
cfe/trunk/test/Analysis/new.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=158784&r1=158783&r2=158784&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Tue Jun 19 20:32:01 2012
@@ -170,6 +170,16 @@
Bldr.generateNode(PP, Pred, state);
}
+static bool isPointerToConst(const ParmVarDecl *ParamDecl) {
+ // FIXME: Copied from ExprEngineCallAndReturn.cpp
+ QualType PointeeTy = ParamDecl->getOriginalType()->getPointeeType();
+ if (PointeeTy != QualType() && PointeeTy.isConstQualified() &&
+ !PointeeTy->isAnyPointerType() && !PointeeTy->isReferenceType()) {
+ return true;
+ }
+ return false;
+}
+
void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, ExplodedNode *Pred,
ExplodedNodeSet &Dst) {
StmtNodeBuilder Bldr(Pred, Dst, *currentBuilderContext);
@@ -182,18 +192,108 @@
QualType ObjTy = CNE->getType()->getAs<PointerType>()->getPointeeType();
const ElementRegion *EleReg =
getStoreManager().GetElementZeroRegion(NewReg, ObjTy);
+ ProgramStateRef State = Pred->getState();
if (CNE->isArray()) {
// FIXME: allocating an array requires simulating the constructors.
// For now, just return a symbolicated region.
- ProgramStateRef state = Pred->getState();
- state = state->BindExpr(CNE, Pred->getLocationContext(),
+ State = State->BindExpr(CNE, Pred->getLocationContext(),
loc::MemRegionVal(EleReg));
- Bldr.generateNode(CNE, Pred, state);
+ Bldr.generateNode(CNE, Pred, State);
return;
}
- // FIXME: Update for AST changes.
+ FunctionDecl *FD = CNE->getOperatorNew();
+ if (FD && FD->isReservedGlobalPlacementOperator()) {
+ // Non-array placement new should always return the placement location.
+ SVal PlacementLoc = State->getSVal(CNE->getPlacementArg(0), LCtx);
+ State = State->BindExpr(CNE, LCtx, PlacementLoc);
+ // FIXME: Once we have proper support for CXXConstructExprs inside
+ // CXXNewExpr, we need to make sure that the constructed object is not
+ // immediately invalidated here. (The placement call should happen before
+ // the constructor call anyway.)
+ }
+
+ // Invalidate placement args.
+
+ // FIXME: This is largely copied from invalidateArguments, because
+ // CallOrObjCMessage is not general enough to handle new-expressions yet.
+ SmallVector<const MemRegion *, 4> RegionsToInvalidate;
+
+ unsigned Index = 0;
+ for (CXXNewExpr::const_arg_iterator I = CNE->placement_arg_begin(),
+ E = CNE->placement_arg_end();
+ I != E; ++I) {
+ // Pre-increment the argument index to skip over the implicit size arg.
+ ++Index;
+ if (FD && Index < FD->getNumParams())
+ if (isPointerToConst(FD->getParamDecl(Index)))
+ continue;
+
+ SVal V = State->getSVal(*I, LCtx);
+
+ // If we are passing a location wrapped as an integer, unwrap it and
+ // invalidate the values referred by the location.
+ if (nonloc::LocAsInteger *Wrapped = dyn_cast<nonloc::LocAsInteger>(&V))
+ V = Wrapped->getLoc();
+ else if (!isa<Loc>(V))
+ continue;
+
+ if (const MemRegion *R = V.getAsRegion()) {
+ // Invalidate the value of the variable passed by reference.
+
+ // Are we dealing with an ElementRegion? If the element type is
+ // a basic integer type (e.g., char, int) and the underlying region
+ // is a variable region then strip off the ElementRegion.
+ // FIXME: We really need to think about this for the general case
+ // as sometimes we are reasoning about arrays and other times
+ // about (char*), etc., is just a form of passing raw bytes.
+ // e.g., void *p = alloca(); foo((char*)p);
+ if (const ElementRegion *ER = dyn_cast<ElementRegion>(R)) {
+ // Checking for 'integral type' is probably too promiscuous, but
+ // we'll leave it in for now until we have a systematic way of
+ // handling all of these cases. Eventually we need to come up
+ // with an interface to StoreManager so that this logic can be
+ // appropriately delegated to the respective StoreManagers while
+ // still allowing us to do checker-specific logic (e.g.,
+ // invalidating reference counts), probably via callbacks.
+ if (ER->getElementType()->isIntegralOrEnumerationType()) {
+ const MemRegion *superReg = ER->getSuperRegion();
+ if (isa<VarRegion>(superReg) || isa<FieldRegion>(superReg) ||
+ isa<ObjCIvarRegion>(superReg))
+ R = cast<TypedRegion>(superReg);
+ }
+ // FIXME: What about layers of ElementRegions?
+ }
+
+ // Mark this region for invalidation. We batch invalidate regions
+ // below for efficiency.
+ RegionsToInvalidate.push_back(R);
+ } else {
+ // Nuke all other arguments passed by reference.
+ // FIXME: is this necessary or correct? This handles the non-Region
+ // cases. Is it ever valid to store to these?
+ State = State->unbindLoc(cast<Loc>(V));
+ }
+ }
+
+ // Invalidate designated regions using the batch invalidation API.
+
+ // FIXME: We can have collisions on the conjured symbol if the
+ // expression *I also creates conjured symbols. We probably want
+ // to identify conjured symbols by an expression pair: the enclosing
+ // expression (the context) and the expression itself. This should
+ // disambiguate conjured symbols.
+ unsigned Count = currentBuilderContext->getCurrentBlockCount();
+
+ // NOTE: Even if RegionsToInvalidate is empty, we may still invalidate
+ // global variables.
+ State = State->invalidateRegions(RegionsToInvalidate, CNE, Count, LCtx);
+ Bldr.generateNode(CNE, Pred, State);
+ return;
+
+ // FIXME: The below code is long-since dead. However, constructor handling
+ // in new-expressions is far from complete. See PR12014 for more details.
#if 0
// Evaluate constructor arguments.
const FunctionProtoType *FnType = NULL;
Added: cfe/trunk/test/Analysis/new-fail.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/new-fail.cpp?rev=158784&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/new-fail.cpp (added)
+++ cfe/trunk/test/Analysis/new-fail.cpp Tue Jun 19 20:32:01 2012
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc -analyzer-store region -verify %s
+// XFAIL: *
+
+void f1() {
+ int *n = new int;
+ if (*n) { // expected-warning {{Branch condition evaluates to a garbage value}}
+ }
+}
+
+void f2() {
+ int *n = new int(3);
+ if (*n) { // no-warning
+ }
+}
+
+void *operator new(size_t, void *, void *);
+void *testCustomNew() {
+ int *x = (int *)malloc(sizeof(int));
+ void *y = new (0, x) int;
+ return y; // no-warning (placement new could have freed x)
+}
Modified: cfe/trunk/test/Analysis/new.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/new.cpp?rev=158784&r1=158783&r2=158784&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/new.cpp (original)
+++ cfe/trunk/test/Analysis/new.cpp Tue Jun 19 20:32:01 2012
@@ -1,15 +1,36 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-store region -verify %s
-// XFAIL: *
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store region -verify %s
-void f1() {
- int *n = new int;
- if (*n) { // expected-warning {{Branch condition evaluates to a garbage value}}
- }
+void clang_analyzer_eval(bool);
+
+typedef typeof(sizeof(int)) size_t;
+extern "C" void *malloc(size_t);
+
+// This is the standard placement new.
+inline void* operator new(size_t, void* __p) throw()
+{
+ return __p;
+}
+
+void *testPlacementNew() {
+ int *x = (int *)malloc(sizeof(int));
+ *x = 1;
+ clang_analyzer_eval(*x == 1); // expected-warning{{TRUE}};
+
+ void *y = new (x) int;
+ clang_analyzer_eval(x == y); // expected-warning{{TRUE}};
+ clang_analyzer_eval(*x == 1); // expected-warning{{UNKNOWN}};
+
+ return y;
}
-void f2() {
- int *n = new int(3);
- if (*n) { // no-warning
- }
+void *operator new(size_t, size_t, int *);
+void *testCustomNew() {
+ int x[1] = {1};
+ clang_analyzer_eval(*x == 1); // expected-warning{{TRUE}};
+
+ void *y = new (0, x) int;
+ clang_analyzer_eval(*x == 1); // expected-warning{{UNKNOWN}};
+
+ return y; // no-warning
}
More information about the cfe-commits
mailing list