NoQ created this revision.
NoQ added reviewers: zaks.anna, dcoughlin.
NoQ added a subscriber: cfe-commits.

This patch applies the bug reporting technique that is being introduced in 
D24278 to ObjCDeallocChecker. Extra notes highlight instance variable or 
property declarations that are of relevance to the bug report. Extra notes are 
separate from bug reports.

Originally D24278 contained this patch, but then it was decided to make a 
separate review. The original diff is an exact copy of the relevant section of 
the original diff in D24278, with no review comments addressed.

https://reviews.llvm.org/D24915

Files:
  lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  test/Analysis/DeallocMissingRelease.m
  test/Analysis/PR2978.m
  test/Analysis/properties.m

Index: test/Analysis/properties.m
===================================================================
--- test/Analysis/properties.m
+++ test/Analysis/properties.m
@@ -134,11 +134,17 @@
   NSString *_name;
 }
 @property (retain) NSString * name;
+#if !__has_feature(objc_arc)
+// expected-note@-2 {{The property is declared here}}
+#endif
 @property (assign) id friend;
 @end
 
 @implementation Person
 @synthesize name = _name;
+#if !__has_feature(objc_arc)
+// expected-note@-2 {{The property is synthesized here}}
+#endif
 
 -(void)dealloc {
 #if !__has_feature(objc_arc)
Index: test/Analysis/PR2978.m
===================================================================
--- test/Analysis/PR2978.m
+++ test/Analysis/PR2978.m
@@ -29,22 +29,22 @@
   id _nonPropertyIvar;
 }
 @property(retain) id X;
-@property(retain) id Y;
-@property(assign) id Z;
+@property(retain) id Y; // expected-note{{The property is declared here}}
+@property(assign) id Z; // expected-note{{The property is declared here}}
 @property(assign) id K;
 @property(weak) id L;
 @property(readonly) id N;
 @property(retain) id M;
 @property(weak) id P;
-@property(weak) id Q;
+@property(weak) id Q; // expected-note{{The property is declared here}}
 @property(retain) id R;
-@property(weak, readonly) id S;
+@property(weak, readonly) id S; // expected-note{{The property is declared here}}
 
 @property(assign, readonly) id T; // Shadowed in class extension
 @property(assign) id U;
 
 @property(retain) id V;
-@property(retain) id W;
+@property(retain) id W; // expected-note{{The property is declared here}}
 -(id) O;
 -(void) setO: (id) arg;
 @end
@@ -56,16 +56,16 @@
 
 @implementation MyClass
 @synthesize X = _X;
-@synthesize Y = _Y;
-@synthesize Z = _Z;
+@synthesize Y = _Y; // expected-note{{The property is synthesized here}}
+@synthesize Z = _Z; // expected-note{{The property is synthesized here}}
 @synthesize K = _K;
 @synthesize L = _L;
 @synthesize N = _N;
 @synthesize M = _M;
-@synthesize Q = _Q;
+@synthesize Q = _Q; // expected-note{{The property is synthesized here}}
 @synthesize R = _R;
 @synthesize V = _V;
-@synthesize W = _W;
+@synthesize W = _W; // expected-note{{The property is synthesized here}}
 
 -(id) O{ return 0; }
 -(void) setO:(id)arg { }
Index: test/Analysis/DeallocMissingRelease.m
===================================================================
--- test/Analysis/DeallocMissingRelease.m
+++ test/Analysis/DeallocMissingRelease.m
@@ -80,6 +80,9 @@
 
 @interface MyPropertyClass1 : NSObject
 @property (copy) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @end
 
 @implementation MyPropertyClass1
@@ -93,6 +96,9 @@
 
 @interface MyPropertyClass2 : NSObject
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @end
 
 @implementation MyPropertyClass2
@@ -108,10 +114,16 @@
   NSObject *_ivar;
 }
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @end
 
 @implementation MyPropertyClass3
 @synthesize ivar = _ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is synthesized here}}
