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 13:50:38 PST 2017


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