[analyzer] Make analyzer less aggressive when dealing with [self init].
With inlining, retain count checker starts tracking 'self' through the
init methods. The analyser results were too noisy if the developer
did not follow 'self = [super init]' pattern (which is common
especially in older code bases) - we reported self init anti-pattern AND
possible use-after-free. This patch teaches the retain count
checker to assume that [super init] does not fail when it's not consumed
by another expression. This silences the retain count warning that warns
about possibility of use-after-free when init fails, while preserving
all the other checking on 'self'.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@162508 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index c78db52..367d392 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -153,16 +153,6 @@
: State(Original.State), LCtx(Original.LCtx), Origin(Original.Origin),
Data(Original.Data), Location(Original.Location), RefCount(0) {}
-
- ProgramStateRef getState() const {
- return State;
- }
-
- const LocationContext *getLocationContext() const {
- return LCtx;
- }
-
-
/// Copies this CallEvent, with vtable intact, into a new block of memory.
virtual void cloneTo(void *Dest) const = 0;
@@ -192,6 +182,16 @@
return Origin.dyn_cast<const Decl *>();
}
+ /// \brief The state in which the call is being evaluated.
+ ProgramStateRef getState() const {
+ return State;
+ }
+
+ /// \brief The context in which the call is being evaluated.
+ const LocationContext *getLocationContext() const {
+ return LCtx;
+ }
+
/// \brief Returns the definition of the function or method that will be
/// called.
virtual RuntimeDefinition getRuntimeDefinition() const = 0;
@@ -810,6 +810,9 @@
/// \brief Returns the value of the receiver at the time of this call.
SVal getReceiverSVal() const;
+ /// \brief Return the value of 'self' if available.
+ SVal getSelfSVal() const;
+
/// \brief Get the interface for the receiver.
///
/// This works whether this is an instance message or a class message.
@@ -818,6 +821,9 @@
return getOriginExpr()->getReceiverInterface();
}
+ /// \brief Checks if the receiver refers to 'self' or 'super'.
+ bool isReceiverSelfOrSuper() const;
+
/// Returns how the message was written in the source (property access,
/// subscript, or explicit message send).
ObjCMessageKind getMessageKind() const;
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
index 03956bb..ab8acb1 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -930,6 +930,35 @@
S = getPersistentSummary(RE, RecEffect, DefEffect);
}
+
+ // Special case '[super init];' and '[self init];'
+ //
+ // Even though calling '[super init]' without assigning the result to self
+ // and checking if the parent returns 'nil' is a bad pattern, it is common.
+ // Additionally, our Self Init checker already warns about it. To avoid
+ // overwhelming the user with messages from both checkers, we model the case
+ // of '[super init]' in cases when it is not consumed by another expression
+ // as if the call preserves the value of 'self'; essentially, assuming it can
+ // never fail and return 'nil'.
+ // Note, we don't want to just stop tracking the value since we want the
+ // RetainCount checker to report leaks and use-after-free if SelfInit checker
+ // is turned off.
+ if (const ObjCMethodCall *MC = dyn_cast<ObjCMethodCall>(&Call)) {
+ if (MC->getMethodFamily() == OMF_init && MC->isReceiverSelfOrSuper()) {
+
+ // Check if the message is not consumed, we know it will not be used in
+ // an assignment, ex: "self = [super init]".
+ const Expr *ME = MC->getOriginExpr();
+ const LocationContext *LCtx = MC->getLocationContext();
+ ParentMap &PM = LCtx->getAnalysisDeclContext()->getParentMap();
+ if (!PM.isConsumedExpr(ME)) {
+ RetainSummaryTemplate ModifiableSummaryTemplate(S, *this);
+ ModifiableSummaryTemplate->setReceiverEffect(DoNothing);
+ ModifiableSummaryTemplate->setRetEffect(RetEffect::MakeNoRet());
+ }
+ }
+
+ }
}
const RetainSummary *
diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp
index 5345bd5..d4ee841 100644
--- a/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -574,6 +574,14 @@
return D->getResultType();
}
+SVal ObjCMethodCall::getSelfSVal() const {
+ const LocationContext *LCtx = getLocationContext();
+ const ImplicitParamDecl *SelfDecl = LCtx->getSelfDecl();
+ if (!SelfDecl)
+ return SVal();
+ return getState()->getSVal(getState()->getRegion(SelfDecl, LCtx));
+}
+
SVal ObjCMethodCall::getReceiverSVal() const {
// FIXME: Is this the best way to handle class receivers?
if (!isInstanceMessage())
@@ -584,10 +592,23 @@
// An instance message with no expression means we are sending to super.
// In this case the object reference is the same as 'self'.
- const LocationContext *LCtx = getLocationContext();
- const ImplicitParamDecl *SelfDecl = LCtx->getSelfDecl();
- assert(SelfDecl && "No message receiver Expr, but not in an ObjC method");
- return getState()->getSVal(getState()->getRegion(SelfDecl, LCtx));
+ assert(getOriginExpr()->getReceiverKind() == ObjCMessageExpr::SuperInstance);
+ SVal SelfVal = getSelfSVal();
+ assert(SelfVal.isValid() && "Calling super but not in ObjC method");
+ return SelfVal;
+}
+
+bool ObjCMethodCall::isReceiverSelfOrSuper() const {
+ if (getOriginExpr()->getReceiverKind() == ObjCMessageExpr::SuperInstance ||
+ getOriginExpr()->getReceiverKind() == ObjCMessageExpr::SuperClass)
+ return true;
+
+ if (!isInstanceMessage())
+ return false;
+
+ SVal RecVal = getSVal(getOriginExpr()->getInstanceReceiver());
+
+ return (RecVal == getSelfSVal());
}
SourceRange ObjCMethodCall::getSourceRange() const {
diff --git a/test/Analysis/inlining/RetainCountExamples.m b/test/Analysis/inlining/RetainCountExamples.m
index 2b682c2..c48e7b0 100644
--- a/test/Analysis/inlining/RetainCountExamples.m
+++ b/test/Analysis/inlining/RetainCountExamples.m
@@ -30,4 +30,36 @@
void selfStaysLive() {
SelfStaysLive *foo = [[SelfStaysLive alloc] init];
[foo release];
-}
\ No newline at end of file
+}
+
+// Test that retain release checker warns on leaks and use-after-frees when
+// self init is not enabled.
+// radar://12115830
+@interface ParentOfCell : NSObject
+- (id)initWithInt: (int)inInt;
+@end
+@interface Cell : ParentOfCell{
+ int x;
+}
+- (id)initWithInt: (int)inInt;
++ (void)testOverRelease;
++ (void)testLeak;
+@property int x;
+@end
+@implementation Cell
+@synthesize x;
+- (id) initWithInt: (int)inInt {
+ [super initWithInt: inInt];
+ self.x = inInt; // no-warning
+ return self; // Self Init checker would produce a warning here.
+}
++ (void) testOverRelease {
+ Cell *sharedCell3 = [[Cell alloc] initWithInt: 3];
+ [sharedCell3 release];
+ [sharedCell3 release]; // expected-warning {{Reference-counted object is used after it is released}}
+}
++ (void) testLeak {
+ Cell *sharedCell4 = [[Cell alloc] initWithInt: 3]; // expected-warning {{leak}}
+}
+@end
+
diff --git a/test/Analysis/inlining/retain-count-self-init.m b/test/Analysis/inlining/retain-count-self-init.m
new file mode 100644
index 0000000..ee8dbe3
--- /dev/null
+++ b/test/Analysis/inlining/retain-count-self-init.m
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx.cocoa.RetainCount,osx.cocoa.SelfInit -analyzer-ipa=dynamic-bifurcate -verify %s
+
+typedef signed char BOOL;
+typedef struct objc_class *Class;
+typedef struct objc_object {
+ Class isa;
+} *id;
+@protocol NSObject - (BOOL)isEqual:(id)object; @end
+@interface NSObject <NSObject> {}
++(id)alloc;
++(id)new;
+- (oneway void)release;
+-(id)init;
+-(id)autorelease;
+-(id)copy;
+- (Class)class;
+-(id)retain;
+@end
+
+// We do not want to overhelm user with error messages in case they forgot to
+// assign to self and check that the result of [super init] is non-nil. So
+// stop tracking the receiver of init with respect to Retain Release checker.
+// radar://12115830
+@interface ParentOfCell : NSObject
+- (id)initWithInt: (int)inInt;
+@end
+@interface Cell : ParentOfCell{
+ int x;
+}
+- (id)init;
++ (void)test;
+@property int x;
+@end
+@implementation Cell
+@synthesize x;
+- (id) init {
+ [super init];
+ self.x = 3; // no-warning
+ return self; // expected-warning {{Returning 'self' while it is not set to the result of '[(super or self)}}
+}
+- (id) initWithInt: (int)inInt {
+ [super initWithInt: inInt];
+ self.x = inInt; // no-warning
+ return self; // expected-warning {{Returning 'self' while it is not set to the result of '[(super or self)}}
+}
+- (id) init2 {
+ [self init]; // The call [self init] is inlined. We will warn inside the inlined body.
+ self.x = 2; // no-warning
+ return self;
+}
+
+- (id) initWithIntGood: (int)inInt {
+ if (self = [super initWithInt: inInt]) {
+ self.x = inInt;
+ }
+ return self;
+}
++ (void) test {
+ Cell *sharedCell1 = [[Cell alloc] init];
+ [sharedCell1 release];
+ Cell *sharedCell2 = [[Cell alloc] initWithInt: 3];
+ [sharedCell2 release];
+ Cell *sharedCell3 = [[Cell alloc] initWithIntGood: 3];
+ [sharedCell3 release];
+}
+
+@end
+
