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

[Hans Wennborg via cfe-commits]

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  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  wrote:
>
> Merged in r292858.
>
> Thanks,
> Hans
>
> On Mon, Jan 23, 2017 at 4:15 PM, Anna Zaks  wrote:
>
> Yes, ok to merge!
> Thank you.
>
> Sent from my iPhone
>
> On Jan 23, 2017, at 1:50 PM, Hans Wennborg  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 
> 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=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=292799=292800=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
> ) {
> +  // 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())
> +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();
> -if (!STC)
> -  sReg = getUnknownRegion();
> -else {
> +if (!STC) {
> +  if (D->isStaticLocal()) {
> +const CodeTextRegion *fReg = nullptr;
> +if (const auto *ND = dyn_cast(DC))
> +  fReg = getFunctionCodeRegion(ND);
> +else if (const auto *BD = dyn_cast(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(D) || isa(D)
>? static_cast MemRegion*>(getStackArgumentsRegion(STC))
> @@ -831,22 +860,9 @@ const VarRegion* MemRegionManager::getVa
>   sReg =
> getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,
>
> getFunctionCodeRegion(cast(STCD)));
> else if (const BlockDecl *BD = dyn_cast(STCD)) {
> -  // FIXME: The fallback type here is totally bogus -- though it
> 

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

[Devin Coughlin via cfe-commits]

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  wrote:
> 
> Merged in r292858.
> 
> Thanks,
> Hans
> 
> On Mon, Jan 23, 2017 at 4:15 PM, Anna Zaks  wrote:
>> Yes, ok to merge!
>> Thank you.
>> 
>> Sent from my iPhone
>> 
>>> On Jan 23, 2017, at 1:50 PM, Hans Wennborg  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  
 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=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=292799=292800=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
> ) {
> +  // 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())
> +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();
> -if (!STC)
> -  sReg = getUnknownRegion();
> -else {
> +if (!STC) {
> +  if (D->isStaticLocal()) {
> +const CodeTextRegion *fReg = nullptr;
> +if (const auto *ND = dyn_cast(DC))
> +  fReg = getFunctionCodeRegion(ND);
> +else if (const auto *BD = dyn_cast(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(D) || isa(D)
>? static_cast MemRegion*>(getStackArgumentsRegion(STC))
> @@ -831,22 +860,9 @@ const VarRegion* MemRegionManager::getVa
>  

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

[Hans Wennborg via cfe-commits]

Merged in r292858.

Thanks,
Hans

On Mon, Jan 23, 2017 at 4:15 PM, Anna Zaks  wrote:
> Yes, ok to merge!
> Thank you.
>
> Sent from my iPhone
>
>> On Jan 23, 2017, at 1:50 PM, Hans Wennborg  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  
>>> 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=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=292799=292800=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
 ) {
 +  // 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())
 +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();
  -if (!STC)
 -  sReg = getUnknownRegion();
 -else {
 +if (!STC) {
 +  if (D->isStaticLocal()) {
 +const CodeTextRegion *fReg = nullptr;
 +if (const auto *ND = dyn_cast(DC))
 +  fReg = getFunctionCodeRegion(ND);
 +else if (const auto *BD = dyn_cast(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(D) || isa(D)
 ? static_cast>>> MemRegion*>(getStackArgumentsRegion(STC))
 @@ -831,22 +860,9 @@ const VarRegion* MemRegionManager::getVa
sReg =
 getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,

 getFunctionCodeRegion(cast(STCD)));
  else if (const BlockDecl *BD = dyn_cast(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

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

[Anna Zaks via cfe-commits]

Yes, ok to merge!
Thank you.

Sent from my iPhone

> On Jan 23, 2017, at 1:50 PM, Hans Wennborg  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  
>> 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=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=292799=292800=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
>>> ) {
>>> +  // 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())
>>> +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();
>>>  -if (!STC)
>>> -  sReg = getUnknownRegion();
>>> -else {
>>> +if (!STC) {
>>> +  if (D->isStaticLocal()) {
>>> +const CodeTextRegion *fReg = nullptr;
>>> +if (const auto *ND = dyn_cast(DC))
>>> +  fReg = getFunctionCodeRegion(ND);
>>> +else if (const auto *BD = dyn_cast(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(D) || isa(D)
>>> ? static_cast>> MemRegion*>(getStackArgumentsRegion(STC))
>>> @@ -831,22 +860,9 @@ const VarRegion* MemRegionManager::getVa
>>>sReg =
>>> getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,
>>> 
>>> getFunctionCodeRegion(cast(STCD)));
>>>  else if (const BlockDecl *BD = dyn_cast(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 = 

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

[Hans Wennborg via cfe-commits]

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  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=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=292799=292800=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
>> ) {
>> +  // 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())
>> +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();
>>   -if (!STC)
>> -  sReg = getUnknownRegion();
>> -else {
>> +if (!STC) {
>> +  if (D->isStaticLocal()) {
>> +const CodeTextRegion *fReg = nullptr;
>> +if (const auto *ND = dyn_cast(DC))
>> +  fReg = getFunctionCodeRegion(ND);
>> +else if (const auto *BD = dyn_cast(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(D) || isa(D)
>>  ? static_cast> MemRegion*>(getStackArgumentsRegion(STC))
>> @@ -831,22 +860,9 @@ const VarRegion* MemRegionManager::getVa
>> sReg =
>> getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,
>>
>> getFunctionCodeRegion(cast(STCD)));
>>   else if (const BlockDecl *BD = dyn_cast(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())
>> -T = getContext().getFunctionNoProtoType(T);
>> -  T = getContext().getBlockPointerType(T);
>> -
>> const BlockCodeRegion *BTR =
>> -getBlockCodeRegion(BD, 

Re: 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=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=292799=292800=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 ) {

+  // 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())
+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();
  
-if (!STC)

-  sReg = getUnknownRegion();
-else {
+if (!STC) {
+  if (D->isStaticLocal()) {
+const CodeTextRegion *fReg = nullptr;
+if (const auto *ND = dyn_cast(DC))
+  fReg = getFunctionCodeRegion(ND);
+else if (const auto *BD = dyn_cast(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(D) || isa(D)
 ? static_cast(getStackArgumentsRegion(STC))
@@ -831,22 +860,9 @@ const VarRegion* MemRegionManager::getVa
sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,

getFunctionCodeRegion(cast(STCD)));
  else if (const BlockDecl *BD = dyn_cast(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())
-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: