From: Ted Kremenek Date: Fri, 4 May 2012 21:48:42 +0000 (+0000) Subject: Explicitly model capturing variables for blocks in the static analyzer. Fixes . git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@156211 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h b/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h index 198c1bfdb2..0b21c46516 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h @@ -580,25 +580,37 @@ class BlockDataRegion : public SubRegion { const BlockTextRegion *BC; const LocationContext *LC; // Can be null */ void *ReferencedVars; + void *OriginalVars; BlockDataRegion(const BlockTextRegion *bc, const LocationContext *lc, const MemRegion *sreg) - : SubRegion(sreg, BlockDataRegionKind), BC(bc), LC(lc), ReferencedVars(0) {} + : SubRegion(sreg, BlockDataRegionKind), BC(bc), LC(lc), + ReferencedVars(0), OriginalVars(0) {} -public: +public: const BlockTextRegion *getCodeRegion() const { return BC; } const BlockDecl *getDecl() const { return BC->getDecl(); } class referenced_vars_iterator { const MemRegion * const *R; + const MemRegion * const *OriginalR; public: - explicit referenced_vars_iterator(const MemRegion * const *r) : R(r) {} + explicit referenced_vars_iterator(const MemRegion * const *r, + const MemRegion * const *originalR) + : R(r), OriginalR(originalR) {} operator const MemRegion * const *() const { return R; } - + + const MemRegion *getCapturedRegion() const { + return *R; + } + const MemRegion *getOriginalRegion() const { + return *OriginalR; + } + const VarRegion* operator*() const { return cast(*R); } @@ -611,6 +623,7 @@ public: } referenced_vars_iterator &operator++() { ++R; + ++OriginalR; return *this; } }; diff --git a/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/lib/StaticAnalyzer/Core/ExprEngineC.cpp index d5555daecd..cf94ac3060 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineC.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineC.cpp @@ -182,14 +182,35 @@ void ExprEngine::VisitBlockExpr(const BlockExpr *BE, ExplodedNode *Pred, ExplodedNodeSet &Dst) { CanQualType T = getContext().getCanonicalType(BE->getType()); + + // Get the value of the block itself. SVal V = svalBuilder.getBlockPointer(BE->getBlockDecl(), T, Pred->getLocationContext()); + ProgramStateRef State = Pred->getState(); + + // If we created a new MemRegion for the block, we should explicitly bind + // the captured variables. + if (const BlockDataRegion *BDR = + dyn_cast_or_null(V.getAsRegion())) { + + BlockDataRegion::referenced_vars_iterator I = BDR->referenced_vars_begin(), + E = BDR->referenced_vars_end(); + + for (; I != E; ++I) { + const MemRegion *capturedR = I.getCapturedRegion(); + const MemRegion *originalR = I.getOriginalRegion(); + if (capturedR != originalR) { + SVal originalV = State->getSVal(loc::MemRegionVal(originalR)); + State = State->bindLoc(loc::MemRegionVal(capturedR), originalV); + } + } + } + ExplodedNodeSet Tmp; StmtNodeBuilder Bldr(Pred, Tmp, *currentBuilderContext); Bldr.generateNode(BE, Pred, - Pred->getState()->BindExpr(BE, Pred->getLocationContext(), - V), + State->BindExpr(BE, Pred->getLocationContext(), V), false, 0, ProgramPoint::PostLValueKind); diff --git a/lib/StaticAnalyzer/Core/MemRegion.cpp b/lib/StaticAnalyzer/Core/MemRegion.cpp index 1969ebd435..e7c57ede88 100644 --- a/lib/StaticAnalyzer/Core/MemRegion.cpp +++ b/lib/StaticAnalyzer/Core/MemRegion.cpp @@ -1056,26 +1056,37 @@ void BlockDataRegion::LazyInitializeReferencedVars() { typedef BumpVector VarVec; VarVec *BV = (VarVec*) A.Allocate(); new (BV) VarVec(BC, E - I); + VarVec *BVOriginal = (VarVec*) A.Allocate(); + new (BVOriginal) VarVec(BC, E - I); for ( ; I != E; ++I) { const VarDecl *VD = *I; const VarRegion *VR = 0; + const VarRegion *OriginalVR = 0; - if (!VD->getAttr() && VD->hasLocalStorage()) + if (!VD->getAttr() && VD->hasLocalStorage()) { VR = MemMgr.getVarRegion(VD, this); + OriginalVR = MemMgr.getVarRegion(VD, LC); + } else { - if (LC) + if (LC) { VR = MemMgr.getVarRegion(VD, LC); + OriginalVR = VR; + } else { VR = MemMgr.getVarRegion(VD, MemMgr.getUnknownRegion()); + OriginalVR = MemMgr.getVarRegion(VD, LC); } } assert(VR); + assert(OriginalVR); BV->push_back(VR, BC); + BVOriginal->push_back(OriginalVR, BC); } ReferencedVars = BV; + OriginalVars = BVOriginal; } BlockDataRegion::referenced_vars_iterator @@ -1085,8 +1096,14 @@ BlockDataRegion::referenced_vars_begin() const { BumpVector *Vec = static_cast*>(ReferencedVars); - return BlockDataRegion::referenced_vars_iterator(Vec == (void*) 0x1 ? - NULL : Vec->begin()); + if (Vec == (void*) 0x1) + return BlockDataRegion::referenced_vars_iterator(0, 0); + + BumpVector *VecOriginal = + static_cast*>(OriginalVars); + + return BlockDataRegion::referenced_vars_iterator(Vec->begin(), + VecOriginal->begin()); } BlockDataRegion::referenced_vars_iterator @@ -1096,6 +1113,12 @@ BlockDataRegion::referenced_vars_end() const { BumpVector *Vec = static_cast*>(ReferencedVars); - return BlockDataRegion::referenced_vars_iterator(Vec == (void*) 0x1 ? - NULL : Vec->end()); + if (Vec == (void*) 0x1) + return BlockDataRegion::referenced_vars_iterator(0, 0); + + BumpVector *VecOriginal = + static_cast*>(OriginalVars); + + return BlockDataRegion::referenced_vars_iterator(Vec->end(), + VecOriginal->end()); } diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp index d5db03d7ce..26a7e4b24f 100644 --- a/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -681,8 +681,22 @@ void invalidateRegionsWorker::VisitBaseRegion(const MemRegion *baseR) { BI != BE; ++BI) { const VarRegion *VR = *BI; const VarDecl *VD = VR->getDecl(); - if (VD->getAttr() || !VD->hasLocalStorage()) + if (VD->getAttr() || !VD->hasLocalStorage()) { AddToWorkList(VR); + } + else if (Loc::isLocType(VR->getValueType())) { + // Map the current bindings to a Store to retrieve the value + // of the binding. If that binding itself is a region, we should + // invalidate that region. This is because a block may capture + // a pointer value, but the thing pointed by that pointer may + // get invalidated. + Store store = B.getRootWithoutRetain(); + SVal V = RM.getBinding(store, loc::MemRegionVal(VR)); + if (const Loc *L = dyn_cast(&V)) { + if (const MemRegion *LR = L->getAsRegion()) + AddToWorkList(LR); + } + } } return; } diff --git a/test/Analysis/misc-ps-region-store.m b/test/Analysis/misc-ps-region-store.m index d263d4da30..7b9578eb3e 100644 --- a/test/Analysis/misc-ps-region-store.m +++ b/test/Analysis/misc-ps-region-store.m @@ -1341,3 +1341,23 @@ static unsigned rdar_11127008(void) { return values[index].value; } +// Test handling invalidating arrays passed to a block via captured +// pointer value (not a __block variable). +typedef void (^radar11125868_cb)(int *, unsigned); + +void rdar11125868_aux(radar11125868_cb cb); + +int rdar11125868() { + int integersStackArray[1]; + int *integers = integersStackArray; + rdar11125868_aux(^(int *integerValue, unsigned index) { + integers[index] = integerValue[index]; + }); + return integers[0] == 0; // no-warning +} + +int rdar11125868_positive() { + int integersStackArray[1]; + int *integers = integersStackArray; + return integers[0] == 0; // expected-warning {{he left operand of '==' is a}} +}