r292800 - [analyzer] Fix memory space of static locals seen from nested blocks.
Devin Coughlin via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 23 18:30:30 PST 2017
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 <hans at chromium.org> wrote:
>
> Merged in r292858.
>
> Thanks,
> Hans
>
> On Mon, Jan 23, 2017 at 4:15 PM, Anna Zaks <ganna at apple.com> wrote:
>> Yes, ok to merge!
>> Thank you.
>>
>> Sent from my iPhone
>>
>>> On Jan 23, 2017, at 1:50 PM, Hans Wennborg <hans at 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 <noqnoqneo at 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 at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170123/985e6703/attachment-0001.html>
More information about the cfe-commits
mailing list