This revision was automatically updated to reflect the committed changes.
Closed by commit rL293043: [analyzer] Fix MacOSXAPIChecker fp with static
locals seen from nested blocks. (authored by dergachev).
Changed prior to commit:
https://reviews.llvm.org/D28946?vs=85588&id=85722#toc
Repository:
rL LLVM
https://reviews.llvm.org/D28946
Files:
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
cfe/trunk/test/Analysis/null-deref-static.m
Index: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1849,6 +1849,8 @@
// 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);
Index: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -816,9 +816,11 @@
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))
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
@@ -94,11 +94,18 @@
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 '";
Index: cfe/trunk/test/Analysis/null-deref-static.m
===================================================================
--- cfe/trunk/test/Analysis/null-deref-static.m
+++ cfe/trunk/test/Analysis/null-deref-static.m
@@ -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);
+ }
+}
Index: cfe/trunk/test/Analysis/dispatch-once.m
===================================================================
--- cfe/trunk/test/Analysis/dispatch-once.m
+++ cfe/trunk/test/Analysis/dispatch-once.m
@@ -107,3 +107,10 @@
};
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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
