aboutsummaryrefslogtreecommitdiff
path: root/lib/Sema/Sema.cpp
diff options
context:
space:
mode:
authorJordan Rose <jordan_rose@apple.com>2012-09-28 22:21:30 +0000
committerJordan Rose <jordan_rose@apple.com>2012-09-28 22:21:30 +0000
commit58b6bdcdeb683a3504f2248a409e1f4e85876cee (patch)
treea561b1f8661b4089411d8327833fa00999cedd34 /lib/Sema/Sema.cpp
parentb22c7dc707cf3770ff3b5e5f11f11fd0aaa06d9b (diff)
Add a warning (off by default) for repeated use of the same weak property.
The motivating example: if (self.weakProp) use(self.weakProp); As with any non-atomic test-then-use, it is possible a weak property to be non-nil at the 'if', but be deallocated by the time it is used. The correct way to write this example is as follows: id tmp = self.weakProp; if (tmp) use(tmp); The warning is controlled by -Warc-repeated-use-of-receiver, and uses the property name and base to determine if the same property on the same object is being accessed multiple times. In cases where the base is more complicated than just a single Decl (e.g. 'foo.bar.weakProp'), it picks a Decl for some degree of uniquing and reports the problem under a subflag, -Warc-maybe-repeated-use-of-receiver. This gives a way to tune the aggressiveness of the warning for a particular project. The warning is not on by default because it is not flow-sensitive and thus may have a higher-than-acceptable rate of false positives, though it is less noisy than -Wreceiver-is-weak. On the other hand, it will not warn about some cases that may be legitimate issues that -Wreceiver-is-weak will catch, and it does not attempt to reason about methods returning weak values. Even though this is not a real "analysis-based" check I've put the bug emission code in AnalysisBasedWarnings for two reasons: (1) to run on every kind of code body (function, method, block, or lambda), and (2) to suggest that it may be enhanced by flow-sensitive analysis in the future. The second (smaller) half of this work is to extend it to weak locals and weak ivars. This should use most of the same infrastructure. Part of <rdar://problem/12280249> git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@164854 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'lib/Sema/Sema.cpp')
-rw-r--r--lib/Sema/Sema.cpp126
1 files changed, 126 insertions, 0 deletions
diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index 08ccfa4259..84cdb2b6f0 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -54,6 +54,132 @@ void FunctionScopeInfo::Clear() {
Returns.clear();
ErrorTrap.reset();
PossiblyUnreachableDiags.clear();
+ WeakObjectUses.clear();
+}
+
+static const NamedDecl *getBestPropertyDecl(const ObjCPropertyRefExpr *PropE) {
+ if (PropE->isExplicitProperty())
+ return PropE->getExplicitProperty();
+
+ return PropE->getImplicitPropertyGetter();
+}
+
+static bool isSelfExpr(const Expr *E) {
+ E = E->IgnoreParenImpCasts();
+
+ const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E);
+ if (!DRE)
+ return false;
+
+ const ImplicitParamDecl *Param = dyn_cast<ImplicitParamDecl>(DRE->getDecl());
+ if (!Param)
+ return false;
+
+ const ObjCMethodDecl *M = dyn_cast<ObjCMethodDecl>(Param->getDeclContext());
+ if (!M)
+ return false;
+
+ return M->getSelfDecl() == Param;
+}
+
+FunctionScopeInfo::WeakObjectProfileTy::WeakObjectProfileTy(
+ const ObjCPropertyRefExpr *PropE)
+ : Base(0, false), Property(getBestPropertyDecl(PropE)) {
+
+ if (PropE->isObjectReceiver()) {
+ const OpaqueValueExpr *OVE = cast<OpaqueValueExpr>(PropE->getBase());
+ const Expr *E = OVE->getSourceExpr()->IgnoreParenCasts();
+
+ switch (E->getStmtClass()) {
+ case Stmt::DeclRefExprClass:
+ Base.setPointer(cast<DeclRefExpr>(E)->getDecl());
+ Base.setInt(isa<VarDecl>(Base.getPointer()));
+ break;
+ case Stmt::MemberExprClass: {
+ const MemberExpr *ME = cast<MemberExpr>(E);
+ Base.setPointer(ME->getMemberDecl());
+ Base.setInt(isa<CXXThisExpr>(ME->getBase()->IgnoreParenImpCasts()));
+ break;
+ }
+ case Stmt::ObjCIvarRefExprClass: {
+ const ObjCIvarRefExpr *IE = cast<ObjCIvarRefExpr>(E);
+ Base.setPointer(IE->getDecl());
+ if (isSelfExpr(IE->getBase()))
+ Base.setInt(true);
+ break;
+ }
+ case Stmt::PseudoObjectExprClass: {
+ const PseudoObjectExpr *POE = cast<PseudoObjectExpr>(E);
+ const ObjCPropertyRefExpr *BaseProp =
+ dyn_cast<ObjCPropertyRefExpr>(POE->getSyntacticForm());
+ if (BaseProp) {
+ Base.setPointer(getBestPropertyDecl(BaseProp));
+
+ const Expr *DoubleBase = BaseProp->getBase();
+ if (const OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(DoubleBase))
+ DoubleBase = OVE->getSourceExpr();
+
+ if (isSelfExpr(DoubleBase))
+ Base.setInt(true);
+ }
+ break;
+ }
+ default:
+ break;
+ }
+ } else if (PropE->isClassReceiver()) {
+ Base.setPointer(PropE->getClassReceiver());
+ Base.setInt(true);
+ } else {
+ assert(PropE->isSuperReceiver());
+ Base.setInt(true);
+ }
+}
+
+void FunctionScopeInfo::recordUseOfWeak(const ObjCPropertyRefExpr *RefExpr) {
+ assert(RefExpr);
+ WeakUseVector &Uses =
+ WeakObjectUses[FunctionScopeInfo::WeakObjectProfileTy(RefExpr)];
+ Uses.push_back(WeakUseTy(RefExpr, RefExpr->isMessagingGetter()));
+}
+
+void FunctionScopeInfo::markSafeWeakUse(const Expr *E) {
+ E = E->IgnoreParenImpCasts();
+
+ if (const PseudoObjectExpr *POE = dyn_cast<PseudoObjectExpr>(E)) {
+ markSafeWeakUse(POE->getSyntacticForm());
+ return;
+ }
+
+ if (const ConditionalOperator *Cond = dyn_cast<ConditionalOperator>(E)) {
+ markSafeWeakUse(Cond->getTrueExpr());
+ markSafeWeakUse(Cond->getFalseExpr());
+ return;
+ }
+
+ if (const BinaryConditionalOperator *Cond =
+ dyn_cast<BinaryConditionalOperator>(E)) {
+ markSafeWeakUse(Cond->getCommon());
+ markSafeWeakUse(Cond->getFalseExpr());
+ return;
+ }
+
+ if (const ObjCPropertyRefExpr *RefExpr = dyn_cast<ObjCPropertyRefExpr>(E)) {
+ // Has this property been seen as a weak property?
+ FunctionScopeInfo::WeakObjectUseMap::iterator Uses =
+ WeakObjectUses.find(FunctionScopeInfo::WeakObjectProfileTy(RefExpr));
+ if (Uses == WeakObjectUses.end())
+ return;
+
+ // Has there been a read from the property using this Expr?
+ FunctionScopeInfo::WeakUseVector::reverse_iterator ThisUse =
+ std::find(Uses->second.rbegin(), Uses->second.rend(), WeakUseTy(E, true));
+ if (ThisUse == Uses->second.rend())
+ return;
+
+ ThisUse->markSafe();
+ return;
+ }
}
BlockScopeInfo::~BlockScopeInfo() { }