summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h22
-rw-r--r--clang/lib/StaticAnalyzer/Core/ExprEngine.cpp8
-rw-r--r--clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp5
-rw-r--r--clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp126
-rw-r--r--clang/test/Analysis/initializer.cpp54
5 files changed, 177 insertions, 38 deletions
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 8337f770d4b..99083c9d613 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -604,6 +604,28 @@ private:
const LocationContext *LC,
const Expr *E,
const Expr *ResultE = nullptr);
+
+ /// For a DeclStmt or CXXInitCtorInitializer, walk backward in the current CFG
+ /// block to find the constructor expression that directly constructed into
+ /// the storage for this statement. Returns null if the constructor for this
+ /// statement created a temporary object region rather than directly
+ /// constructing into an existing region.
+ const CXXConstructExpr *findDirectConstructorForCurrentCFGElement();
+
+ /// For a CXXConstructExpr, walk forward in the current CFG block to find the
+ /// CFGElement for the DeclStmt or CXXInitCtorInitializer for which is
+ /// directly constructed by this constructor. Returns None if the current
+ /// constructor expression did not directly construct into an existing
+ /// region.
+ Optional<CFGElement> findElementDirectlyInitializedByCurrentConstructor();
+
+ /// For a given constructor, look forward in the current CFG block to
+ /// determine the region into which an object will be constructed by \p CE.
+ /// Returns either a field or local variable region if the object will be
+ /// directly constructed in an existing region or a temporary object region
+ /// if not.
+ const MemRegion *getRegionForConstructedObject(const CXXConstructExpr *CE,
+ ExplodedNode *Pred);
};
/// Traits for storing the call processing policy inside GDM.
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 7ef91d65385..662b0a2dd79 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -476,8 +476,12 @@ void ExprEngine::ProcessInitializer(const CFGInitializer Init,
if (BMI->isAnyMemberInitializer()) {
// Constructors build the object directly in the field,
// but non-objects must be copied in from the initializer.
- const Expr *Init = BMI->getInit()->IgnoreImplicit();
- if (!isa<CXXConstructExpr>(Init)) {
+ if (auto *CtorExpr = findDirectConstructorForCurrentCFGElement()) {
+ assert(BMI->getInit()->IgnoreImplicit() == CtorExpr);
+ (void)CtorExpr;
+ // The field was directly constructed, so there is no need to bind.
+ } else {
+ const Expr *Init = BMI->getInit()->IgnoreImplicit();
const ValueDecl *Field;
if (BMI->isIndirectMemberInitializer()) {
Field = BMI->getIndirectMember();
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 35af3055243..a5b58710b21 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -492,7 +492,10 @@ void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred,
ExplodedNode *UpdatedN = N;
SVal InitVal = state->getSVal(InitEx, LC);
- if (isa<CXXConstructExpr>(InitEx->IgnoreImplicit())) {
+ assert(DS->isSingleDecl());
+ if (auto *CtorExpr = findDirectConstructorForCurrentCFGElement()) {
+ assert(InitEx->IgnoreImplicit() == CtorExpr);
+ (void)CtorExpr;
// We constructed the object directly in the variable.
// No need to bind anything.
B.generateNode(DS, UpdatedN, state);
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 07ae41027d3..556e2239abf 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -103,49 +103,32 @@ static SVal makeZeroElementRegion(ProgramStateRef State, SVal LValue,
}
-static const MemRegion *getRegionForConstructedObject(
- const CXXConstructExpr *CE, ExplodedNode *Pred, ExprEngine &Eng,
- unsigned int CurrStmtIdx) {
+const MemRegion *
+ExprEngine::getRegionForConstructedObject(const CXXConstructExpr *CE,
+ ExplodedNode *Pred) {
const LocationContext *LCtx = Pred->getLocationContext();
ProgramStateRef State = Pred->getState();
- const NodeBuilderContext &CurrBldrCtx = Eng.getBuilderContext();
// See if we're constructing an existing region by looking at the next
// element in the CFG.
- const CFGBlock *B = CurrBldrCtx.getBlock();
- unsigned int NextStmtIdx = CurrStmtIdx + 1;
- if (NextStmtIdx < B->size()) {
- CFGElement Next = (*B)[NextStmtIdx];
-
- // Is this a destructor? If so, we might be in the middle of an assignment
- // to a local or member: look ahead one more element to see what we find.
- while (Next.getAs<CFGImplicitDtor>() && NextStmtIdx + 1 < B->size()) {
- ++NextStmtIdx;
- Next = (*B)[NextStmtIdx];
- }
- // Is this a constructor for a local variable?
- if (Optional<CFGStmt> StmtElem = Next.getAs<CFGStmt>()) {
- if (const DeclStmt *DS = dyn_cast<DeclStmt>(StmtElem->getStmt())) {
- if (const VarDecl *Var = dyn_cast<VarDecl>(DS->getSingleDecl())) {
- if (Var->getInit() && Var->getInit()->IgnoreImplicit() == CE) {
- SVal LValue = State->getLValue(Var, LCtx);
- QualType Ty = Var->getType();
- LValue = makeZeroElementRegion(State, LValue, Ty);
- return LValue.getAsRegion();
- }
+ if (auto Elem = findElementDirectlyInitializedByCurrentConstructor()) {
+ if (Optional<CFGStmt> StmtElem = Elem->getAs<CFGStmt>()) {
+ auto *DS = cast<DeclStmt>(StmtElem->getStmt());
+ if (const auto *Var = dyn_cast<VarDecl>(DS->getSingleDecl())) {
+ if (Var->getInit() && Var->getInit()->IgnoreImplicit() == CE) {
+ SVal LValue = State->getLValue(Var, LCtx);
+ QualType Ty = Var->getType();
+ LValue = makeZeroElementRegion(State, LValue, Ty);
+ return LValue.getAsRegion();
}
}
- }
-
- // Is this a constructor for a member?
- if (Optional<CFGInitializer> InitElem = Next.getAs<CFGInitializer>()) {
+ } else if (Optional<CFGInitializer> InitElem = Elem->getAs<CFGInitializer>()) {
const CXXCtorInitializer *Init = InitElem->getInitializer();
assert(Init->isAnyMemberInitializer());
-
const CXXMethodDecl *CurCtor = cast<CXXMethodDecl>(LCtx->getDecl());
- Loc ThisPtr = Eng.getSValBuilder().getCXXThis(CurCtor,
- LCtx->getCurrentStackFrame());
+ Loc ThisPtr =
+ getSValBuilder().getCXXThis(CurCtor, LCtx->getCurrentStackFrame());
SVal ThisVal = State->getSVal(ThisPtr);
const ValueDecl *Field;
@@ -167,13 +150,86 @@ static const MemRegion *getRegionForConstructedObject(
// Don't forget to update the pre-constructor initialization code in
// ExprEngine::VisitCXXConstructExpr.
}
-
// If we couldn't find an existing region to construct into, assume we're
// constructing a temporary.
- MemRegionManager &MRMgr = Eng.getSValBuilder().getRegionManager();
+ MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
return MRMgr.getCXXTempObjectRegion(CE, LCtx);
}
+/// Returns true if the initializer for \Elem can be a direct
+/// constructor.
+static bool canHaveDirectConstructor(CFGElement Elem){
+ // DeclStmts and CXXCtorInitializers for fields can be directly constructed.
+
+ if (Optional<CFGStmt> StmtElem = Elem.getAs<CFGStmt>()) {
+ if (isa<DeclStmt>(StmtElem->getStmt())) {
+ return true;
+ }
+ }
+
+ if (Elem.getKind() == CFGElement::Initializer) {
+ return true;
+ }
+
+ return false;
+}
+
+Optional<CFGElement>
+ExprEngine::findElementDirectlyInitializedByCurrentConstructor() {
+ const NodeBuilderContext &CurrBldrCtx = getBuilderContext();
+ // See if we're constructing an existing region by looking at the next
+ // element in the CFG.
+ const CFGBlock *B = CurrBldrCtx.getBlock();
+ assert(isa<CXXConstructExpr>(((*B)[currStmtIdx]).castAs<CFGStmt>().getStmt()));
+ unsigned int NextStmtIdx = currStmtIdx + 1;
+ if (NextStmtIdx >= B->size())
+ return None;
+
+ CFGElement Next = (*B)[NextStmtIdx];
+
+ // Is this a destructor? If so, we might be in the middle of an assignment
+ // to a local or member: look ahead one more element to see what we find.
+ while (Next.getAs<CFGImplicitDtor>() && NextStmtIdx + 1 < B->size()) {
+ ++NextStmtIdx;
+ Next = (*B)[NextStmtIdx];
+ }
+
+ if (canHaveDirectConstructor(Next))
+ return Next;
+
+ return None;
+}
+
+const CXXConstructExpr *
+ExprEngine::findDirectConstructorForCurrentCFGElement() {
+ // Go backward in the CFG to see if the previous element (ignoring
+ // destructors) was a CXXConstructExpr. If so, that constructor
+ // was constructed directly into an existing region.
+ // This process is essentially the inverse of that performed in
+ // findElementDirectlyInitializedByCurrentConstructor().
+ if (currStmtIdx == 0)
+ return nullptr;
+
+ const CFGBlock *B = getBuilderContext().getBlock();
+ assert(canHaveDirectConstructor((*B)[currStmtIdx]));
+
+ unsigned int PreviousStmtIdx = currStmtIdx - 1;
+ CFGElement Previous = (*B)[PreviousStmtIdx];
+
+ while (Previous.getAs<CFGImplicitDtor>() && PreviousStmtIdx > 0) {
+ --PreviousStmtIdx;
+ Previous = (*B)[PreviousStmtIdx];
+ }
+
+ if (Optional<CFGStmt> PrevStmtElem = Previous.getAs<CFGStmt>()) {
+ if (auto *CtorExpr = dyn_cast<CXXConstructExpr>(PrevStmtElem->getStmt())) {
+ return CtorExpr;
+ }
+ }
+
+ return nullptr;
+}
+
void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
ExplodedNode *Pred,
ExplodedNodeSet &destNodes) {
@@ -188,7 +244,7 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
switch (CE->getConstructionKind()) {
case CXXConstructExpr::CK_Complete: {
- Target = getRegionForConstructedObject(CE, Pred, *this, currStmtIdx);
+ Target = getRegionForConstructedObject(CE, Pred);
break;
}
case CXXConstructExpr::CK_VirtualBase:
diff --git a/clang/test/Analysis/initializer.cpp b/clang/test/Analysis/initializer.cpp
index a71e35dc63c..b31c315ba52 100644
--- a/clang/test/Analysis/initializer.cpp
+++ b/clang/test/Analysis/initializer.cpp
@@ -143,3 +143,57 @@ namespace DefaultMemberInitializers {
clang_analyzer_eval(w.p[1] == 'y'); // expected-warning{{TRUE}}
}
}
+
+namespace ReferenceInitialization {
+ struct OtherStruct {
+ OtherStruct(int i);
+ ~OtherStruct();
+ };
+
+ struct MyStruct {
+ MyStruct(int i);
+ MyStruct(OtherStruct os);
+
+ void method() const;
+ };
+
+ void referenceInitializeLocal() {
+ const MyStruct &myStruct(5);
+ myStruct.method(); // no-warning
+ }
+
+ void referenceInitializeMultipleLocals() {
+ const MyStruct &myStruct1(5), myStruct2(5), &myStruct3(5);
+ myStruct1.method(); // no-warning
+ myStruct2.method(); // no-warning
+ myStruct3.method(); // no-warning
+ }
+
+ void referenceInitializeLocalWithCleanup() {
+ const MyStruct &myStruct(OtherStruct(5));
+ myStruct.method(); // no-warning
+ }
+
+ struct HasMyStruct {
+ const MyStruct &ms; // expected-note {{reference member declared here}}
+ const MyStruct &msWithCleanups; // expected-note {{reference member declared here}}
+
+ // clang's Sema issues a warning when binding a reference member to a
+ // temporary value.
+ HasMyStruct() : ms(5), msWithCleanups(OtherStruct(5)) {
+ // expected-warning@-1 {{binding reference member 'ms' to a temporary value}}
+ // expected-warning@-2 {{binding reference member 'msWithCleanups' to a temporary value}}
+
+ // At this point the members are not garbage so we should not expect an
+ // analyzer warning here even though binding a reference member
+ // to a member is a terrible idea.
+ ms.method(); // no-warning
+ msWithCleanups.method(); // no-warning
+ }
+ };
+
+ void referenceInitializeField() {
+ HasMyStruct hms;
+ }
+
+};
OpenPOWER on IntegriCloud