[PATCH] D55680: [analyzer] ObjCDealloc: Fix a crash when a class attempts to deallocate another class.

2018-12-14 Thread Phabricator via Phabricator via cfe-commits 
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL349228: [analyzer] ObjCDealloc: Fix a crash when a class 
attempts to deallocate a class. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55680?vs=178146=178330#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55680/new/

https://reviews.llvm.org/D55680

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  cfe/trunk/test/Analysis/MissingDealloc.m


Index: cfe/trunk/test/Analysis/MissingDealloc.m
===
--- cfe/trunk/test/Analysis/MissingDealloc.m
+++ cfe/trunk/test/Analysis/MissingDealloc.m
@@ -183,4 +183,17 @@
 @implementation NonNSObjectMissingDealloc
 @end
 
-// CHECK: 4 warnings generated.
+
+//======
+// Don't crash on calls to dealloc as a class method.
+
+@interface DeallocingClass : NSObject {}
+@end
+@implementation DeallocingClass
+- (void)dealloc {
+  [DeallocingClass dealloc]; // FIXME: Should we warn on this specifically?
+}
+#if NON_ARC
+// expected-warning@-2{{method possibly missing a [super dealloc] call}}
+#endif
+@end
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -715,6 +715,10 @@
 bool ObjCDeallocChecker::diagnoseMistakenDealloc(SymbolRef DeallocedValue,
  const ObjCMethodCall ,
  CheckerContext ) const {
+  // TODO: Apart from unknown/undefined receivers, this may happen when
+  // dealloc is called as a class method. Should we warn?
+  if (!DeallocedValue)
+return false;
 
   // Find the property backing the instance variable that M
   // is dealloc'ing.


Index: cfe/trunk/test/Analysis/MissingDealloc.m
===
--- cfe/trunk/test/Analysis/MissingDealloc.m
+++ cfe/trunk/test/Analysis/MissingDealloc.m
@@ -183,4 +183,17 @@
 @implementation NonNSObjectMissingDealloc
 @end
 
-// CHECK: 4 warnings generated.
+
+//======
+// Don't crash on calls to dealloc as a class method.
+
+@interface DeallocingClass : NSObject {}
+@end
+@implementation DeallocingClass
+- (void)dealloc {
+  [DeallocingClass dealloc]; // FIXME: Should we warn on this specifically?
+}
+#if NON_ARC
+// expected-warning@-2{{method possibly missing a [super dealloc] call}}
+#endif
+@end
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -715,6 +715,10 @@
 bool ObjCDeallocChecker::diagnoseMistakenDealloc(SymbolRef DeallocedValue,
  const ObjCMethodCall ,
  CheckerContext ) const {
+  // TODO: Apart from unknown/undefined receivers, this may happen when
+  // dealloc is called as a class method. Should we warn?
+  if (!DeallocedValue)
+return false;
 
   // Find the property backing the instance variable that M
   // is dealloc'ing.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55680: [analyzer] ObjCDealloc: Fix a crash when a class attempts to deallocate another class.

2018-12-13 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ created this revision.
NoQ added reviewers: dcoughlin, george.karpenkov.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.

The checker wasn't prepared to see the `dealloc` message sent to the class 
itself rather than to an instance, as if it was `+dealloc`.

Additionally, it wasn't prepared for pure unknown `self` values; the new guard 
covers that as well, but it is annoying to test because both kinds of values 
shouldn't really appear and we generally want to get rid of all of them (by 
modeling unknown values with symbols and by warning on use of undefined values 
before they are used).

The `CHECK:` directive for `FileCheck` at the end of the test looks useless, so 
i removed it.


Repository:
  rC Clang

https://reviews.llvm.org/D55680

Files:
  lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  test/Analysis/MissingDealloc.m


Index: test/Analysis/MissingDealloc.m
===
--- test/Analysis/MissingDealloc.m
+++ test/Analysis/MissingDealloc.m
@@ -183,4 +183,17 @@
 @implementation NonNSObjectMissingDealloc
 @end
 
-// CHECK: 4 warnings generated.
+
+//======
+// Don't crash on calls to dealloc as a class method.
+
+@interface DeallocingClass : NSObject {}
+@end
+@implementation DeallocingClass
+- (void)dealloc {
+  [DeallocingClass dealloc]; // FIXME: Should we warn on this specifically?
+}
+#if NON_ARC
+// expected-warning@-2{{method possibly missing a [super dealloc] call}}
+#endif
+@end
Index: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -715,6 +715,10 @@
 bool ObjCDeallocChecker::diagnoseMistakenDealloc(SymbolRef DeallocedValue,
  const ObjCMethodCall ,
  CheckerContext ) const {
+  // TODO: Apart from unknown/undefined receivers, this may happen when
+  // dealloc is called as a class method. Should we warn?
+  if (!DeallocedValue)
+return false;
 
   // Find the property backing the instance variable that M
   // is dealloc'ing.


Index: test/Analysis/MissingDealloc.m
===
--- test/Analysis/MissingDealloc.m
+++ test/Analysis/MissingDealloc.m
@@ -183,4 +183,17 @@
 @implementation NonNSObjectMissingDealloc
 @end
 
-// CHECK: 4 warnings generated.
+
+//======
+// Don't crash on calls to dealloc as a class method.
+
+@interface DeallocingClass : NSObject {}
+@end
+@implementation DeallocingClass
+- (void)dealloc {
+  [DeallocingClass dealloc]; // FIXME: Should we warn on this specifically?
+}
+#if NON_ARC
+// expected-warning@-2{{method possibly missing a [super dealloc] call}}
+#endif
+@end
Index: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -715,6 +715,10 @@
 bool ObjCDeallocChecker::diagnoseMistakenDealloc(SymbolRef DeallocedValue,
  const ObjCMethodCall ,
  CheckerContext ) const {
+  // TODO: Apart from unknown/undefined receivers, this may happen when
+  // dealloc is called as a class method. Should we warn?
+  if (!DeallocedValue)
+return false;
 
   // Find the property backing the instance variable that M
   // is dealloc'ing.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits