Sounds good to me. Anna, you're the code owner here. Ok to merge this?
Thanks, Hans On Mon, Jan 23, 2017 at 10:37 AM, Artem Dergachev <noqnoq...@gmail.com> wrote: > Hans, > > Could we merge this one into the 4.0.0 release branch? It's a recent bugfix > for the analyzer. > > Thanks, > Artem. > > > > On 1/23/17 7:57 PM, Artem Dergachev via cfe-commits wrote: >> >> Author: dergachev >> Date: Mon Jan 23 10:57:11 2017 >> New Revision: 292800 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=292800&view=rev >> Log: >> [analyzer] Fix memory space of static locals seen from nested blocks. >> >> When a block within a function accesses a function's static local >> variable, >> this local is captured by reference rather than copied to the heap. >> >> Therefore this variable's memory space is known: StaticGlobalSpaceRegion. >> Used to be UnknownSpaceRegion, same as for stack locals. >> >> Fixes a false positive in MacOSXAPIChecker. >> >> rdar://problem/30105546 >> Differential revision: https://reviews.llvm.org/D28946 >> >> Modified: >> cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp >> cfe/trunk/test/Analysis/dispatch-once.m >> >> Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=292800&r1=292799&r2=292800&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original) >> +++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Mon Jan 23 10:57:11 >> 2017 >> @@ -776,6 +776,22 @@ getStackOrCaptureRegionForDeclContext(co >> return (const StackFrameContext *)nullptr; >> } >> +static CanQualType getBlockPointerType(const BlockDecl *BD, ASTContext >> &C) { >> + // FIXME: The fallback type here is totally bogus -- though it should >> + // never be queried, it will prevent uniquing with the real >> + // BlockCodeRegion. Ideally we'd fix the AST so that we always had a >> + // signature. >> + QualType T; >> + if (const TypeSourceInfo *TSI = BD->getSignatureAsWritten()) >> + T = TSI->getType(); >> + if (T.isNull()) >> + T = C.VoidTy; >> + if (!T->getAs<FunctionType>()) >> + T = C.getFunctionNoProtoType(T); >> + T = C.getBlockPointerType(T); >> + return C.getCanonicalType(T); >> +} >> + >> const VarRegion* MemRegionManager::getVarRegion(const VarDecl *D, >> const LocationContext >> *LC) { >> const MemRegion *sReg = nullptr; >> @@ -803,7 +819,7 @@ const VarRegion* MemRegionManager::getVa >> sReg = getGlobalsRegion(); >> } >> - // Finally handle static locals. >> + // Finally handle locals. >> } else { >> // FIXME: Once we implement scope handling, we will need to properly >> lookup >> // 'D' to the proper LocationContext. >> @@ -816,9 +832,22 @@ const VarRegion* MemRegionManager::getVa >> const StackFrameContext *STC = V.get<const StackFrameContext*>(); >> - if (!STC) >> - sReg = getUnknownRegion(); >> - else { >> + if (!STC) { >> + if (D->isStaticLocal()) { >> + const CodeTextRegion *fReg = nullptr; >> + if (const auto *ND = dyn_cast<NamedDecl>(DC)) >> + fReg = getFunctionCodeRegion(ND); >> + else if (const auto *BD = dyn_cast<BlockDecl>(DC)) >> + fReg = getBlockCodeRegion(BD, getBlockPointerType(BD, >> getContext()), >> + LC->getAnalysisDeclContext()); >> + assert(fReg && "Unable to determine code region for a static >> local!"); >> + sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind, >> fReg); >> + } else { >> + // We're looking at a block-captured local variable, which may be >> either >> + // still local, or already moved to the heap. So we're not sure. >> + sReg = getUnknownRegion(); >> + } >> + } else { >> if (D->hasLocalStorage()) { >> sReg = isa<ParmVarDecl>(D) || isa<ImplicitParamDecl>(D) >> ? static_cast<const >> MemRegion*>(getStackArgumentsRegion(STC)) >> @@ -831,22 +860,9 @@ const VarRegion* MemRegionManager::getVa >> sReg = >> getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind, >> >> getFunctionCodeRegion(cast<NamedDecl>(STCD))); >> else if (const BlockDecl *BD = dyn_cast<BlockDecl>(STCD)) { >> - // FIXME: The fallback type here is totally bogus -- though it >> should >> - // never be queried, it will prevent uniquing with the real >> - // BlockCodeRegion. Ideally we'd fix the AST so that we always >> had a >> - // signature. >> - QualType T; >> - if (const TypeSourceInfo *TSI = BD->getSignatureAsWritten()) >> - T = TSI->getType(); >> - if (T.isNull()) >> - T = getContext().VoidTy; >> - if (!T->getAs<FunctionType>()) >> - T = getContext().getFunctionNoProtoType(T); >> - T = getContext().getBlockPointerType(T); >> - >> const BlockCodeRegion *BTR = >> - getBlockCodeRegion(BD, C.getCanonicalType(T), >> - STC->getAnalysisDeclContext()); >> + getBlockCodeRegion(BD, getBlockPointerType(BD, >> getContext()), >> + STC->getAnalysisDeclContext()); >> sReg = >> getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind, >> BTR); >> } >> >> Modified: cfe/trunk/test/Analysis/dispatch-once.m >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dispatch-once.m?rev=292800&r1=292799&r2=292800&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/Analysis/dispatch-once.m (original) >> +++ cfe/trunk/test/Analysis/dispatch-once.m Mon Jan 23 10:57:11 2017 >> @@ -107,3 +107,10 @@ void test_block_var_from_outside_block() >> }; >> dispatch_once(&once, ^{}); // expected-warning{{Call to >> 'dispatch_once' uses the block variable 'once' for the predicate value.}} >> } >> + >> +void test_static_var_from_outside_block() { >> + static dispatch_once_t once; >> + ^{ >> + dispatch_once(&once, ^{}); // no-warning >> + }; >> +} >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
