r285605 - [analyzer] MacOSXAPIChecker: Disallow dispatch_once_t in ivars and heap.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 31 10:27:26 PDT 2016
Author: dergachev
Date: Mon Oct 31 12:27:26 2016
New Revision: 285605
URL: http://llvm.org/viewvc/llvm-project?rev=285605&view=rev
Log:
[analyzer] MacOSXAPIChecker: Disallow dispatch_once_t in ivars and heap.
Unlike global/static variables, calloc etc. functions that allocate ObjC
objects behave differently in terms of memory barriers, and hacks that make
dispatch_once as fast as it possibly could be start failing.
Differential Revision: https://reviews.llvm.org/D25909
Added:
cfe/trunk/test/Analysis/dispatch-once.m
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp?rev=285605&r1=285604&r2=285605&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp Mon Oct 31 12:27:26 2016
@@ -33,6 +33,8 @@ namespace {
class MacOSXAPIChecker : public Checker< check::PreStmt<CallExpr> > {
mutable std::unique_ptr<BugType> BT_dispatchOnce;
+ static const ObjCIvarRegion *getParentIvarRegion(const MemRegion *R);
+
public:
void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
@@ -49,27 +51,34 @@ public:
// dispatch_once and dispatch_once_f
//===----------------------------------------------------------------------===//
+const ObjCIvarRegion *
+MacOSXAPIChecker::getParentIvarRegion(const MemRegion *R) {
+ const SubRegion *SR = dyn_cast<SubRegion>(R);
+ while (SR) {
+ if (const ObjCIvarRegion *IR = dyn_cast<ObjCIvarRegion>(SR))
+ return IR;
+ SR = dyn_cast<SubRegion>(SR->getSuperRegion());
+ }
+ return nullptr;
+}
+
void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE,
StringRef FName) const {
if (CE->getNumArgs() < 1)
return;
- // Check if the first argument is stack allocated. If so, issue a warning
- // because that's likely to be bad news.
- ProgramStateRef state = C.getState();
- const MemRegion *R =
- state->getSVal(CE->getArg(0), C.getLocationContext()).getAsRegion();
- if (!R || !isa<StackSpaceRegion>(R->getMemorySpace()))
+ // Check if the first argument is improperly allocated. If so, issue a
+ // warning because that's likely to be bad news.
+ const MemRegion *R = C.getSVal(CE->getArg(0)).getAsRegion();
+ if (!R)
return;
- ExplodedNode *N = C.generateErrorNode(state);
- if (!N)
+ // Global variables are fine.
+ const MemRegion *RB = R->getBaseRegion();
+ const MemSpaceRegion *RS = RB->getMemorySpace();
+ if (isa<GlobalsSpaceRegion>(RS))
return;
- if (!BT_dispatchOnce)
- BT_dispatchOnce.reset(new BugType(this, "Improper use of 'dispatch_once'",
- "API Misuse (Apple)"));
-
// Handle _dispatch_once. In some versions of the OS X SDK we have the case
// that dispatch_once is a macro that wraps a call to _dispatch_once.
// _dispatch_once is then a function which then calls the real dispatch_once.
@@ -82,16 +91,40 @@ void MacOSXAPIChecker::CheckDispatchOnce
SmallString<256> S;
llvm::raw_svector_ostream os(S);
+ bool SuggestStatic = false;
os << "Call to '" << FName << "' uses";
- if (const VarRegion *VR = dyn_cast<VarRegion>(R))
+ if (const VarRegion *VR = dyn_cast<VarRegion>(RB)) {
+ // We filtered out globals earlier, so it must be a local variable.
+ if (VR != R)
+ os << " memory within";
os << " the local variable '" << VR->getDecl()->getName() << '\'';
- else
+ SuggestStatic = true;
+ } else if (const ObjCIvarRegion *IVR = getParentIvarRegion(R)) {
+ if (IVR != R)
+ os << " memory within";
+ os << " the instance variable '" << IVR->getDecl()->getName() << '\'';
+ } else if (isa<HeapSpaceRegion>(RS)) {
+ os << " heap-allocated memory";
+ } else if (isa<UnknownSpaceRegion>(RS)) {
+ // Presence of an IVar superregion has priority over this branch, because
+ // ObjC objects are on the heap even if the core doesn't realize this.
+ return;
+ } else {
os << " stack allocated memory";
+ }
os << " for the predicate value. Using such transient memory for "
"the predicate is potentially dangerous.";
- if (isa<VarRegion>(R) && isa<StackLocalsSpaceRegion>(R->getMemorySpace()))
+ if (SuggestStatic)
os << " Perhaps you intended to declare the variable as 'static'?";
+ ExplodedNode *N = C.generateErrorNode();
+ if (!N)
+ return;
+
+ if (!BT_dispatchOnce)
+ BT_dispatchOnce.reset(new BugType(this, "Improper use of 'dispatch_once'",
+ "API Misuse (Apple)"));
+
auto report = llvm::make_unique<BugReport>(*BT_dispatchOnce, os.str(), N);
report->addRange(CE->getArg(0)->getSourceRange());
C.emitReport(std::move(report));
Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp?rev=285605&r1=285604&r2=285605&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Mon Oct 31 12:27:26 2016
@@ -753,6 +753,12 @@ SVal SimpleSValBuilder::evalBinOpLL(Prog
// Note, heap base symbolic regions are assumed to not alias with
// each other; for example, we assume that malloc returns different address
// on each invocation.
+ // FIXME: ObjC object pointers always reside on the heap, but currently
+ // we treat their memory space as unknown, because symbolic pointers
+ // to ObjC objects may alias. There should be a way to construct
+ // possibly-aliasing heap-based regions. For instance, MacOSXApiChecker
+ // guesses memory space for ObjC object pointers manually instead of
+ // relying on us.
if (LeftBase != RightBase &&
((!isa<SymbolicRegion>(LeftBase) && !isa<SymbolicRegion>(RightBase)) ||
(isa<HeapSpaceRegion>(LeftMS) || isa<HeapSpaceRegion>(RightMS))) ){
Added: cfe/trunk/test/Analysis/dispatch-once.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dispatch-once.m?rev=285605&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/dispatch-once.m (added)
+++ cfe/trunk/test/Analysis/dispatch-once.m Mon Oct 31 12:27:26 2016
@@ -0,0 +1,92 @@
+// RUN: %clang_cc1 -w -fblocks -analyze -analyzer-checker=core,osx.API,unix.Malloc -verify %s
+// RUN: %clang_cc1 -w -fblocks -fobjc-arc -analyze -analyzer-checker=core,osx.API,unix.Malloc -verify %s
+
+#include "Inputs/system-header-simulator-objc.h"
+
+typedef unsigned long size_t;
+void *calloc(size_t nmemb, size_t size);
+
+typedef void (^dispatch_block_t)(void);
+typedef long dispatch_once_t;
+void dispatch_once(dispatch_once_t *predicate, dispatch_block_t block);
+
+void test_stack() {
+ dispatch_once_t once;
+ dispatch_once(&once, ^{}); // expected-warning{{Call to 'dispatch_once' uses the local variable 'once' for the predicate value. Using such transient memory for the predicate is potentially dangerous. Perhaps you intended to declare the variable as 'static'?}}
+}
+
+void test_static_local() {
+ static dispatch_once_t once;
+ dispatch_once(&once, ^{}); // no-warning
+}
+
+void test_heap_var() {
+ dispatch_once_t *once = calloc(1, sizeof(dispatch_once_t));
+ // Use regexps to check that we're NOT suggesting to make this static.
+ dispatch_once(once, ^{}); // expected-warning-re{{{{^Call to 'dispatch_once' uses heap-allocated memory for the predicate value. Using such transient memory for the predicate is potentially dangerous$}}}}
+}
+
+void test_external_pointer(dispatch_once_t *once) {
+ // External pointer does not necessarily point to the heap.
+ dispatch_once(once, ^{}); // no-warning
+}
+
+typedef struct {
+ dispatch_once_t once;
+} Struct;
+
+void test_local_struct() {
+ Struct s;
+ dispatch_once(&s.once, ^{}); // expected-warning{{Call to 'dispatch_once' uses memory within the local variable 's' for the predicate value.}}
+}
+
+void test_heap_struct() {
+ Struct *s = calloc(1, sizeof(Struct));
+ dispatch_once(&s->once, ^{}); // expected-warning{{Call to 'dispatch_once' uses heap-allocated memory for the predicate value.}}
+}
+
+ at interface Object : NSObject {
+ at public
+ dispatch_once_t once;
+ Struct s;
+ dispatch_once_t once_array[2];
+}
+- (void)test_ivar_from_inside;
+- (void)test_ivar_struct_from_inside;
+ at end
+
+ at implementation Object
+- (void)test_ivar_from_inside {
+ dispatch_once(&once, ^{}); // expected-warning{{Call to 'dispatch_once' uses the instance variable 'once' for the predicate value.}}
+}
+- (void)test_ivar_struct_from_inside {
+ dispatch_once(&s.once, ^{}); // expected-warning{{Call to 'dispatch_once' uses memory within the instance variable 's' for the predicate value.}}
+}
+- (void)test_ivar_array_from_inside {
+ dispatch_once(&once_array[1], ^{}); // expected-warning{{Call to 'dispatch_once' uses memory within the instance variable 'once_array' for the predicate value.}}
+}
+ at end
+
+void test_ivar_from_alloc_init() {
+ Object *o = [[Object alloc] init];
+ dispatch_once(&o->once, ^{}); // expected-warning{{Call to 'dispatch_once' uses the instance variable 'once' for the predicate value.}}
+}
+void test_ivar_struct_from_alloc_init() {
+ Object *o = [[Object alloc] init];
+ dispatch_once(&o->s.once, ^{}); // expected-warning{{Call to 'dispatch_once' uses memory within the instance variable 's' for the predicate value.}}
+}
+void test_ivar_array_from_alloc_init() {
+ Object *o = [[Object alloc] init];
+ dispatch_once(&o->once_array[1], ^{}); // expected-warning{{Call to 'dispatch_once' uses memory within the instance variable 'once_array' for the predicate value.}}
+}
+
+void test_ivar_from_external_obj(Object *o) {
+ // ObjC object pointer always points to the heap.
+ dispatch_once(&o->once, ^{}); // expected-warning{{Call to 'dispatch_once' uses the instance variable 'once' for the predicate value.}}
+}
+void test_ivar_struct_from_external_obj(Object *o) {
+ dispatch_once(&o->s.once, ^{}); // expected-warning{{Call to 'dispatch_once' uses memory within the instance variable 's' for the predicate value.}}
+}
+void test_ivar_array_from_external_obj(Object *o) {
+ dispatch_once(&o->once_array[1], ^{}); // expected-warning{{Call to 'dispatch_once' uses memory within the instance variable 'once_array' for the predicate value.}}
+}
More information about the cfe-commits
mailing list