r178602 - [analyzer] Better model for copying of array fields in implicit copy ctors.
Jordan Rose
jordan_rose at apple.com
Tue Apr 2 18:39:08 PDT 2013
Author: jrose
Date: Tue Apr 2 20:39:08 2013
New Revision: 178602
URL: http://llvm.org/viewvc/llvm-project?rev=178602&view=rev
Log:
[analyzer] Better model for copying of array fields in implicit copy ctors.
- Find the correct region to represent the first array element when
constructing a CXXConstructorCall.
- If the array is trivial, model the copy with a primitive load/store.
- Don't warn about the "uninitialized" subscript in the AST -- we don't use
the helper variable that Sema provides.
<rdar://problem/13091608>
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
cfe/trunk/test/Analysis/ctor-inlining.mm
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp?rev=178602&r1=178601&r2=178602&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp Tue Apr 2 20:39:08 2013
@@ -34,18 +34,28 @@ public:
void
UndefinedArraySubscriptChecker::checkPreStmt(const ArraySubscriptExpr *A,
CheckerContext &C) const {
- if (C.getState()->getSVal(A->getIdx(), C.getLocationContext()).isUndef()) {
- if (ExplodedNode *N = C.generateSink()) {
- if (!BT)
- BT.reset(new BuiltinBug("Array subscript is undefined"));
+ const Expr *Index = A->getIdx();
+ if (!C.getSVal(Index).isUndef())
+ return;
- // Generate a report for this bug.
- BugReport *R = new BugReport(*BT, BT->getName(), N);
- R->addRange(A->getIdx()->getSourceRange());
- bugreporter::trackNullOrUndefValue(N, A->getIdx(), *R);
- C.emitReport(R);
- }
- }
+ // Sema generates anonymous array variables for copying array struct fields.
+ // Don't warn if we're in an implicitly-generated constructor.
+ const Decl *D = C.getLocationContext()->getDecl();
+ if (const CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(D))
+ if (Ctor->isImplicitlyDefined())
+ return;
+
+ ExplodedNode *N = C.generateSink();
+ if (!N)
+ return;
+ if (!BT)
+ BT.reset(new BuiltinBug("Array subscript is undefined"));
+
+ // Generate a report for this bug.
+ BugReport *R = new BugReport(*BT, BT->getName(), N);
+ R->addRange(A->getIdx()->getSourceRange());
+ bugreporter::trackNullOrUndefValue(N, A->getIdx(), *R);
+ C.emitReport(R);
}
void ento::registerUndefinedArraySubscriptChecker(CheckerManager &mgr) {
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=178602&r1=178601&r2=178602&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Tue Apr 2 20:39:08 2013
@@ -432,14 +432,41 @@ void ExprEngine::ProcessInitializer(cons
// but non-objects must be copied in from the initializer.
const Expr *Init = BMI->getInit()->IgnoreImplicit();
if (!isa<CXXConstructExpr>(Init)) {
+ const ValueDecl *Field;
SVal FieldLoc;
- if (BMI->isIndirectMemberInitializer())
+ if (BMI->isIndirectMemberInitializer()) {
+ Field = BMI->getIndirectMember();
FieldLoc = State->getLValue(BMI->getIndirectMember(), thisVal);
- else
+ } else {
+ Field = BMI->getMember();
FieldLoc = State->getLValue(BMI->getMember(), thisVal);
+ }
- SVal InitVal = State->getSVal(BMI->getInit(), stackFrame);
+ SVal InitVal;
+ if (BMI->getNumArrayIndices() > 0) {
+ // Handle arrays of trivial type. We can represent this with a
+ // primitive load/copy from the base array region.
+ const ArraySubscriptExpr *ASE;
+ while ((ASE = dyn_cast<ArraySubscriptExpr>(Init)))
+ Init = ASE->getBase()->IgnoreImplicit();
+ SVal LValue = State->getSVal(Init, stackFrame);
+ if (Optional<Loc> LValueLoc = LValue.getAs<Loc>())
+ InitVal = State->getSVal(*LValueLoc);
+
+ // If we fail to get the value for some reason, use a symbolic value.
+ if (InitVal.isUnknownOrUndef()) {
+ SValBuilder &SVB = getSValBuilder();
+ InitVal = SVB.conjureSymbolVal(BMI->getInit(), stackFrame,
+ Field->getType(),
+ currBldrCtx->blockCount());
+ }
+ } else {
+ InitVal = State->getSVal(BMI->getInit(), stackFrame);
+ }
+
+ assert(Tmp.size() == 1 && "have not generated any new nodes yet");
+ assert(*Tmp.begin() == Pred && "have not generated any new nodes yet");
Tmp.clear();
evalBind(Tmp, Init, Pred, FieldLoc, InitVal, /*isInit=*/true, &PP);
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=178602&r1=178601&r2=178602&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Tue Apr 2 20:39:08 2013
@@ -96,6 +96,26 @@ void ExprEngine::performTrivialCopy(Node
}
}
+
+/// Returns a region representing the first element of a (possibly
+/// multi-dimensional) array.
+///
+/// On return, \p Ty will be set to the base type of the array.
+///
+/// If the type is not an array type at all, the original value is returned.
+static SVal makeZeroElementRegion(ProgramStateRef State, SVal LValue,
+ QualType &Ty) {
+ SValBuilder &SVB = State->getStateManager().getSValBuilder();
+ ASTContext &Ctx = SVB.getContext();
+
+ while (const ArrayType *AT = Ctx.getAsArrayType(Ty)) {
+ Ty = AT->getElementType();
+ LValue = State->getLValue(Ty, SVB.makeZeroArrayIndex(), LValue);
+ }
+
+ return LValue;
+}
+
void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
ExplodedNode *Pred,
ExplodedNodeSet &destNodes) {
@@ -103,7 +123,10 @@ void ExprEngine::VisitCXXConstructExpr(c
ProgramStateRef State = Pred->getState();
const MemRegion *Target = 0;
- bool IsArray = false;
+
+ // FIXME: Handle arrays, which run the same constructor for every element.
+ // For now, we just run the first constructor (which should still invalidate
+ // the entire array).
switch (CE->getConstructionKind()) {
case CXXConstructExpr::CK_Complete: {
@@ -118,19 +141,10 @@ void ExprEngine::VisitCXXConstructExpr(c
if (const DeclStmt *DS = dyn_cast<DeclStmt>(StmtElem->getStmt())) {
if (const VarDecl *Var = dyn_cast<VarDecl>(DS->getSingleDecl())) {
if (Var->getInit()->IgnoreImplicit() == CE) {
+ SVal LValue = State->getLValue(Var, LCtx);
QualType Ty = Var->getType();
- if (const ArrayType *AT = getContext().getAsArrayType(Ty)) {
- // FIXME: Handle arrays, which run the same constructor for
- // every element. This workaround will just run the first
- // constructor (which should still invalidate the entire array).
- SVal Base = State->getLValue(Var, LCtx);
- Target = State->getLValue(AT->getElementType(),
- getSValBuilder().makeZeroArrayIndex(),
- Base).getAsRegion();
- IsArray = true;
- } else {
- Target = State->getLValue(Var, LCtx).getAsRegion();
- }
+ LValue = makeZeroElementRegion(State, LValue, Ty);
+ Target = LValue.getAsRegion();
}
}
}
@@ -146,13 +160,19 @@ void ExprEngine::VisitCXXConstructExpr(c
LCtx->getCurrentStackFrame());
SVal ThisVal = State->getSVal(ThisPtr);
+ const ValueDecl *Field;
+ SVal FieldVal;
if (Init->isIndirectMemberInitializer()) {
- SVal Field = State->getLValue(Init->getIndirectMember(), ThisVal);
- Target = Field.getAsRegion();
+ Field = Init->getIndirectMember();
+ FieldVal = State->getLValue(Init->getIndirectMember(), ThisVal);
} else {
- SVal Field = State->getLValue(Init->getMember(), ThisVal);
- Target = Field.getAsRegion();
+ Field = Init->getMember();
+ FieldVal = State->getLValue(Init->getMember(), ThisVal);
}
+
+ QualType Ty = Field->getType();
+ FieldVal = makeZeroElementRegion(State, FieldVal, Ty);
+ Target = FieldVal.getAsRegion();
}
// FIXME: This will eventually need to handle new-expressions as well.
@@ -202,6 +222,7 @@ void ExprEngine::VisitCXXConstructExpr(c
ExplodedNodeSet DstEvaluated;
StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx);
+ bool IsArray = isa<ElementRegion>(Target);
if (CE->getConstructor()->isTrivial() &&
CE->getConstructor()->isCopyOrMoveConstructor() &&
!IsArray) {
@@ -234,12 +255,9 @@ void ExprEngine::VisitCXXDestructor(Qual
// FIXME: We need to run the same destructor on every element of the array.
// This workaround will just run the first destructor (which will still
// invalidate the entire array).
- // This is a loop because of multidimensional arrays.
- while (const ArrayType *AT = getContext().getAsArrayType(ObjectType)) {
- ObjectType = AT->getElementType();
- Dest = State->getLValue(ObjectType, getSValBuilder().makeZeroArrayIndex(),
- loc::MemRegionVal(Dest)).getAsRegion();
- }
+ SVal DestVal = loc::MemRegionVal(Dest);
+ DestVal = makeZeroElementRegion(State, DestVal, ObjectType);
+ Dest = DestVal.getAsRegion();
const CXXRecordDecl *RecordDecl = ObjectType->getAsCXXRecordDecl();
assert(RecordDecl && "Only CXXRecordDecls should have destructors");
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=178602&r1=178601&r2=178602&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Tue Apr 2 20:39:08 2013
@@ -605,6 +605,8 @@ static CallInlinePolicy mayInlineCallKin
const CXXConstructorCall &Ctor = cast<CXXConstructorCall>(Call);
// FIXME: We don't handle constructors or destructors for arrays properly.
+ // Even once we do, we still need to be careful about implicitly-generated
+ // initializers for array fields in default move/copy constructors.
const MemRegion *Target = Ctor.getCXXThisVal().getAsRegion();
if (Target && isa<ElementRegion>(Target))
return CIP_DisallowedOnce;
Modified: cfe/trunk/test/Analysis/ctor-inlining.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ctor-inlining.mm?rev=178602&r1=178601&r2=178602&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/ctor-inlining.mm (original)
+++ cfe/trunk/test/Analysis/ctor-inlining.mm Tue Apr 2 20:39:08 2013
@@ -307,3 +307,196 @@ namespace PODUninitialized {
}
}
}
+
+namespace ArrayMembers {
+ struct Primitive {
+ int values[3];
+ };
+
+ void testPrimitive() {
+ Primitive a = { { 1, 2, 3 } };
+
+ clang_analyzer_eval(a.values[0] == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.values[1] == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.values[2] == 3); // expected-warning{{TRUE}}
+
+ Primitive b = a;
+
+ clang_analyzer_eval(b.values[0] == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(b.values[1] == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(b.values[2] == 3); // expected-warning{{TRUE}}
+
+ Primitive c;
+ c = b;
+
+ clang_analyzer_eval(c.values[0] == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(c.values[1] == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(c.values[2] == 3); // expected-warning{{TRUE}}
+ }
+
+ struct NestedPrimitive {
+ int values[2][3];
+ };
+
+ void testNestedPrimitive() {
+ NestedPrimitive a = { { { 0, 0, 0 }, { 1, 2, 3 } } };
+
+ clang_analyzer_eval(a.values[1][0] == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.values[1][1] == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.values[1][2] == 3); // expected-warning{{TRUE}}
+
+ NestedPrimitive b = a;
+
+ clang_analyzer_eval(b.values[1][0] == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(b.values[1][1] == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(b.values[1][2] == 3); // expected-warning{{TRUE}}
+
+ NestedPrimitive c;
+ c = b;
+
+ clang_analyzer_eval(c.values[1][0] == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(c.values[1][1] == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(c.values[1][2] == 3); // expected-warning{{TRUE}}
+ }
+
+ struct POD {
+ IntWrapper values[3];
+ };
+
+ void testPOD() {
+ POD a = { { { 1 }, { 2 }, { 3 } } };
+
+ clang_analyzer_eval(a.values[0].x == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.values[1].x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.values[2].x == 3); // expected-warning{{TRUE}}
+
+ POD b = a;
+
+ clang_analyzer_eval(b.values[0].x == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(b.values[1].x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(b.values[2].x == 3); // expected-warning{{TRUE}}
+
+ POD c;
+ c = b;
+
+ clang_analyzer_eval(c.values[0].x == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(c.values[1].x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(c.values[2].x == 3); // expected-warning{{TRUE}}
+ }
+
+ struct NestedPOD {
+ IntWrapper values[2][3];
+ };
+
+ void testNestedPOD() {
+ NestedPOD a = { { { { 0 }, { 0 }, { 0 } }, { { 1 }, { 2 }, { 3 } } } };
+
+ clang_analyzer_eval(a.values[1][0].x == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.values[1][1].x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.values[1][2].x == 3); // expected-warning{{TRUE}}
+
+ NestedPOD b = a;
+
+ clang_analyzer_eval(b.values[1][0].x == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(b.values[1][1].x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(b.values[1][2].x == 3); // expected-warning{{TRUE}}
+
+ NestedPOD c;
+ c = b;
+
+ clang_analyzer_eval(c.values[1][0].x == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(c.values[1][1].x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(c.values[1][2].x == 3); // expected-warning{{TRUE}}
+ }
+
+ struct NonPOD {
+ NonPODIntWrapper values[3];
+ };
+
+ void testNonPOD() {
+ NonPOD a;
+ a.values[0].x = 1;
+ a.values[1].x = 2;
+ a.values[2].x = 3;
+
+ clang_analyzer_eval(a.values[0].x == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.values[1].x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.values[2].x == 3); // expected-warning{{TRUE}}
+
+ NonPOD b = a;
+
+ clang_analyzer_eval(b.values[0].x == 1); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(b.values[1].x == 2); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(b.values[2].x == 3); // expected-warning{{UNKNOWN}}
+
+ NonPOD c;
+ c = b;
+
+ clang_analyzer_eval(c.values[0].x == 1); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(c.values[1].x == 2); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(c.values[2].x == 3); // expected-warning{{UNKNOWN}}
+ }
+
+ struct NestedNonPOD {
+ NonPODIntWrapper values[2][3];
+ };
+
+ void testNestedNonPOD() {
+ NestedNonPOD a;
+ a.values[0][0].x = 0;
+ a.values[0][1].x = 0;
+ a.values[0][2].x = 0;
+ a.values[1][0].x = 1;
+ a.values[1][1].x = 2;
+ a.values[1][2].x = 3;
+
+ clang_analyzer_eval(a.values[1][0].x == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.values[1][1].x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.values[1][2].x == 3); // expected-warning{{TRUE}}
+
+ NestedNonPOD b = a;
+
+ clang_analyzer_eval(b.values[1][0].x == 1); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(b.values[1][1].x == 2); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(b.values[1][2].x == 3); // expected-warning{{UNKNOWN}}
+
+ NestedNonPOD c;
+ c = b;
+
+ clang_analyzer_eval(c.values[1][0].x == 1); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(c.values[1][1].x == 2); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(c.values[1][2].x == 3); // expected-warning{{UNKNOWN}}
+ }
+
+ struct NonPODDefaulted {
+ NonPODIntWrapper values[3];
+
+ NonPODDefaulted() = default;
+ NonPODDefaulted(const NonPODDefaulted &) = default;
+ NonPODDefaulted &operator=(const NonPODDefaulted &) = default;
+ };
+
+ void testNonPODDefaulted() {
+ NonPODDefaulted a;
+ a.values[0].x = 1;
+ a.values[1].x = 2;
+ a.values[2].x = 3;
+
+ clang_analyzer_eval(a.values[0].x == 1); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.values[1].x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(a.values[2].x == 3); // expected-warning{{TRUE}}
+
+ NonPODDefaulted b = a;
+
+ clang_analyzer_eval(b.values[0].x == 1); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(b.values[1].x == 2); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(b.values[2].x == 3); // expected-warning{{UNKNOWN}}
+
+ NonPODDefaulted c;
+ c = b;
+
+ clang_analyzer_eval(c.values[0].x == 1); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(c.values[1].x == 2); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(c.values[2].x == 3); // expected-warning{{UNKNOWN}}
+ }
+};
More information about the cfe-commits
mailing list