+#endif
 - (void)dealloc
 {
 #if NON_ARC
@@ -125,13 +137,19 @@
   void (^_blockPropertyIvar)(void);
 }
 @property (copy) void (^blockProperty)(void);
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @property (copy) void (^blockProperty2)(void);
 @property (copy) void (^blockProperty3)(void);
 
 @end
 
 @implementation MyPropertyClass4
 @synthesize blockProperty = _blockPropertyIvar;
+#if NON_ARC
+// expected-note@-2 {{The property is synthesized here}}
+#endif
 - (void)dealloc
 {
 #if NON_ARC
@@ -163,10 +181,16 @@
   NSObject *_ivar;
 }
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @end
 
 @implementation MyPropertyClassWithReturnInDealloc
 @synthesize ivar = _ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is synthesized here}}
+#endif
 - (void)dealloc
 {
   return;
@@ -182,12 +206,18 @@
   MyPropertyClassWithReleaseInOtherInstance *_other;
 }
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 
 -(void)releaseIvars;
 @end
 
 @implementation MyPropertyClassWithReleaseInOtherInstance
 @synthesize ivar = _ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is synthesized here}}
+#endif
 
 -(void)releaseIvars; {
 #if NON_ARC
@@ -208,10 +238,16 @@
   NSObject *_ivar;
 }
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @end
 
 @implementation MyPropertyClassWithNeitherReturnNorSuperDealloc
 @synthesize ivar = _ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is synthesized here}}
+#endif
 - (void)dealloc
 {
 }
@@ -246,6 +282,9 @@
   BOOL _ivar1;
 }
 @property (retain) NSObject *ivar2;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @end
 
 @implementation ClassWithControlFlowInRelease
@@ -287,6 +326,9 @@
 
 @interface ClassWithNildOutIvar : NSObject
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @end
 
 @implementation ClassWithNildOutIvar
@@ -305,6 +347,9 @@
 
 @interface ClassWithUpdatedIvar : NSObject
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @end
 
 @implementation ClassWithUpdatedIvar
@@ -349,6 +394,9 @@
 @property (retain) NSObject *propNilledOutInFunction;
 
 @property (retain) NSObject *ivarNeverReleased;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 - (void)invalidateInMethod;
 @end
 
@@ -425,6 +473,9 @@
 
 @interface ClassWhereSelfEscapesViaSynthesizedPropertyAccess : NSObject
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @property (retain) NSObject *otherIvar;
 @end
 
@@ -442,6 +493,9 @@
 
 @interface ClassWhereSelfEscapesViaCallToSystem : NSObject
 @property (retain) NSObject *ivar1;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @property (retain) NSObject *ivar2;
 @property (retain) NSObject *ivar3;
 @property (retain) NSObject *ivar4;
@@ -536,6 +590,9 @@
 
 @interface SuperClassOfClassWithInlinedSuperDealloc : NSObject
 @property (retain) NSObject *propInSuper;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @end
 
 @implementation SuperClassOfClassWithInlinedSuperDealloc
@@ -548,6 +605,9 @@
 
 @interface ClassWithInlinedSuperDealloc : SuperClassOfClassWithInlinedSuperDealloc
 @property (retain) NSObject *propInSub;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @end
 
 @implementation ClassWithInlinedSuperDealloc
@@ -605,6 +665,9 @@
 
 @interface SuperClassOfClassThatEscapesBeforeInliningSuper : NSObject
 @property (retain) NSObject *propInSuper;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @end
 
 @implementation SuperClassOfClassThatEscapesBeforeInliningSuper
@@ -794,6 +857,9 @@
 
 @property(retain) NSObject *inputIvar;
 @property(retain) NSObject *nonInputIvar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @property(retain) NSObject *inputAutoSynthesizedIvar;
 @property(retain) NSObject *inputExplicitlySynthesizedToNonPrefixedIvar;
 @property(retain) NSObject *nonPrefixedPropertyBackedByExplicitlySynthesizedPrefixedIvar;
