};
private:
- Kind kind;
- RetEffect::ObjKind okind;
+ /// The number of outstanding retains.
unsigned Cnt;
+ /// The number of outstanding autoreleases.
unsigned ACnt;
+ /// The (static) type of the object at the time we started tracking it.
QualType T;
- RefVal(Kind k, RetEffect::ObjKind o, unsigned cnt, unsigned acnt, QualType t)
- : kind(k), okind(o), Cnt(cnt), ACnt(acnt), T(t) {}
+ /// The current state of the object.
+ ///
+ /// See the RefVal::Kind enum for possible values.
+ unsigned RawKind : 5;
+
+ /// The kind of object being tracked (CF or ObjC), if known.
+ ///
+ /// See the RetEffect::ObjKind enum for possible values.
+ unsigned RawObjectKind : 2;
+
+ /// True if the current state and/or retain count may turn out to not be the
+ /// best possible approximation of the reference counting state.
+ ///
+ /// If true, the checker may decide to throw away ("override") this state
+ /// in favor of something else when it sees the object being used in new ways.
+ ///
+ /// This setting should not be propagated to state derived from this state.
+ /// Once we start deriving new states, it would be inconsistent to override
+ /// them.
+ unsigned IsOverridable : 1;
+
+ RefVal(Kind k, RetEffect::ObjKind o, unsigned cnt, unsigned acnt, QualType t,
+ bool Overridable = false)
+ : Cnt(cnt), ACnt(acnt), T(t), RawKind(static_cast<unsigned>(k)),
+ RawObjectKind(static_cast<unsigned>(o)), IsOverridable(Overridable) {
+ assert(getKind() == k && "not enough bits for the kind");
+ assert(getObjKind() == o && "not enough bits for the object kind");
+ }
public:
- Kind getKind() const { return kind; }
+ Kind getKind() const { return static_cast<Kind>(RawKind); }
- RetEffect::ObjKind getObjKind() const { return okind; }
+ RetEffect::ObjKind getObjKind() const {
+ return static_cast<RetEffect::ObjKind>(RawObjectKind);
+ }
unsigned getCount() const { return Cnt; }
unsigned getAutoreleaseCount() const { return ACnt; }
unsigned getCombinedCounts() const { return Cnt + ACnt; }
- void clearCounts() { Cnt = 0; ACnt = 0; }
- void setCount(unsigned i) { Cnt = i; }
- void setAutoreleaseCount(unsigned i) { ACnt = i; }
+ void clearCounts() {
+ Cnt = 0;
+ ACnt = 0;
+ IsOverridable = false;
+ }
+ void setCount(unsigned i) {
+ Cnt = i;
+ IsOverridable = false;
+ }
+ void setAutoreleaseCount(unsigned i) {
+ ACnt = i;
+ IsOverridable = false;
+ }
QualType getType() const { return T; }
+ bool isOverridable() const { return IsOverridable; }
+
bool isOwned() const {
return getKind() == Owned;
}
return getKind() == ReturnedNotOwned;
}
+ /// Create a state for an object whose lifetime is the responsibility of the
+ /// current function, at least partially.
+ ///
+ /// Most commonly, this is an owned object with a retain count of +1.
static RefVal makeOwned(RetEffect::ObjKind o, QualType t,
unsigned Count = 1) {
return RefVal(Owned, o, Count, 0, t);
}
+ /// Create a state for an object whose lifetime is not the responsibility of
+ /// the current function.
+ ///
+ /// Most commonly, this is an unowned object with a retain count of +0.
static RefVal makeNotOwned(RetEffect::ObjKind o, QualType t,
unsigned Count = 0) {
return RefVal(NotOwned, o, Count, 0, t);
}
- // Comparison, profiling, and pretty-printing.
-
- bool operator==(const RefVal& X) const {
- return kind == X.kind && Cnt == X.Cnt && T == X.T && ACnt == X.ACnt;
+ /// Create an "overridable" state for an unowned object at +0.
+ ///
+ /// An overridable state is one that provides a good approximation of the
+ /// reference counting state now, but which may be discarded later if the
+ /// checker sees the object being used in new ways.
+ static RefVal makeOverridableNotOwned(RetEffect::ObjKind o, QualType t) {
+ return RefVal(NotOwned, o, 0, 0, t, /*Overridable=*/true);
}
RefVal operator-(size_t i) const {
getType());
}
+ // Comparison, profiling, and pretty-printing.
+
+ bool hasSameState(const RefVal &X) const {
+ return getKind() == X.getKind() && Cnt == X.Cnt && ACnt == X.ACnt;
+ }
+
+ bool operator==(const RefVal& X) const {
+ return T == X.T && hasSameState(X) && getObjKind() == X.getObjKind() &&
+ IsOverridable == X.IsOverridable;
+ }
+
void Profile(llvm::FoldingSetNodeID& ID) const {
- ID.AddInteger((unsigned) kind);
+ ID.Add(T);
+ ID.AddInteger(RawKind);
ID.AddInteger(Cnt);
ID.AddInteger(ACnt);
- ID.Add(T);
+ ID.AddInteger(RawObjectKind);
+ ID.AddBoolean(IsOverridable);
}
void print(raw_ostream &Out) const;
if (!T.isNull())
Out << "Tracked " << T.getAsString() << '/';
+ if (isOverridable())
+ Out << "(overridable) ";
+
switch (getKind()) {
default: llvm_unreachable("Invalid RefVal kind");
case Owned: {
if (!GCEnabled && std::find(AEffects.begin(), AEffects.end(), Dealloc) !=
AEffects.end()) {
// Determine if the object's reference count was pushed to zero.
- assert(!(PrevV == CurrV) && "The typestate *must* have changed.");
+ assert(!PrevV.hasSameState(CurrV) && "The state should have changed.");
// We may not have transitioned to 'release' if we hit an error.
// This case is handled elsewhere.
if (CurrV.getKind() == RefVal::Released) {
if (GCEnabled) {
// Determine if the object's reference count was pushed to zero.
- assert(!(PrevV == CurrV) && "The typestate *must* have changed.");
+ assert(!PrevV.hasSameState(CurrV) && "The state should have changed.");
os << "In GC mode a call to '" << *FD
<< "' decrements an object's retain count and registers the "
}
// Determine if the typestate has changed.
- if (!(PrevV == CurrV))
+ if (!PrevV.hasSameState(CurrV))
switch (CurrV.getKind()) {
case RefVal::Owned:
case RefVal::NotOwned:
check::PostStmt<ObjCArrayLiteral>,
check::PostStmt<ObjCDictionaryLiteral>,
check::PostStmt<ObjCBoxedExpr>,
+ check::PostStmt<ObjCIvarRefExpr>,
check::PostCall,
check::PreStmt<ReturnStmt>,
check::RegionChanges,
void checkPostStmt(const ObjCDictionaryLiteral *DL, CheckerContext &C) const;
void checkPostStmt(const ObjCBoxedExpr *BE, CheckerContext &C) const;
+ void checkPostStmt(const ObjCIvarRefExpr *IRE, CheckerContext &C) const;
+
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
void checkSummary(const RetainSummary &Summ, const CallEvent &Call,
C.addTransition(State);
}
+void RetainCountChecker::checkPostStmt(const ObjCIvarRefExpr *IRE,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ // If an instance variable was previously accessed through a property,
+ // it may have a synthesized refcount of +0. Override right now that we're
+ // doing direct access.
+ if (Optional<Loc> IVarLoc = C.getSVal(IRE).getAs<Loc>())
+ if (SymbolRef Sym = State->getSVal(*IVarLoc).getAsSymbol())
+ if (const RefVal *RV = getRefBinding(State, Sym))
+ if (RV->isOverridable())
+ State = removeRefBinding(State, Sym);
+ C.addTransition(State);
+}
+
void RetainCountChecker::checkPostCall(const CallEvent &Call,
CheckerContext &C) const {
RetainSummaryManager &Summaries = getSummaryManager(C);
state = removeRefBinding(state, Sym);
} else if (RE.getKind() == RetEffect::NotOwnedSymbol) {
if (wasSynthesizedProperty(MsgInvocation, C.getPredecessor())) {
- // Believe the summary if we synthesized the body and the return value is
- // untracked. This handles property getters.
+ // Believe the summary if we synthesized the body of a property getter
+ // and the return value is currently untracked. If the corresponding
+ // instance variable is later accessed directly, however, we're going to
+ // want to override this state, so that the owning object can perform
+ // reference counting operations on its own ivars.
SymbolRef Sym = CallOrMsg.getReturnValue().getAsSymbol();
if (Sym && !getRefBinding(state, Sym))
- state = setRefBinding(state, Sym, RefVal::makeNotOwned(RE.getObjKind(),
- Sym->getType()));
+ state = setRefBinding(state, Sym,
+ RefVal::makeOverridableNotOwned(RE.getObjKind(),
+ Sym->getType()));
}
}
// Property accessor synthesis
//------
+extern void doSomethingWithPerson(Person *p);
+extern void doSomethingWithName(NSString *name);
+
void testConsistencyRetain(Person *p) {
clang_analyzer_eval(p.name == p.name); // expected-warning{{TRUE}}
- extern void doSomethingWithPerson(Person *p);
id origName = p.name;
clang_analyzer_eval(p.name == origName); // expected-warning{{TRUE}}
doSomethingWithPerson(p);
void testConsistencyAssign(Person *p) {
clang_analyzer_eval(p.friend == p.friend); // expected-warning{{TRUE}}
- extern void doSomethingWithPerson(Person *p);
id origFriend = p.friend;
clang_analyzer_eval(p.friend == origFriend); // expected-warning{{TRUE}}
doSomethingWithPerson(p);
#if !__has_feature(objc_arc)
void testOverrelease(Person *p, int coin) {
- if (coin)
+ switch (coin) {
+ case 0:
[p.name release]; // expected-warning{{not owned}}
- else
+ break;
+ case 1:
[p.friend release]; // expected-warning{{not owned}}
+ break;
+ case 2: {
+ id friend = p.friend;
+ doSomethingWithPerson(p);
+ [friend release]; // expected-warning{{not owned}}
+ }
+ }
+}
+
+// <rdar://problem/16333368>
+@implementation Person (Rdar16333368)
+
+- (void)testDeliberateRelease:(Person *)other {
+ doSomethingWithName(self.name);
+ [_name release]; // no-warning
+ self->_name = 0;
+
+ doSomethingWithName(other->_name);
+ [other.name release]; // expected-warning{{not owned}}
}
+
+- (void)deliberateReleaseFalseNegative {
+ // This is arguably a false negative because the result of p.friend shouldn't
+ // be released, even though we are manipulating the ivar in between the two
+ // actions.
+ id name = self.name;
+ _name = 0;
+ [name release];
+}
+
+- (void)testRetainAndRelease {
+ [self.name retain];
+ [self.name release];
+ [self.name release]; // expected-warning{{not owned}}
+}
+
+- (void)testRetainAndReleaseIVar {
+ [self.name retain];
+ [_name release];
+ [_name release]; // expected-warning{{not owned}}
+}
+
+@end
#endif
@interface IntWrapper
projects / clang / commitdiff