FYI, I reverted r292800 from trunk in r292874. It was causing our internal validation bots to have false positives whenever a static local was dereferenced/passed to a nonnull function in a block evaluated at the top level.
Devin > On Jan 23, 2017, at 4:19 PM, Hans Wennborg <h...@chromium.org> wrote: > > Merged in r292858. > > Thanks, > Hans > > On Mon, Jan 23, 2017 at 4:15 PM, Anna Zaks <ga...@apple.com> wrote: >> Yes, ok to merge! >> Thank you. >> >> Sent from my iPhone >> >>> On Jan 23, 2017, at 1:50 PM, Hans Wennborg <h...@chromium.org> wrote: >>> >>> 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