@@ -803,6 +869,9 @@
 @implementation ImmediateSubCIFilter
 @synthesize inputIvar = inputIvar;
 @synthesize nonInputIvar = nonInputIvar;
+#if NON_ARC
+// expected-note@-2 {{The property is synthesized here}}
+#endif
 @synthesize inputExplicitlySynthesizedToNonPrefixedIvar = notPrefixedButBackingPrefixedProperty;
 @synthesize nonPrefixedPropertyBackedByExplicitlySynthesizedPrefixedIvar = inputPrefixedButBackingNonPrefixedProperty;
 
@@ -841,6 +910,9 @@
 }
 
 @property(retain) NSObject *inputIvar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
 @end
 
 @implementation OverreleasingCIFilter
Index: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -128,6 +128,9 @@
   void checkEndFunction(CheckerContext &Ctx) const;
 
 private:
+  void addExtraNoteForDecl(std::unique_ptr<BugReport> &BR, StringRef Msg,
+                           const Decl *D) const;
+
   void diagnoseMissingReleases(CheckerContext &C) const;
 
   bool diagnoseExtraRelease(SymbolRef ReleasedValue, const ObjCMethodCall &M,
@@ -489,6 +492,18 @@
   return State;
 }
 
+/// Add an extra note piece describing a declaration that is important
+/// for understanding the bug report.
+void ObjCDeallocChecker::addExtraNoteForDecl(std::unique_ptr<BugReport> &BR,
+                                             StringRef Msg,
+                                             const Decl *D) const {
+  ASTContext &ACtx = D->getASTContext();
+  SourceManager &SM = ACtx.getSourceManager();
+  PathDiagnosticLocation Pos = PathDiagnosticLocation::createBegin(D, SM);
+  if (Pos.isValid() && Pos.asLocation().isValid())
+    BR->addExtraNote(Msg, Pos, D->getSourceRange());
+}
+
 /// Report any unreleased instance variables for the current instance being
 /// dealloced.
 void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext &C) const {
@@ -586,6 +601,10 @@
     std::unique_ptr<BugReport> BR(
         new BugReport(*MissingReleaseBugType, OS.str(), ErrNode));
 
+    addExtraNoteForDecl(BR, "The property is declared here", PropDecl);
+
+    addExtraNoteForDecl(BR, "The property is synthesized here", PropImpl);
+
     C.emitReport(std::move(BR));
   }
 
@@ -689,11 +708,12 @@
          );
 
   const ObjCImplDecl *Container = getContainingObjCImpl(C.getLocationContext());
-  OS << "The '" << *PropImpl->getPropertyIvarDecl()
-     << "' ivar in '" << *Container;
+  const ObjCIvarDecl *IvarDecl = PropImpl->getPropertyIvarDecl();
+  OS << "The '" << *IvarDecl << "' ivar in '" << *Container;
 
+  bool ReleasedByCIFilterDealloc = isReleasedByCIFilterDealloc(PropImpl);
 
-  if (isReleasedByCIFilterDealloc(PropImpl)) {
+  if (ReleasedByCIFilterDealloc) {
     OS << "' will be released by '-[CIFilter dealloc]' but also released here";
   } else {
     OS << "' was synthesized for ";
@@ -710,6 +730,11 @@
       new BugReport(*ExtraReleaseBugType, OS.str(), ErrNode));
   BR->addRange(M.getOriginExpr()->getSourceRange());
 
+  addExtraNoteForDecl(BR, "The property is declared here", PropDecl);
+
+  if (!ReleasedByCIFilterDealloc)
+    addExtraNoteForDecl(BR, "The property is synthesized here", PropImpl);
+
   C.emitReport(std::move(BR));
 
   return true;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to