NoQ updated this revision to Diff 129365.
NoQ added a comment.
Rebase.
Add a FIXME to bring back the cast in the conservative case.
https://reviews.llvm.org/D41250
Files:
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/new-ctor-conservative.cpp
test/Analysis/new-ctor-inlined.cpp
Index: test/Analysis/new-ctor-inlined.cpp
===================================================================
--- test/Analysis/new-ctor-inlined.cpp
+++ test/Analysis/new-ctor-inlined.cpp
@@ -38,3 +38,18 @@
Sp *p = new Sp(new Sp(0));
clang_analyzer_eval(p->p->p == 0); // expected-warning{{TRUE}}
}
+
+void checkNewPOD() {
+ int *i = new int;
+ clang_analyzer_eval(*i == 0); // expected-warning{{UNKNOWN}}
+ int *j = new int();
+ clang_analyzer_eval(*j == 0); // expected-warning{{TRUE}}
+ int *k = new int(5);
+ clang_analyzer_eval(*k == 5); // expected-warning{{TRUE}}
+}
+
+void checkTrivialCopy() {
+ S s;
+ S *t = new S(s); // no-crash
+ clang_analyzer_eval(t->x == 1); // expected-warning{{TRUE}}
+}
Index: test/Analysis/new-ctor-conservative.cpp
===================================================================
--- test/Analysis/new-ctor-conservative.cpp
+++ test/Analysis/new-ctor-conservative.cpp
@@ -12,3 +12,18 @@
S *s = new S;
clang_analyzer_eval(s->x == 1); // expected-warning{{TRUE}}
}
+
+void checkNewPOD() {
+ int *i = new int;
+ clang_analyzer_eval(*i == 0); // expected-warning{{UNKNOWN}}
+ int *j = new int();
+ clang_analyzer_eval(*j == 0); // expected-warning{{TRUE}}
+ int *k = new int(5);
+ clang_analyzer_eval(*k == 5); // expected-warning{{TRUE}}
+}
+
+void checkNewArray() {
+ S *s = new S[10];
+ // FIXME: Should be true once we inline array constructors.
+ clang_analyzer_eval(s[0].x == 1); // expected-warning{{UNKNOWN}}
+}
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -277,12 +277,19 @@
state = state->BindExpr(CCE, callerCtx, ThisV);
}
- if (const CXXNewExpr *CNE = dyn_cast<CXXNewExpr>(CE)) {
+ if (const auto *CNE = dyn_cast<CXXNewExpr>(CE)) {
// We are currently evaluating a CXXNewAllocator CFGElement. It takes a
// while to reach the actual CXXNewExpr element from here, so keep the
// region for later use.
- state = setCXXNewAllocatorValue(state, CNE, calleeCtx->getParent(),
- state->getSVal(CE, callerCtx));
+ // Additionally cast the return value of the inlined operator new
+ // (which is of type 'void *') to the correct object type.
+ SVal AllocV = state->getSVal(CNE, callerCtx);
+ AllocV = svalBuilder.evalCast(
+ AllocV, CNE->getType(),
+ getContext().getPointerType(getContext().VoidTy));
+
+ state =
+ setCXXNewAllocatorValue(state, CNE, calleeCtx->getParent(), AllocV);
}
}
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -119,9 +119,17 @@
if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
// TODO: Detect when the allocator returns a null pointer.
// Constructor shall not be called in this case.
- if (const MemRegion *MR =
- getCXXNewAllocatorValue(State, CNE, LCtx).getAsRegion())
+ if (const SubRegion *MR = dyn_cast_or_null<SubRegion>(
+ getCXXNewAllocatorValue(State, CNE, LCtx).getAsRegion())) {
+ if (CNE->isArray()) {
+ // TODO: This code exists only to trigger the suppression for
+ // array constructors. In fact, we need to call the constructor
+ // for every allocated element, not just the first one!
+ return getStoreManager().GetElementZeroRegion(
+ MR, CNE->getType()->getPointeeType());
+ }
return MR;
+ }
}
} else if (auto *DS = dyn_cast<DeclStmt>(StmtElem->getStmt())) {
if (const auto *Var = dyn_cast<VarDecl>(DS->getSingleDecl())) {
@@ -473,12 +481,22 @@
StmtNodeBuilder CallBldr(DstPreCall, DstPostCall, *currBldrCtx);
for (auto I : DstPreCall)
defaultEvalCall(CallBldr, I, *Call);
+ // If the call is inlined, DstPostCall will be empty and we bail out now.
// Store return value of operator new() for future use, until the actual
// CXXNewExpr gets processed.
ExplodedNodeSet DstPostValue;
StmtNodeBuilder ValueBldr(DstPostCall, DstPostValue, *currBldrCtx);
for (auto I : DstPostCall) {
+ // FIXME: Because CNE serves as the "call site" for the allocator (due to
+ // lack of a better expression in the AST), the conjured return value symbol
+ // is going to be of the same type (C++ object pointer type). Technically
+ // this is not correct because the operator new's prototype always says that
+ // it returns a 'void *'. So we should change the type of the symbol,
+ // and then evaluate the cast over the symbolic pointer from 'void *' to
+ // the object pointer type. But without changing the symbol's type it
+ // is breaking too much to evaluate the no-op symbolic cast over it, so we
+ // skip it for now.
ProgramStateRef State = I->getState();
ValueBldr.generateNode(
CNE, I,
@@ -564,25 +582,28 @@
StmtNodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+ SVal Result = symVal;
+
if (CNE->isArray()) {
// FIXME: allocating an array requires simulating the constructors.
// For now, just return a symbolicated region.
- const SubRegion *NewReg =
- symVal.castAs<loc::MemRegionVal>().getRegionAs<SubRegion>();
- QualType ObjTy = CNE->getType()->getAs<PointerType>()->getPointeeType();
- const ElementRegion *EleReg =
- getStoreManager().GetElementZeroRegion(NewReg, ObjTy);
- State = State->BindExpr(CNE, Pred->getLocationContext(),
- loc::MemRegionVal(EleReg));
+ if (!AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
+ const SubRegion *NewReg =
+ symVal.castAs<loc::MemRegionVal>().getRegionAs<SubRegion>();
+ QualType ObjTy = CNE->getType()->getAs<PointerType>()->getPointeeType();
+ const ElementRegion *EleReg =
+ getStoreManager().GetElementZeroRegion(NewReg, ObjTy);
+ Result = loc::MemRegionVal(EleReg);
+ }
+ State = State->BindExpr(CNE, Pred->getLocationContext(), Result);
Bldr.generateNode(CNE, Pred, State);
return;
}
// 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.)
- SVal Result = symVal;
if (FD && FD->isReservedGlobalPlacementOperator()) {
// Non-array placement new should always return the placement location.
SVal PlacementLoc = State->getSVal(CNE->getPlacementArg(0), LCtx);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
