[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 llvm-svn: 285605
This commit is contained in:
parent
849a6a5e5a
commit
aacc03c918
|
@ -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(CheckerContext &C, const CallExpr *CE,
|
|||
|
||||
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));
|
||||
|
|
|
@ -753,6 +753,12 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
|
|||
// 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))) ){
|
||||
|
|
|
@ -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.}}
|
||||
}
|
||||
|
||||
@interface Object : NSObject {
|
||||
@public
|
||||
dispatch_once_t once;
|
||||
Struct s;
|
||||
dispatch_once_t once_array[2];
|
||||
}
|
||||
- (void)test_ivar_from_inside;
|
||||
- (void)test_ivar_struct_from_inside;
|
||||
@end
|
||||
|
||||
@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.}}
|
||||
}
|
||||
@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.}}
|
||||
}
|
Loading…
Reference in New Issue