[analyzer] Extend bug reports with extra notes - ObjCDeallocChecker

[NoQ]

Summary

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 path notes.

Examples of HTML reports:

objc-dealloc-normal.html16 KBDownload

objc-dealloc-notes-as-events.html16 KBDownload

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.

[NoQ]

Addressed two inline comments by Devin in the original review D24278. Rebased on top of the updated D24278.

[zaks.anna]

LGTM and Devin's comments have been addressed.

[vitalybuka]

Reverted with r283181 as it depends r283092

cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp

mutable IdentifierInfo *NSObjectII, *SenTestCaseII, *XCTestCaseII,
       *Block_releaseII, *CIFilterII;
 
   mutable Selector DeallocSel, ReleaseSel;
 
   std::unique_ptr<BugType> MissingReleaseBugType;
   std::unique_ptr<BugType> ExtraReleaseBugType;
   std::unique_ptr<BugType> MistakenDeallocBugType;
 
+  static constexpr const char *MsgDeclared = "Property is declared here";
+  static constexpr const char *MsgSynthesized = "Property is synthesized here";
+
 public:
   ObjCDeallocChecker();
 
   void checkASTDecl(const ObjCImplementationDecl *D, AnalysisManager& Mgr,
                     BugReporter &BR) const;
   void checkBeginFunction(CheckerContext &Ctx) const;
   void checkPreObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const;
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const;
 
   ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
                              bool Assumption) const;
 
   ProgramStateRef checkPointerEscape(ProgramStateRef State,
                                      const InvalidatedSymbols &Escaped,
                                      const CallEvent *Call,
                                      PointerEscapeKind Kind) const;
   void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const;
   void checkEndFunction(CheckerContext &Ctx) const;
 
 private:
+  void addNoteForDecl(std::unique_ptr<BugReport> &BR, StringRef Msg,
+                           const Decl *D) const;
+
   void diagnoseMissingReleases(CheckerContext &C) const;
 
   bool diagnoseExtraRelease(SymbolRef ReleasedValue, const ObjCMethodCall &M,
                             CheckerContext &C) const;
 
   bool diagnoseMistakenDealloc(SymbolRef DeallocedValue,
                                const ObjCMethodCall &M,
                                CheckerContext &C) const;
if (!InstanceSymbol)
       continue;
 
     State = removeValueRequiringRelease(State, InstanceSymbol, Sym);
   }
 
   return State;
 }
 
+/// Add an extra note piece describing a declaration that is important
+/// for understanding the bug report.
+void ObjCDeallocChecker::addNoteForDecl(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->addNote(Msg, Pos, D->getSourceRange());
+}
+
 /// Report any unreleased instance variables for the current instance being
 /// dealloced.
 void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext &C) const {
   ProgramStateRef State = C.getState();
 
   SVal SelfVal;
   if (!isInInstanceDealloc(C, SelfVal))
     return;
else
       OS << "copied";
 
     OS << " by a synthesized property but not released"
           " before '[super dealloc]'";
 
     std::unique_ptr<BugReport> BR(
         new BugReport(*MissingReleaseBugType, OS.str(), ErrNode));
 
+    addNoteForDecl(BR, MsgDeclared, PropDecl);
+    addNoteForDecl(BR, MsgSynthesized, PropImpl);
+
     C.emitReport(std::move(BR));
   }
 
   if (NewUnreleased.isEmpty()) {
     State = State->remove<UnreleasedIvarMap>(SelfSym);
   } else {
     State = State->set<UnreleasedIvarMap>(SelfSym, NewUnreleased);
   }
bool ObjCDeallocChecker::diagnoseExtraRelease(SymbolRef ReleasedValue,
 
   assert(PropDecl->getSetterKind() == ObjCPropertyDecl::Weak ||
          (PropDecl->getSetterKind() == ObjCPropertyDecl::Assign &&
           !PropDecl->isReadOnly()) ||
          isReleasedByCIFilterDealloc(PropImpl)
          );
 
   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 ";
 
     if (PropDecl->getSetterKind() == ObjCPropertyDecl::Weak)
       OS << "a weak";
     else
       OS << "an assign, readwrite";
 
     OS <<  " property but was released in 'dealloc'";
   }
 
   std::unique_ptr<BugReport> BR(
       new BugReport(*ExtraReleaseBugType, OS.str(), ErrNode));
   BR->addRange(M.getOriginExpr()->getSourceRange());
 
