[PATCH] D27717: [analyzer] Suppress a leak false positive in Qt's QObject::connectImpl()

2016-12-16 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL289939: [analyzer] Add another exception for Qt in 
MallocChecker (authored by dergachev).

Changed prior to commit:
  https://reviews.llvm.org/D27717?vs=81239=81738#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27717

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  cfe/trunk/test/Analysis/Inputs/qt-simulator.h
  cfe/trunk/test/Analysis/qt_malloc.cpp


Index: cfe/trunk/test/Analysis/qt_malloc.cpp
===
--- cfe/trunk/test/Analysis/qt_malloc.cpp
+++ cfe/trunk/test/Analysis/qt_malloc.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus
 -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -std=c++11 -analyze 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus
 -analyzer-store=region -verify %s
 // expected-no-diagnostics
 #include "Inputs/qt-simulator.h"
 
@@ -13,3 +13,9 @@
   QEvent *e4 = new QEvent(QEvent::None);
   QApplication::postEvent(obj, e4);
 }
+
+void connect(QObject *obj) {
+  obj->connectImpl(nullptr, nullptr, nullptr, nullptr,
+   new QtPrivate::QSlotObjectBase(), (Qt::ConnectionType)0,
+   nullptr, nullptr);
+}
Index: cfe/trunk/test/Analysis/Inputs/qt-simulator.h
===
--- cfe/trunk/test/Analysis/Inputs/qt-simulator.h
+++ cfe/trunk/test/Analysis/Inputs/qt-simulator.h
@@ -1,6 +1,23 @@
 #pragma clang system_header
 
+namespace QtPrivate {
+struct QSlotObjectBase {};
+}
+
+namespace Qt {
+enum ConnectionType {};
+}
+
+struct QMetaObject {
+  struct Connection {};
+};
+
 struct QObject {
+  static QMetaObject::Connection connectImpl(const QObject *, void **,
+ const QObject *, void **,
+ QtPrivate::QSlotObjectBase *,
+ Qt::ConnectionType,
+ const int *, const QMetaObject *);
 };
 
 struct QEvent {
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2579,6 +2579,11 @@
 return true;
   }
 
+  if (FName == "connectImpl" &&
+  FD->getQualifiedNameAsString() == "QObject::connectImpl") {
+return true;
+  }
+
   // Handle cases where we know a buffer's /address/ can escape.
   // Note that the above checks handle some special cases where we know that
   // even though the address escapes, it's still our responsibility to free the


Index: cfe/trunk/test/Analysis/qt_malloc.cpp
===
--- cfe/trunk/test/Analysis/qt_malloc.cpp
+++ cfe/trunk/test/Analysis/qt_malloc.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -std=c++11 -analyze -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus -analyzer-store=region -verify %s
 // expected-no-diagnostics
 #include "Inputs/qt-simulator.h"
 
@@ -13,3 +13,9 @@
   QEvent *e4 = new QEvent(QEvent::None);
   QApplication::postEvent(obj, e4);
 }
+
+void connect(QObject *obj) {
+  obj->connectImpl(nullptr, nullptr, nullptr, nullptr,
+   new QtPrivate::QSlotObjectBase(), (Qt::ConnectionType)0,
+   nullptr, nullptr);
+}
Index: cfe/trunk/test/Analysis/Inputs/qt-simulator.h
===
--- cfe/trunk/test/Analysis/Inputs/qt-simulator.h
+++ cfe/trunk/test/Analysis/Inputs/qt-simulator.h
@@ -1,6 +1,23 @@
 #pragma clang system_header
 
+namespace QtPrivate {
+struct QSlotObjectBase {};
+}
+
+namespace Qt {
+enum ConnectionType {};
+}
+
+struct QMetaObject {
+  struct Connection {};
+};
+
 struct QObject {
+  static QMetaObject::Connection connectImpl(const QObject *, void **,
+ const QObject *, void **,
+ QtPrivate::QSlotObjectBase *,
+ Qt::ConnectionType,
+ const int *, const QMetaObject *);
 };
 
 struct QEvent {
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2579,6 +2579,11 @@
 return true;
   }
 
