Fine with merging. Thank you! Anna. > On Feb 1, 2017, at 11:00 AM, Hans Wennborg <h...@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 <noqnoq...@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@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