+  addNoteForDecl(BR, MsgDeclared, PropDecl);
+  if (!ReleasedByCIFilterDealloc)
+    addNoteForDecl(BR, MsgSynthesized, PropImpl);
+
   C.emitReport(std::move(BR));
 
   return true;
 }
 
 /// Emits a warning if the current context is -dealloc and DeallocedValue
 /// must not be directly dealloced in a -dealloc. Returns true if a diagnostic
 /// was emitted.

cfe/trunk/test/Analysis/DeallocMissingRelease.m

 #endif
 }
 @end
 
 // Warn about missing release in -dealloc for properties.
 
 @interface MyPropertyClass1 : NSObject
 @property (copy) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @end
 
 @implementation MyPropertyClass1
 - (void)dealloc
 {
 #if NON_ARC
   [super dealloc]; // expected-warning {{The '_ivar' ivar in 'MyPropertyClass1' was copied by a synthesized property but not released before '[super dealloc]'}}
 #endif
 }
 @end
 
 @interface MyPropertyClass2 : NSObject
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @end
 
 @implementation MyPropertyClass2
 - (void)dealloc
 {
 #if NON_ARC
   [super dealloc]; // expected-warning {{The '_ivar' ivar in 'MyPropertyClass2' was retained by a synthesized property but not released before '[super dealloc]'}}
 #endif
 }
 @end
 
 @interface MyPropertyClass3 : NSObject {
   NSObject *_ivar;
 }
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @end
 
 @implementation MyPropertyClass3
 @synthesize ivar = _ivar;
+#if NON_ARC
+// expected-note@-2 {{Property is synthesized here}}
+#endif
 - (void)dealloc
 {
 #if NON_ARC
   [super dealloc]; // expected-warning {{The '_ivar' ivar in 'MyPropertyClass3' was retained by a synthesized property but not released before '[super dealloc]'}}
 #endif
 }
 
 @end
 
 @interface MyPropertyClass4 : NSObject {
   void (^_blockPropertyIvar)(void);
 }
 @property (copy) void (^blockProperty)(void);
+#if NON_ARC
+// expected-note@-2 {{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 {{Property is synthesized here}}
+#endif
 - (void)dealloc
 {
 #if NON_ARC
   [_blockProperty2 release];
   Block_release(_blockProperty3);
 
   [super dealloc]; // expected-warning {{The '_blockPropertyIvar' ivar in 'MyPropertyClass4' was copied by a synthesized property but not released before '[super dealloc]'}}
 #endif

 #endif
 }
 @end
 
 @interface MyPropertyClassWithReturnInDealloc : NSObject {
   NSObject *_ivar;
 }
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @end
 
 @implementation MyPropertyClassWithReturnInDealloc
 @synthesize ivar = _ivar;
+#if NON_ARC
+// expected-note@-2 {{Property is synthesized here}}
+#endif
 - (void)dealloc
 {
   return;
 #if NON_ARC
   // expected-warning@-2{{The '_ivar' ivar in 'MyPropertyClassWithReturnInDealloc' was retained by a synthesized property but not released before '[super dealloc]'}}
   [super dealloc];
 #endif
 }
 @end
 
 @interface MyPropertyClassWithReleaseInOtherInstance : NSObject {
   NSObject *_ivar;
   MyPropertyClassWithReleaseInOtherInstance *_other;
 }
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 
 -(void)releaseIvars;
 @end
 
 @implementation MyPropertyClassWithReleaseInOtherInstance
 @synthesize ivar = _ivar;
+#if NON_ARC
+// expected-note@-2 {{Property is synthesized here}}
+#endif
 
 -(void)releaseIvars; {
 #if NON_ARC
   [_ivar release];
 #endif
 }
 
 - (void)dealloc
 {
   [_other releaseIvars];
 #if NON_ARC
   [super dealloc]; // expected-warning {{The '_ivar' ivar in 'MyPropertyClassWithReleaseInOtherInstance' was retained by a synthesized property but not released before '[super dealloc]'}}
 #endif
 }
 @end
 
 @interface MyPropertyClassWithNeitherReturnNorSuperDealloc : NSObject {
   NSObject *_ivar;
 }
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @end
 
 @implementation MyPropertyClassWithNeitherReturnNorSuperDealloc
 @synthesize ivar = _ivar;
