r293043 - [analyzer] Fix MacOSXAPIChecker fp with static locals seen from nested blocks.
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 3 13:56:57 PST 2017
r294050.
Thanks,
Hans
On Fri, Feb 3, 2017 at 1:50 PM, Anna Zaks <ganna at apple.com> wrote:
> Fine with merging. Thank you!
> Anna.
>> On Feb 1, 2017, at 11:00 AM, Hans Wennborg <hans at chromium.org> wrote:
>>
>> If Anna is Ok with it, I'm fine with merging.
>>
>> Thanks,
>> Hans
>>
>> On Wed, Feb 1, 2017 at 10:29 AM, Artem Dergachev <noqnoqneo at gmail.com> wrote:
>>> Hans,
>>>
>>> This is a fixed and tested version of the previously-merged-and-reverted
>>> r292800, do we still have time to land this into 4.0?
>>>
>>> Thanks,
>>> Artem.
>>>
>>>
>>> On 1/25/17 1:21 PM, Artem Dergachev via cfe-commits wrote:
>>>>
>>>> Author: dergachev
>>>> Date: Wed Jan 25 04:21:45 2017
>>>> New Revision: 293043
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=293043&view=rev
>>>> Log:
>>>> [analyzer] Fix MacOSXAPIChecker fp with static locals seen from nested
>>>> blocks.
>>>>
>>>> This is an attempt to avoid new false positives caused by the reverted
>>>> r292800,
>>>> however the scope of the fix is significantly reduced - some variables are
>>>> still
>>>> in incorrect memory spaces.
>>>>
>>>> Relevant test cases added.
>>>>
>>>> rdar://problem/30105546
>>>> rdar://problem/30156693
>>>> Differential revision: https://reviews.llvm.org/D28946
>>>>
>>>> Added:
>>>> cfe/trunk/test/Analysis/null-deref-static.m
>>>> Modified:
>>>> cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
>>>> cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
>>>> cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
>>>> cfe/trunk/test/Analysis/dispatch-once.m
>>>>
>>>> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp?rev=293043&r1=293042&r2=293043&view=diff
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp (original)
>>>> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp Wed Jan 25
>>>> 04:21:45 2017
>>>> @@ -94,11 +94,18 @@ void MacOSXAPIChecker::CheckDispatchOnce
>>>> bool SuggestStatic = false;
>>>> os << "Call to '" << FName << "' uses";
>>>> if (const VarRegion *VR = dyn_cast<VarRegion>(RB)) {
>>>> + const VarDecl *VD = VR->getDecl();
>>>> + // FIXME: These should have correct memory space and thus should be
>>>> filtered
>>>> + // out earlier. This branch only fires when we're looking from a
>>>> block,
>>>> + // which we analyze as a top-level declaration, onto a static local
>>>> + // in a function that contains the block.
>>>> + if (VD->isStaticLocal())
>>>> + return;
>>>> // We filtered out globals earlier, so it must be a local variable
>>>> // or a block variable which is under UnknownSpaceRegion.
>>>> if (VR != R)
>>>> os << " memory within";
>>>> - if (VR->getDecl()->hasAttr<BlocksAttr>())
>>>> + if (VD->hasAttr<BlocksAttr>())
>>>> os << " the block variable '";
>>>> else
>>>> os << " the local variable '";
>>>>
>>>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=293043&r1=293042&r2=293043&view=diff
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
>>>> +++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Wed Jan 25 04:21:45
>>>> 2017
>>>> @@ -816,9 +816,11 @@ const VarRegion* MemRegionManager::getVa
>>>> const StackFrameContext *STC = V.get<const StackFrameContext*>();
>>>> - if (!STC)
>>>> + if (!STC) {
>>>> + // FIXME: Assign a more sensible memory space to static locals
>>>> + // we see from within blocks that we analyze as top-level
>>>> declarations.
>>>> sReg = getUnknownRegion();
>>>> - else {
>>>> + } else {
>>>> if (D->hasLocalStorage()) {
>>>> sReg = isa<ParmVarDecl>(D) || isa<ImplicitParamDecl>(D)
>>>> ? static_cast<const
>>>> MemRegion*>(getStackArgumentsRegion(STC))
>>>>
>>>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=293043&r1=293042&r2=293043&view=diff
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
>>>> +++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Wed Jan 25 04:21:45
>>>> 2017
>>>> @@ -1849,6 +1849,8 @@ SVal RegionStoreManager::getBindingForVa
>>>> // Function-scoped static variables are default-initialized to 0;
>>>> if they
>>>> // have an initializer, it would have been processed by now.
>>>> + // FIXME: This is only true when we're starting analysis from main().
>>>> + // We're losing a lot of coverage here.
>>>> if (isa<StaticGlobalSpaceRegion>(MS))
>>>> return svalBuilder.makeZeroVal(T);
>>>>
>>>> Modified: cfe/trunk/test/Analysis/dispatch-once.m
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dispatch-once.m?rev=293043&r1=293042&r2=293043&view=diff
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/test/Analysis/dispatch-once.m (original)
>>>> +++ cfe/trunk/test/Analysis/dispatch-once.m Wed Jan 25 04:21:45 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
>>>> + };
>>>> +}
>>>>
>>>> Added: cfe/trunk/test/Analysis/null-deref-static.m
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/null-deref-static.m?rev=293043&view=auto
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/test/Analysis/null-deref-static.m (added)
>>>> +++ cfe/trunk/test/Analysis/null-deref-static.m Wed Jan 25 04:21:45 2017
>>>> @@ -0,0 +1,35 @@
>>>> +// RUN: %clang_cc1 -w -fblocks -analyze
>>>> -analyzer-checker=core,deadcode,alpha.core,debug.ExprInspection -verify %s
>>>> +
>>>> +void *malloc(unsigned long);
>>>> +void clang_analyzer_warnIfReached();
>>>> +
>>>> +void test_static_from_block() {
>>>> + static int *x;
>>>> + ^{
>>>> + *x; // no-warning
>>>> + };
>>>> +}
>>>> +
>>>> +void test_static_within_block() {
>>>> + ^{
>>>> + static int *x;
>>>> + *x; // expected-warning{{Dereference of null pointer}}
>>>> + };
>>>> +}
>>>> +
>>>> +void test_static_control_flow(int y) {
>>>> + static int *x;
>>>> + if (x) {
>>>> + // FIXME: Should be reachable.
>>>> + clang_analyzer_warnIfReached(); // no-warning
>>>> + }
>>>> + if (y) {
>>>> + // We are not sure if this branch is possible, because the developer
>>>> + // may argue that function is always called with y == 1 for the first
>>>> time.
>>>> + // In this case, we can only advise the developer to add assertions
>>>> + // for suppressing such path.
>>>> + *x; // expected-warning{{Dereference of null pointer}}
>>>> + } else {
>>>> + x = malloc(1);
>>>> + }
>>>> +}
>>>>
>>>>
>>>> _______________________________________________
>>>> 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