[cfe-commits] r162876 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp test/Analysis/inlining/RetainCountExamples.m
Anna Zaks
ganna at apple.com
Wed Aug 29 16:23:43 PDT 2012
Author: zaks
Date: Wed Aug 29 18:23:43 2012
New Revision: 162876
URL: http://llvm.org/viewvc/llvm-project?rev=162876&view=rev
Log:
[analyzer] Stop tracking symbols based on a retain count summary of
inlined function.
This resolves retain count checker false positives that are caused by
inlining ObjC and other methods. Essentially, if we are passing an
object to a method with "delegate" in the selector or a function pointer
as another argument, we should stop tracking the other parameters/return
value as far as the retain count checker is concerned.
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
cfe/trunk/test/Analysis/inlining/RetainCountExamples.m
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=162876&r1=162875&r2=162876&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Wed Aug 29 18:23:43 2012
@@ -48,9 +48,23 @@
/// 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 function is inlined.
+ StopTrackingHard,
+
+ // The function decrements the reference count and the checker
+ // should stop tracking the argument.
+ DecRefAndStopTrackingHard, DecRefMsgAndStopTrackingHard
+ };
namespace llvm {
template <> struct FoldingSetTrait<ArgEffect> {
@@ -72,7 +86,13 @@
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 summary indirectly produced
+ // by inlining the function
+ NoRetHard
+ };
enum ObjKind { CF, ObjC, AnyObj };
@@ -115,6 +135,9 @@
static RetEffect MakeNoRet() {
return RetEffect(NoRet);
}
+ static RetEffect MakeNoRetHard() {
+ return RetEffect(NoRetHard);
+ }
void Profile(llvm::FoldingSetNodeID& ID) const {
ID.AddInteger((unsigned) K);
@@ -875,7 +898,7 @@
return FName.find("MakeCollectable") != StringRef::npos;
}
-static ArgEffect getStopTrackingEquivalent(ArgEffect E) {
+static ArgEffect getStopTrackingHardEquivalent(ArgEffect E) {
switch (E) {
case DoNothing:
case Autorelease:
@@ -886,13 +909,14 @@
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;
}
@@ -903,19 +927,21 @@
void RetainSummaryManager::updateSummaryForCall(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
@@ -1436,9 +1462,9 @@
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;
}
}
}
@@ -2469,6 +2495,10 @@
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,
@@ -2494,8 +2524,8 @@
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,
@@ -2679,11 +2709,13 @@
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);
}
@@ -2715,6 +2747,46 @@
return RetTy;
}
+// We don't always get the exact 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) {
+ if (Summ.getArg(idx) == StopTrackingHard) {
+ SVal V = CallOrMsg.getArgSVal(idx);
+ if (SymbolRef Sym = V.getAsLocSymbol()) {
+ state = removeRefBinding(state, Sym);
+ }
+ }
+ }
+
+ // Evaluate the effect on the message receiver.
+ const ObjCMethodCall *MsgInvocation = dyn_cast<ObjCMethodCall>(&CallOrMsg);
+ if (MsgInvocation) {
+ if (SymbolRef Sym = MsgInvocation->getReceiverSVal().getAsLocSymbol()) {
+ if (Summ.getReceiverEffect() == StopTrackingHard) {
+ state = removeRefBinding(state, Sym);
+ }
+ }
+ }
+
+ // 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)
+ state = removeRefBinding(state, Sym);
+ }
+
+ C.addTransition(state);
+}
+
void RetainCountChecker::checkSummary(const RetainSummary &Summ,
const CallEvent &CallOrMsg,
CheckerContext &C) const {
@@ -2749,7 +2821,7 @@
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;
@@ -2780,6 +2852,7 @@
llvm_unreachable("Unhandled RetEffect.");
case RetEffect::NoRet:
+ case RetEffect::NoRetHard:
// No work necessary.
break;
@@ -2858,8 +2931,8 @@
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;
@@ -2880,7 +2953,7 @@
case DecRefMsg:
case IncRefMsg:
case MakeCollectable:
- case DecRefMsgAndStopTracking:
+ case DecRefMsgAndStopTrackingHard:
llvm_unreachable("DecRefMsg/IncRefMsg/MakeCollectable already converted");
case Dealloc:
@@ -2929,6 +3002,7 @@
break;
case StopTracking:
+ case StopTrackingHard:
return removeRefBinding(state, sym);
case IncRef:
@@ -2949,7 +3023,7 @@
case DecRef:
case DecRefBridgedTransfered:
- case DecRefAndStopTracking:
+ case DecRefAndStopTrackingHard:
switch (V.getKind()) {
default:
// case 'RefVal::Released' handled above.
@@ -2960,7 +3034,7 @@
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;
@@ -2968,7 +3042,7 @@
case RefVal::NotOwned:
if (V.getCount() > 0) {
- if (E == DecRefAndStopTracking)
+ if (E == DecRefAndStopTrackingHard)
return removeRefBinding(state, sym);
V = V - 1;
} else {
Modified: cfe/trunk/test/Analysis/inlining/RetainCountExamples.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/RetainCountExamples.m?rev=162876&r1=162875&r2=162876&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inlining/RetainCountExamples.m (original)
+++ cfe/trunk/test/Analysis/inlining/RetainCountExamples.m Wed Aug 29 18:23:43 2012
@@ -15,6 +15,7 @@
-(id)copy;
- (Class)class;
-(id)retain;
+- (oneway void)release;
@end
@interface SelfStaysLive : NSObject
@@ -62,4 +63,65 @@
Cell *sharedCell4 = [[Cell alloc] initWithInt: 3]; // expected-warning {{leak}}
}
@end
+
+// We should stop tracking some objects even when we inline the call.
+// Specialically, the objects passed into calls with delegate and callback
+// parameters.
+ at class DelegateTest;
+typedef void (*ReleaseCallbackTy) (DelegateTest *c);
+
+ at interface Delegate : NSObject
+ at end
+
+ at interface DelegateTest : NSObject {
+ Delegate *myDel;
+}
+// Object initialized with a delagate which could potentially release it.
+- (id)initWithDelegate: (id) d;
+
+- (void) setDelegate: (id) d;
+
+// Releases object through callback.
++ (void)updateObject:(DelegateTest*)obj WithCallback:(ReleaseCallbackTy)rc;
+
++ (void)test: (Delegate *)d;
+
+ at property (assign) Delegate* myDel;
+ at end
+
+void releaseObj(DelegateTest *c);
+
+// Releases object through callback.
+void updateObject(DelegateTest *c, ReleaseCallbackTy rel) {
+ rel(c);
+}
+
+ at implementation DelegateTest
+ at synthesize myDel;
+
+- (id) initWithDelegate: (id) d {
+ if ((self = [super init]))
+ myDel = d;
+ return self;
+}
+
+- (void) setDelegate: (id) d {
+ myDel = d;
+}
+
++ (void)updateObject:(DelegateTest*)obj WithCallback:(ReleaseCallbackTy)rc {
+ rc(obj);
+}
+
++ (void) test: (Delegate *)d {
+ DelegateTest *obj1 = [[DelegateTest alloc] initWithDelegate: d]; // no-warning
+ DelegateTest *obj2 = [[DelegateTest alloc] init]; // no-warning
+ DelegateTest *obj3 = [[DelegateTest alloc] init]; // no-warning
+ updateObject(obj2, releaseObj);
+ [DelegateTest updateObject: obj3
+ WithCallback: releaseObj];
+ DelegateTest *obj4 = [[DelegateTest alloc] init]; // no-warning
+ [obj4 setDelegate: d];
+}
+ at end
More information about the cfe-commits
mailing list