+#if NON_ARC
+// expected-note@-2 {{Property is synthesized here}}
+#endif
 - (void)dealloc
 {
 }
 #if NON_ARC
   // expected-warning@-2 {{method possibly missing a [super dealloc] call}} (From Sema)
   // expected-warning@-3{{The '_ivar' ivar in 'MyPropertyClassWithNeitherReturnNorSuperDealloc' was retained by a synthesized property but not released before '[super dealloc]'}}
 #endif
 @end

 #endif
 }
 @end
 
 @interface ClassWithControlFlowInRelease : NSObject {
   BOOL _ivar1;
 }
 @property (retain) NSObject *ivar2;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @end
 
 @implementation ClassWithControlFlowInRelease
 - (void)dealloc; {
   if (_ivar1) {
     // We really should warn because there is a path through -dealloc on which
     // _ivar2 is not released.
 #if NON_ARC

 #endif
 }
 @end
 
 // Do warn when the ivar but not the property is nil'd out in -dealloc
 
 @interface ClassWithNildOutIvar : NSObject
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @end
 
 @implementation ClassWithNildOutIvar
 - (void)dealloc; {
   // Oops. Meant self.ivar = nil
   _ivar = nil;
 
 #if NON_ARC
   [super dealloc]; // expected-warning {{The '_ivar' ivar in 'ClassWithNildOutIvar' was retained by a synthesized property but not released before '[super dealloc]'}}
 #endif
 }
 @end
 
 // Do warn when the ivar is updated to a different value that is then
 // released.
 
 @interface ClassWithUpdatedIvar : NSObject
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @end
 
 @implementation ClassWithUpdatedIvar
 - (void)dealloc; {
   _ivar = [[NSObject alloc] init];
 
 #if NON_ARC
   [_ivar release];

 @interface ClassWithDeallocHelpers : NSObject
 @property (retain) NSObject *ivarReleasedInMethod;
 @property (retain) NSObject *propNilledOutInMethod;
 
 @property (retain) NSObject *ivarReleasedInFunction;
 @property (retain) NSObject *propNilledOutInFunction;
 
 @property (retain) NSObject *ivarNeverReleased;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 - (void)invalidateInMethod;
 @end
 
 void ReleaseValueHelper(NSObject *iv) {
 #if NON_ARC
   [iv release];
 #endif
 }

 } // no-warning
 @end
 
 // Don't treat self as escaping when setter called on *synthesized*
 // property.
 
 @interface ClassWhereSelfEscapesViaSynthesizedPropertyAccess : NSObject
 @property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @property (retain) NSObject *otherIvar;
 @end
 
 @implementation ClassWhereSelfEscapesViaSynthesizedPropertyAccess
 - (void)dealloc; {
   self.otherIvar = nil;
 #if NON_ARC
   [super dealloc];  // expected-warning {{The '_ivar' ivar in 'ClassWhereSelfEscapesViaSynthesizedPropertyAccess' was retained by a synthesized property but not released before '[super dealloc]'}}
 #endif
 }
 @end
 
 
 // Don't treat calls to system headers as escapes
 
 @interface ClassWhereSelfEscapesViaCallToSystem : NSObject
 @property (retain) NSObject *ivar1;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @property (retain) NSObject *ivar2;
 @property (retain) NSObject *ivar3;
 @property (retain) NSObject *ivar4;
 @property (retain) NSObject *ivar5;
 @property (retain) NSObject *ivar6;
 @end
 
 @implementation ClassWhereSelfEscapesViaCallToSystem
#if NON_ARC
   [super dealloc];
 #endif
 } // no-warning
 @end
 
 
 @interface SuperClassOfClassWithInlinedSuperDealloc : NSObject
 @property (retain) NSObject *propInSuper;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @end
 
 @implementation SuperClassOfClassWithInlinedSuperDealloc
 - (void)dealloc {
 #if NON_ARC
   [super dealloc]; // expected-warning {{The '_propInSuper' ivar in 'SuperClassOfClassWithInlinedSuperDealloc' was retained by a synthesized property but not released before '[super dealloc]'}}
 #endif
 }
 @end
 
 @interface ClassWithInlinedSuperDealloc : SuperClassOfClassWithInlinedSuperDealloc
 @property (retain) NSObject *propInSub;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @end
 
 @implementation ClassWithInlinedSuperDealloc
 - (void)dealloc {
 #if NON_ARC
   [super dealloc]; // expected-warning {{The '_propInSub' ivar in 'ClassWithInlinedSuperDealloc' was retained by a synthesized property but not released before '[super dealloc]'}}
 #endif
 }
