diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index 3c00d99..9e53ceb 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -66,9 +66,23 @@ public: /// particular argument. enum ArgEffect { DoNothing, Autorelease, Dealloc, DecRef, DecRefMsg, DecRefBridgedTransfered, - DecRefAndStopTracking, DecRefMsgAndStopTracking, IncRefMsg, IncRef, MakeCollectable, MayEscape, - NewAutoreleasePool, StopTracking }; + NewAutoreleasePool, + + // Stop tracking the argument - the effect of the call is + // unknown. + StopTracking, + + // In some cases, we obtain a better summary for this checker + // by looking at the call site than by inlining the function. + // Signifies that we should stop tracking the symbol even if + // the perfect summary is available. + StopTrackingHard, + + // The function decrements the reference count and we should + // stop tracking the argument. + DecRefAndStopTrackingHard, DecRefMsgAndStopTrackingHard + }; namespace llvm { template <> struct FoldingSetTrait { @@ -90,7 +104,13 @@ class RetEffect { public: enum Kind { NoRet, OwnedSymbol, OwnedAllocatedSymbol, NotOwnedSymbol, GCNotOwnedSymbol, ARCNotOwnedSymbol, - OwnedWhenTrackedReceiver }; + OwnedWhenTrackedReceiver, + // Treat this function as returning a non-tracked symbol even if + // the function has been inlined. This is used where the call + // site summary is more presise than the inlining produced + // summary. + NoRetHard + }; enum ObjKind { CF, ObjC, AnyObj }; @@ -133,6 +153,9 @@ public: static RetEffect MakeNoRet() { return RetEffect(NoRet); } + static RetEffect MakeNoRetHard() { + return RetEffect(NoRetHard); + } void Profile(llvm::FoldingSetNodeID& ID) const { ID.AddInteger((unsigned) K); @@ -796,8 +819,8 @@ public: void updateSummaryFromAnnotations(const RetainSummary *&Summ, const FunctionDecl *FD); - void updateSummaryForCall(const RetainSummary *&Summ, - const CallEvent &Call); + void updateForCallbackArgument(const RetainSummary *&Summ, + const CallEvent &Call); bool isGCEnabled() const { return GCEnabled; } @@ -893,7 +916,7 @@ static bool isMakeCollectable(const FunctionDecl *FD, StringRef FName) { return FName.find("MakeCollectable") != StringRef::npos; } -static ArgEffect getStopTrackingEquivalent(ArgEffect E) { +static ArgEffect getStopTrackingHardEquivalent(ArgEffect E) { switch (E) { case DoNothing: case Autorelease: @@ -904,13 +927,14 @@ static ArgEffect getStopTrackingEquivalent(ArgEffect E) { case MayEscape: case NewAutoreleasePool: case StopTracking: - return StopTracking; + case StopTrackingHard: + return StopTrackingHard; case DecRef: - case DecRefAndStopTracking: - return DecRefAndStopTracking; + case DecRefAndStopTrackingHard: + return DecRefAndStopTrackingHard; case DecRefMsg: - case DecRefMsgAndStopTracking: - return DecRefMsgAndStopTracking; + case DecRefMsgAndStopTrackingHard: + return DecRefMsgAndStopTrackingHard; case Dealloc: return Dealloc; } @@ -918,22 +942,24 @@ static ArgEffect getStopTrackingEquivalent(ArgEffect E) { llvm_unreachable("Unknown ArgEffect kind"); } -void RetainSummaryManager::updateSummaryForCall(const RetainSummary *&S, - const CallEvent &Call) { +void RetainSummaryManager::updateForCallbackArgument(const RetainSummary *&S, + const CallEvent &Call) { if (Call.hasNonZeroCallbackArg()) { - ArgEffect RecEffect = getStopTrackingEquivalent(S->getReceiverEffect()); - ArgEffect DefEffect = getStopTrackingEquivalent(S->getDefaultArgEffect()); + ArgEffect RecEffect = + getStopTrackingHardEquivalent(S->getReceiverEffect()); + ArgEffect DefEffect = + getStopTrackingHardEquivalent(S->getDefaultArgEffect()); ArgEffects CustomArgEffects = S->getArgEffects(); for (ArgEffects::iterator I = CustomArgEffects.begin(), E = CustomArgEffects.end(); I != E; ++I) { - ArgEffect Translated = getStopTrackingEquivalent(I->second); + ArgEffect Translated = getStopTrackingHardEquivalent(I->second); if (Translated != DefEffect) ScratchArgs = AF.add(ScratchArgs, I->first, Translated); } - RetEffect RE = RetEffect::MakeNoRet(); + RetEffect RE = RetEffect::MakeNoRetHard(); // Special cases where the callback argument CANNOT free the return value. // This can generally only happen if we know that the callback will only be @@ -976,7 +1002,7 @@ RetainSummaryManager::getSummary(const CallEvent &Call, } } - updateSummaryForCall(Summ, Call); + updateForCallbackArgument(Summ, Call); assert(Summ && "Unknown call type?"); return Summ; @@ -1441,9 +1467,9 @@ RetainSummaryManager::getStandardMethodSummary(const ObjCMethodDecl *MD, StringRef Slot = S.getNameForSlot(i); if (Slot.substr(Slot.size() - 8).equals_lower("delegate")) { if (ResultEff == ObjCInitRetE) - ResultEff = RetEffect::MakeNoRet(); + ResultEff = RetEffect::MakeNoRetHard(); else - ReceiverEff = StopTracking; + ReceiverEff = StopTrackingHard; } } } @@ -2474,6 +2500,10 @@ public: void checkSummary(const RetainSummary &Summ, const CallEvent &Call, CheckerContext &C) const; + void processSummaryOfInlined(const RetainSummary &Summ, + const CallEvent &Call, + CheckerContext &C) const; + bool evalCall(const CallExpr *CE, CheckerContext &C) const; ProgramStateRef evalAssume(ProgramStateRef state, SVal Cond, @@ -2499,8 +2529,8 @@ public: void checkEndPath(CheckerContext &C) const; ProgramStateRef updateSymbol(ProgramStateRef state, SymbolRef sym, - RefVal V, ArgEffect E, RefVal::Kind &hasErr, - CheckerContext &C) const; + RefVal V, ArgEffect E, RefVal::Kind &hasErr, + CheckerContext &C) const; void processNonLeakError(ProgramStateRef St, SourceRange ErrorRange, RefVal::Kind ErrorKind, SymbolRef Sym, @@ -2685,11 +2715,13 @@ void RetainCountChecker::checkPostStmt(const ObjCBoxedExpr *Ex, void RetainCountChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { - if (C.wasInlined) - return; - RetainSummaryManager &Summaries = getSummaryManager(C); const RetainSummary *Summ = Summaries.getSummary(Call, C.getState()); + + if (C.wasInlined) { + processSummaryOfInlined(*Summ, Call, C); + return; + } checkSummary(*Summ, Call, C); } @@ -2721,6 +2753,49 @@ static QualType GetReturnType(const Expr *RetE, ASTContext &Ctx) { return RetTy; } +// We don't always get the perfect modeling of the function with regards to the +// retain count checker even when the function is inlined. For example, we need +// to stop tracking the symbols which were marked with StopTrackingHard. +void RetainCountChecker::processSummaryOfInlined(const RetainSummary &Summ, + const CallEvent &CallOrMsg, + CheckerContext &C) const { + ProgramStateRef state = C.getState(); + + // Evaluate the effect of the arguments. + for (unsigned idx = 0, e = CallOrMsg.getNumArgs(); idx != e; ++idx) { + SVal V = CallOrMsg.getArgSVal(idx); + if (SymbolRef Sym = V.getAsLocSymbol()) { + if (Summ.getArg(idx) == StopTrackingHard) { + state = removeRefBinding(state, Sym); + C.addTransition(state); + } + } + } + + // Evaluate the effect on the message receiver. + const ObjCMethodCall *MsgInvocation = dyn_cast(&CallOrMsg); + if (MsgInvocation) { + if (SymbolRef Sym = MsgInvocation->getReceiverSVal().getAsLocSymbol()) { + if (Summ.getReceiverEffect() == StopTrackingHard) { + state = removeRefBinding(state, Sym); + C.addTransition(state); + } + } + } + + // Consult the summary for the return value. + RetEffect RE = Summ.getRetEffect(); + if (RE.getKind() == RetEffect::NoRetHard) { + SymbolRef Sym = state->getSVal(CallOrMsg.getOriginExpr(), + C.getLocationContext()).getAsSymbol(); + if (!Sym) + return; + state = removeRefBinding(state, Sym); + C.addTransition(state); + + } +} + void RetainCountChecker::checkSummary(const RetainSummary &Summ, const CallEvent &CallOrMsg, CheckerContext &C) const { @@ -2755,7 +2830,7 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ, if (const RefVal *T = getRefBinding(state, Sym)) { ReceiverIsTracked = true; state = updateSymbol(state, Sym, *T, Summ.getReceiverEffect(), - hasErr, C); + hasErr, C); if (hasErr) { ErrorRange = MsgInvocation->getOriginExpr()->getReceiverRange(); ErrorSym = Sym; @@ -2786,6 +2861,7 @@ void RetainCountChecker::checkSummary(const RetainSummary &Summ, llvm_unreachable("Unhandled RetEffect."); case RetEffect::NoRet: + case RetEffect::NoRetHard: // No work necessary. break; @@ -2864,8 +2940,8 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, case DecRefMsg: E = IgnoreRetainMsg ? DoNothing : DecRef; break; - case DecRefMsgAndStopTracking: - E = IgnoreRetainMsg ? StopTracking : DecRefAndStopTracking; + case DecRefMsgAndStopTrackingHard: + E = IgnoreRetainMsg ? StopTracking : DecRefAndStopTrackingHard; break; case MakeCollectable: E = C.isObjCGCEnabled() ? DecRef : DoNothing; @@ -2886,7 +2962,7 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, case DecRefMsg: case IncRefMsg: case MakeCollectable: - case DecRefMsgAndStopTracking: + case DecRefMsgAndStopTrackingHard: llvm_unreachable("DecRefMsg/IncRefMsg/MakeCollectable already converted"); case Dealloc: @@ -2935,6 +3011,7 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, break; case StopTracking: + case StopTrackingHard: return removeRefBinding(state, sym); case IncRef: @@ -2955,7 +3032,7 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, case DecRef: case DecRefBridgedTransfered: - case DecRefAndStopTracking: + case DecRefAndStopTrackingHard: switch (V.getKind()) { default: // case 'RefVal::Released' handled above. @@ -2966,7 +3043,7 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, if (V.getCount() == 1) V = V ^ (E == DecRefBridgedTransfered ? RefVal::NotOwned : RefVal::Released); - else if (E == DecRefAndStopTracking) + else if (E == DecRefAndStopTrackingHard) return removeRefBinding(state, sym); V = V - 1; @@ -2974,7 +3051,7 @@ RetainCountChecker::updateSymbol(ProgramStateRef state, SymbolRef sym, case RefVal::NotOwned: if (V.getCount() > 0) { - if (E == DecRefAndStopTracking) + if (E == DecRefAndStopTrackingHard) return removeRefBinding(state, sym); V = V - 1; } else { diff --git a/test/Analysis/inlining/RetainCountExamples.m b/test/Analysis/inlining/RetainCountExamples.m index 2b682c2..7e53900 100644 --- a/test/Analysis/inlining/RetainCountExamples.m +++ b/test/Analysis/inlining/RetainCountExamples.m @@ -15,6 +15,7 @@ typedef struct objc_object { -(id)copy; - (Class)class; -(id)retain; +- (oneway void)release; @end @interface SelfStaysLive : NSObject @@ -30,4 +31,58 @@ typedef struct objc_object { void selfStaysLive() { SelfStaysLive *foo = [[SelfStaysLive alloc] init]; [foo release]; -} \ No newline at end of file +} + +// We should stop tracking some objects even when we inline the call. +// Specialically, the objects passed into calls with delegate and callback +// parameters. + +@class Cell; +typedef void (*ReleaseCallbackTy) (Cell *c); + +@interface Delegate : NSObject +@end + +@interface Cell : NSObject { + Delegate *myDel; +} +// Object initialized with a delagate which could potentially release it. +- (id)initWithDelegate: (id) d; +// Releases object through callback. ++ (void)updateObject:(Cell*)obj WithCallback:(ReleaseCallbackTy)rc; + ++ (void)test: (Delegate *)d; + +@property (assign) Delegate* myDel; +@end + +void releaseObj(Cell *c); + +// Releases object through callback. +void updateObject(Cell *c, ReleaseCallbackTy rel) { + rel(c); +} + +@implementation Cell +@synthesize myDel; + +- (id) initWithDelegate: (id) d { + if ((self = [super init])) + myDel = d; + return self; +} + ++ (void)updateObject:(Cell*)obj WithCallback:(ReleaseCallbackTy)rc { + rc(obj); +} + ++ (void) test: (Delegate *)d { + Cell *obj1 = [[Cell alloc] initWithDelegate: d]; // no-warning + Cell *obj2 = [[Cell alloc] init]; // no-warning + updateObject(obj2, releaseObj); + [Cell updateObject: obj2 + WithCallback: releaseObj]; + +} +@end +