r292800 - [analyzer] Fix memory space of static locals seen from nested blocks.
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 24 09:05:18 PST 2017
Thanks for letting me know!
I've merged your revert to the branch in r292947. Please let me know
if there is any more follow-up.
On Mon, Jan 23, 2017 at 6:30 PM, Devin Coughlin <dcoughlin at apple.com> wrote:
> 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
>
>
>
>
More information about the cfe-commits
mailing list