#if NON_ARC
   [super dealloc]; // no-warning
 #endif
 }
 @end
 
 
 @interface SuperClassOfClassThatEscapesBeforeInliningSuper : NSObject
 @property (retain) NSObject *propInSuper;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @end
 
 @implementation SuperClassOfClassThatEscapesBeforeInliningSuper
 
 - (void)dealloc {
 
 #if NON_ARC
   [super dealloc]; // expected-warning {{The '_propInSuper' ivar in 'SuperClassOfClassThatEscapesBeforeInliningSuper' was retained by a synthesized property but not released before '[super dealloc]'}}
@interface ImmediateSubCIFilter : CIFilter {
   NSObject *inputIvar;
   NSObject *nonInputIvar;
   NSObject *notPrefixedButBackingPrefixedProperty;
   NSObject *inputPrefixedButBackingNonPrefixedProperty;
 }
 
 @property(retain) NSObject *inputIvar;
 @property(retain) NSObject *nonInputIvar;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @property(retain) NSObject *inputAutoSynthesizedIvar;
 @property(retain) NSObject *inputExplicitlySynthesizedToNonPrefixedIvar;
 @property(retain) NSObject *nonPrefixedPropertyBackedByExplicitlySynthesizedPrefixedIvar;
 
 @end
 
 @implementation ImmediateSubCIFilter
 @synthesize inputIvar = inputIvar;
 @synthesize nonInputIvar = nonInputIvar;
+#if NON_ARC
+// expected-note@-2 {{Property is synthesized here}}
+#endif
 @synthesize inputExplicitlySynthesizedToNonPrefixedIvar = notPrefixedButBackingPrefixedProperty;
 @synthesize nonPrefixedPropertyBackedByExplicitlySynthesizedPrefixedIvar = inputPrefixedButBackingNonPrefixedProperty;
 
 - (void)dealloc {
 #if NON_ARC
   // We don't want warnings here for:
   // inputIvar
   // inputAutoSynthesizedIvar

 #endif
 }
 @end
 @interface OverreleasingCIFilter : CIFilter {
   NSObject *inputIvar;
 }
 
 @property(retain) NSObject *inputIvar;
+#if NON_ARC
+// expected-note@-2 {{Property is declared here}}
+#endif
 @end
 
 @implementation OverreleasingCIFilter
 @synthesize inputIvar = inputIvar;
 
 - (void)dealloc {
 #if NON_ARC
   // This is an over release because CIFilter's dealloc will also release it.

cfe/trunk/test/Analysis/PR2978.m

@private
   id _V;
   id _W;
 
   MyClass *_other;
 
   id _nonPropertyIvar;
 }
 @property(retain) id X;
-@property(retain) id Y;
-@property(assign) id Z;
+@property(retain) id Y; // expected-note{{Property is declared here}}
+@property(assign) id Z; // expected-note{{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{{Property is declared here}}
 @property(retain) id R;
-@property(weak, readonly) id S;
+@property(weak, readonly) id S; // expected-note{{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{{Property is declared here}}
 -(id) O;
 -(void) setO: (id) arg;
 @end
 
 @interface MyClass ()
 // Shadows T to make it readwrite internally but readonly externally.
 @property(assign, readwrite) id T;
 @end
 
 @implementation MyClass
 @synthesize X = _X;
-@synthesize Y = _Y;
-@synthesize Z = _Z;
+@synthesize Y = _Y; // expected-note{{Property is synthesized here}}
+@synthesize Z = _Z; // expected-note{{Property is synthesized here}}
 @synthesize K = _K;
 @synthesize L = _L;
 @synthesize N = _N;
 @synthesize M = _M;
-@synthesize Q = _Q;
+@synthesize Q = _Q; // expected-note{{Property is synthesized here}}
 @synthesize R = _R;
 @synthesize V = _V;
-@synthesize W = _W;
+@synthesize W = _W; // expected-note{{Property is synthesized here}}
 
 -(id) O{ return 0; }
 -(void) setO:(id)arg { }
 
 
 -(void) releaseInHelper {
   [_R release]; // no-warning
   _R = @"Hi";

cfe/trunk/test/Analysis/properties.m

 
 
 // rdar://6611873
 
 @interface Person : NSObject {
   NSString *_name;
 }
 @property (retain) NSString * name;
+#if !__has_feature(objc_arc)
+// expected-note@-2 {{Property is declared here}}
+#endif
 @property (assign) id friend;
 @end
 
 @implementation Person
 @synthesize name = _name;
+#if !__has_feature(objc_arc)
+// expected-note@-2 {{Property is synthesized here}}
+#endif
 
 -(void)dealloc {
 #if !__has_feature(objc_arc)
   self.name = [[NSString alloc] init]; // expected-warning {{leak}}
 
   [super dealloc]; // expected-warning {{The '_name' ivar in 'Person' was retained by a synthesized property but not released before '[super dealloc]}}
 #endif
 }