+  if (FName == "connectImpl" &&
+  FD->getQualifiedNameAsString() ==

[PATCH] D27717: [analyzer] Suppress a leak false positive in Qt's QObject::connectImpl()

2016-12-13 Thread Aleksei Sidorin via Phabricator via cfe-commits 
a.sidorin accepted this revision.
a.sidorin added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!




Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2577
 
   if (FName == "postEvent" &&
   FD->getQualifiedNameAsString() == "QCoreApplication::postEvent") {

Sorry for disturbing you for code which is not yours but this line looks 
exactly like the line upper and seems to be a dead/useless code.


https://reviews.llvm.org/D27717



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27717: [analyzer] Suppress a leak false positive in Qt's QObject::connectImpl()

2016-12-13 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ created this revision.
NoQ added reviewers: zaks.anna, dcoughlin, xazax.hun, a.sidorin.
NoQ added a subscriber: cfe-commits.

A quick fix to address the false positive posted by Tiago Macarios in the 
mailing lists: http://lists.llvm.org/pipermail/cfe-dev/2016-December/051738.html

MallocChecker processes pointer escapes heuristically rather than regularly. 
Force it to treat the pointers passed to `connectImpl()` as escaping.

I dislike a few things about this patch - we're patching against implementation 
details and hardcoding names of private methods in an external library. I don't 
see a significantly better solution within the current approach though.

See also https://reviews.llvm.org/D27599 - a similar hack for another function.


https://reviews.llvm.org/D27717

Files:
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/Inputs/qt-simulator.h
  test/Analysis/qt_malloc.cpp


Index: test/Analysis/qt_malloc.cpp
===
--- test/Analysis/qt_malloc.cpp
+++ test/Analysis/qt_malloc.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus
 -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -std=c++11 -analyze 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus
 -analyzer-store=region -verify %s
 // expected-no-diagnostics
 #include "Inputs/qt-simulator.h"
 
@@ -13,3 +13,9 @@
   QEvent *e4 = new QEvent(QEvent::None);
   QApplication::postEvent(obj, e4);
 }
+
+void connect(QObject *obj) {
+  obj->connectImpl(nullptr, nullptr, nullptr, nullptr,
+   new QtPrivate::QSlotObjectBase(), (Qt::ConnectionType)0,
+   nullptr, nullptr);
+}
Index: test/Analysis/Inputs/qt-simulator.h
===
--- test/Analysis/Inputs/qt-simulator.h
+++ test/Analysis/Inputs/qt-simulator.h
@@ -1,6 +1,23 @@
 #pragma clang system_header
 
+namespace QtPrivate {
+struct QSlotObjectBase {};
+}
+
+namespace Qt {
+enum ConnectionType {};
+}
+
+struct QMetaObject {
+  struct Connection {};
+};
+
 struct QObject {
+  static QMetaObject::Connection connectImpl(const QObject *, void **,
+ const QObject *, void **,
+ QtPrivate::QSlotObjectBase *,
+ Qt::ConnectionType,
+ const int *, const QMetaObject *);
 };
 
 struct QEvent {
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2579,6 +2579,11 @@
 return true;
   }
 
+  if (FName == "connectImpl" &&
+  FD->getQualifiedNameAsString() == "QObject::connectImpl") {
+return true;
+  }
+
   // Handle cases where we know a buffer's /address/ can escape.
   // Note that the above checks handle some special cases where we know that
   // even though the address escapes, it's still our responsibility to free the


Index: test/Analysis/qt_malloc.cpp
===
--- test/Analysis/qt_malloc.cpp
+++ test/Analysis/qt_malloc.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -std=c++11 -analyze -analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus -analyzer-store=region -verify %s
 // expected-no-diagnostics
 #include "Inputs/qt-simulator.h"
 
@@ -13,3 +13,9 @@
   QEvent *e4 = new QEvent(QEvent::None);
   QApplication::postEvent(obj, e4);
 }
+
+void connect(QObject *obj) {
+  obj->connectImpl(nullptr, nullptr, nullptr, nullptr,
+   new QtPrivate::QSlotObjectBase(), (Qt::ConnectionType)0,
+   nullptr, nullptr);
+}
Index: test/Analysis/Inputs/qt-simulator.h
===
--- test/Analysis/Inputs/qt-simulator.h
+++ test/Analysis/Inputs/qt-simulator.h
@@ -1,6 +1,23 @@
 #pragma clang system_header
 
+namespace QtPrivate {
+struct QSlotObjectBase {};
+}
+
+namespace Qt {
+enum ConnectionType {};
+}
+
+struct QMetaObject {
+  struct Connection {};
+};
+
 struct QObject {
+  static QMetaObject::Connection connectImpl(const QObject *, void **,
+ const QObject *, void **,
+ QtPrivate::QSlotObjectBase *,
+ Qt::ConnectionType,
+ const int *, const QMetaObject *);
 };
 
 struct QEvent {
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp