r292800 - [analyzer] Fix memory space of static locals seen from nested blocks.
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 23 16:19:42 PST 2017
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
>>>
>>>
More information about the cfe-commits
mailing list