[cfe-commits] r162681 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/ExprEngineC.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp test/Analysis/base-init.cpp test/Analysis/method-call.cpp test/Analysis/new.cpp
Jordan Rose
jordan_rose at apple.com
Mon Aug 27 10:50:07 PDT 2012
Author: jrose
Date: Mon Aug 27 12:50:07 2012
New Revision: 162681
URL: http://llvm.org/viewvc/llvm-project?rev=162681&view=rev
Log:
[analyzer] Inline constructors for any object with a trivial destructor.
This allows us to better reason about status objects, like Clang's own
llvm::Optional (when its contents are trivially destructible), which are
often intended to be passed around by value.
We still don't inline constructors for temporaries in the general case.
<rdar://problem/11986434>
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
cfe/trunk/test/Analysis/base-init.cpp
cfe/trunk/test/Analysis/method-call.cpp
cfe/trunk/test/Analysis/new.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp?rev=162681&r1=162680&r2=162681&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp Mon Aug 27 12:50:07 2012
@@ -118,8 +118,9 @@
const Expr *RetE = RS->getRetValue();
if (!RetE)
return;
-
- SVal V = C.getState()->getSVal(RetE, C.getLocationContext());
+
+ const LocationContext *LCtx = C.getLocationContext();
+ SVal V = C.getState()->getSVal(RetE, LCtx);
const MemRegion *R = V.getAsRegion();
if (!R)
@@ -132,8 +133,9 @@
return;
// Return stack memory in an ancestor stack frame is fine.
- const StackFrameContext *SFC = SS->getStackFrame();
- if (SFC != C.getLocationContext()->getCurrentStackFrame())
+ const StackFrameContext *CurFrame = LCtx->getCurrentStackFrame();
+ const StackFrameContext *MemFrame = SS->getStackFrame();
+ if (MemFrame != CurFrame)
return;
// Automatic reference counting automatically copies blocks.
@@ -141,6 +143,11 @@
isa<BlockDataRegion>(R))
return;
+ // Returning a record by value is fine. (In this case, the returned
+ // expression will be a copy-constructor.)
+ if (isa<CXXConstructExpr>(RetE) && RetE->getType()->isRecordType())
+ return;
+
EmitStackError(C, R, RetE);
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=162681&r1=162680&r2=162681&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Mon Aug 27 12:50:07 2012
@@ -839,12 +839,8 @@
case Expr::MaterializeTemporaryExprClass: {
Bldr.takeNodes(Pred);
- const MaterializeTemporaryExpr *Materialize
- = cast<MaterializeTemporaryExpr>(S);
- if (Materialize->getType()->isRecordType())
- Dst.Add(Pred);
- else
- CreateCXXTemporaryObject(Materialize, Pred, Dst);
+ const MaterializeTemporaryExpr *MTE = cast<MaterializeTemporaryExpr>(S);
+ CreateCXXTemporaryObject(MTE, Pred, Dst);
Bldr.addNodes(Dst);
break;
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp?rev=162681&r1=162680&r2=162681&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp Mon Aug 27 12:50:07 2012
@@ -264,6 +264,7 @@
case CK_NonAtomicToAtomic:
// True no-ops.
case CK_NoOp:
+ case CK_ConstructorConversion:
case CK_UserDefinedConversion:
case CK_FunctionToPointerDecay: {
// Copy the SVal of Ex to CastE.
@@ -375,7 +376,6 @@
case CK_BaseToDerivedMemberPointer:
case CK_DerivedToBaseMemberPointer:
case CK_ReinterpretMemberPointer:
- case CK_ConstructorConversion:
case CK_VectorSplat:
case CK_LValueBitCast: {
// Recover some path-sensitivty by conjuring a new value.
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=162681&r1=162680&r2=162681&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Mon Aug 27 12:50:07 2012
@@ -32,13 +32,20 @@
// Bind the temporary object to the value of the expression. Then bind
// the expression to the location of the object.
- SVal V = state->getSVal(tempExpr, Pred->getLocationContext());
+ SVal V = state->getSVal(tempExpr, LCtx);
- const MemRegion *R =
- svalBuilder.getRegionManager().getCXXTempObjectRegion(ME, 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()) {
+ const MemRegion *R =
+ svalBuilder.getRegionManager().getCXXTempObjectRegion(ME, LCtx);
+
+ SVal L = loc::MemRegionVal(R);
+ state = state->bindLoc(L, V);
+ V = L;
+ }
- state = state->bindLoc(loc::MemRegionVal(R), V);
- Bldr.generateNode(ME, Pred, state->BindExpr(ME, LCtx, loc::MemRegionVal(R)));
+ Bldr.generateNode(ME, Pred, state->BindExpr(ME, LCtx, V));
}
void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
@@ -101,8 +108,12 @@
// FIXME: This will eventually need to handle new-expressions as well.
}
- // If we couldn't find an existing region to construct into, we'll just
- // generate a symbolic region, which is fine.
+ // If we couldn't find an existing region to construct into, assume we're
+ // constructing a temporary.
+ if (!Target) {
+ MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
+ Target = MRMgr.getCXXTempObjectRegion(CE, LCtx);
+ }
break;
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=162681&r1=162680&r2=162681&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Mon Aug 27 12:50:07 2012
@@ -340,13 +340,6 @@
if (!shouldInlineCXX(getAnalysisManager()))
return false;
- // Only inline constructors and destructors if we built the CFGs for them
- // properly.
- const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext();
- if (!ADC->getCFGBuildOptions().AddImplicitDtors ||
- !ADC->getCFGBuildOptions().AddInitializers)
- return false;
-
const CXXConstructorCall &Ctor = cast<CXXConstructorCall>(Call);
// FIXME: We don't handle constructors or destructors for arrays properly.
@@ -354,6 +347,17 @@
if (Target && isa<ElementRegion>(Target))
return false;
+ // If the destructor is trivial, it's always safe to inline the constructor.
+ if (Ctor.getDecl()->getParent()->hasTrivialDestructor())
+ break;
+
+ // For other types, only inline constructors if we built the CFGs for the
+ // destructor properly.
+ const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext();
+ assert(ADC->getCFGBuildOptions().AddInitializers && "No CFG initializers");
+ if (!ADC->getCFGBuildOptions().AddImplicitDtors)
+ return false;
+
// FIXME: This is a hack. We don't handle temporary destructors
// right now, so we shouldn't inline their constructors.
const CXXConstructExpr *CtorExpr = Ctor.getOriginExpr();
@@ -370,8 +374,7 @@
// Only inline constructors and destructors if we built the CFGs for them
// properly.
const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext();
- if (!ADC->getCFGBuildOptions().AddImplicitDtors ||
- !ADC->getCFGBuildOptions().AddInitializers)
+ if (!ADC->getCFGBuildOptions().AddImplicitDtors)
return false;
const CXXDestructorCall &Dtor = cast<CXXDestructorCall>(Call);
Modified: cfe/trunk/test/Analysis/base-init.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/base-init.cpp?rev=162681&r1=162680&r2=162681&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/base-init.cpp (original)
+++ cfe/trunk/test/Analysis/base-init.cpp Mon Aug 27 12:50:07 2012
@@ -1,5 +1,4 @@
// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-store region -analyzer-ipa=inlining -verify %s
-// XFAIL: *
void clang_analyzer_eval(bool);
Modified: cfe/trunk/test/Analysis/method-call.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/method-call.cpp?rev=162681&r1=162680&r2=162681&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/method-call.cpp (original)
+++ cfe/trunk/test/Analysis/method-call.cpp Mon Aug 27 12:50:07 2012
@@ -15,23 +15,22 @@
clang_analyzer_eval(a); // expected-warning{{TRUE}}
}
-
-// FIXME: These require constructor inlining to be enabled.
-
void f1() {
A x(3);
- // should be TRUE
- clang_analyzer_eval(x.getx() == 3); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(x.getx() == 3); // expected-warning{{TRUE}}
}
void f2() {
const A &x = A(3);
- // should be TRUE
- clang_analyzer_eval(x.getx() == 3); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(x.getx() == 3); // expected-warning{{TRUE}}
}
void f3() {
const A &x = (A)3;
- // should be TRUE
- clang_analyzer_eval(x.getx() == 3); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(x.getx() == 3); // expected-warning{{TRUE}}
+}
+
+void f4() {
+ A x = 3;
+ clang_analyzer_eval(x.getx() == 3); // expected-warning{{TRUE}}
}
Modified: cfe/trunk/test/Analysis/new.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/new.cpp?rev=162681&r1=162680&r2=162681&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/new.cpp (original)
+++ cfe/trunk/test/Analysis/new.cpp Mon Aug 27 12:50:07 2012
@@ -74,6 +74,18 @@
}
+struct PtrWrapper {
+ int *x;
+
+ PtrWrapper(int *input) : x(input) {}
+};
+
+PtrWrapper *testNewInvalidation() {
+ // Ensure that we don't consider this a leak.
+ return new PtrWrapper(static_cast<int *>(malloc(4)));
+}
+
+
//--------------------------------
// Incorrectly-modelled behavior
//--------------------------------
More information about the cfe-commits
mailing list