r292800 - [analyzer] Fix memory space of static locals seen from nested blocks.

[Artem Dergachev via cfe-commits]

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