Fix false positive warning about memory leak for QApplication::postEvent

[Dushistov]

Summary

Recent version of clang (3.7) start complain about such code:
void send(QObject *obj)
{

QKeyEvent *event = new QKeyEvent(QEvent::KeyRelease, Qt::Key_Tab, Qt::NoModifier);
qApp->postEvent(obj, event);

}

warning: Potential leak of memory pointed to by 'event'

This is false positive, because of according to Qt documentation Qt take care about memory allocated for QEvent:
http://doc.qt.io/qt-4.8/qcoreapplication.html#postEvent

Because of Qt popular enought I suggest to handle postEvent case in MallocChecker

[zaks.anna]

This needs a test case. You can either extend ./test/Analysis/malloc.cpp or add another QT specific test file.

Thanks!
Anna.

[Dushistov]

I added test case and move it to separate file, because of
1)it require additional option -analyzer-checker=cplusplus to trigger warning
2)Simple emulation of qt type system not work (not trigger any warning), so I add preprocessored file.

[zaks.anna]

Test cases need to be small and self contained.
You might be having a problem with reproducing with a simple test case, where you define QCoreApplication::postEvent in the test file because it is not considered to be in the system header. You can use special pragmas for that. See ./test/Analysis/Inputs, where we have a bunch of .h files declaring system APIs. Please add another file for QT there.

lib/StaticAnalyzer/Checkers/MallocChecker.cpp

2516 ↗

(On Diff #39009)

http://llvm.org/docs/CodingStandards.html#id16

[Dushistov]

I reduce testcase to almost minimal variant.

[Dushistov]

Fix line length issue

[Dushistov]

mistype, actually I want to use '&&' here, not '||' to not create std::string,
if match failed.

[xazax.hun]

lib/StaticAnalyzer/Checkers/MallocChecker.cpp

2514 ↗

(On Diff #39985)

Shouldn't you use == instead of endswith here?

[Dushistov]

Replace 'endswith' with 'operator==', also update test to check that all four ways to call postEvent not produce warning

[Dushistov]

apply all suggestions

[zaks.anna]

test/Analysis/qt_malloc.cpp

11 ↗

(On Diff #40485)

Should the leak be reported when the object is passed to QApplication::postEvent?

In this test, 'event' escapes during one of these calls and the rest of the calls are not tested/make no difference. If you want to test that 'event' escapes when each of them is called, you'd need to test each of them with a different object.

[Dushistov]

Make test for usage of different variants of postEvent more robust.

[Dushistov]

Should the leak be reported when the object is passed to QApplication::postEvent?

No, QApplication::postEvent == QCoreApplication::postEvent, it is just the same function,
because of QApplication inherit from QCoreApplication, and not introduce it's own postEvent.

In this test, 'event' escapes during one of these calls and the rest of the calls are not tested/make no >difference.

fixed

[zaks.anna]

Thanks!
LGTM. I'll commit.

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

bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly(
   if (FName == "CGBitmapContextCreate" ||
       FName == "CGBitmapContextCreateWithData" ||
       FName == "CVPixelBufferCreateWithBytes" ||
       FName == "CVPixelBufferCreateWithPlanarBytes" ||
       FName == "OSAtomicEnqueue") {
     return true;
   }
 
+  if (FName == "postEvent" &&
+      FD->getQualifiedNameAsString() == "QCoreApplication::postEvent") {
+    return true;
+  }
+
+  if (FName == "postEvent" &&
+      FD->getQualifiedNameAsString() == "QCoreApplication::postEvent") {
+    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
   // buffer.
   if (Call->argumentsMayEscape())
     return true;
 
   // Otherwise, assume that the function does not free memory.

cfe/trunk/test/Analysis/Inputs/qt-simulator.h

+#pragma clang system_header
+
+struct QObject {
+};
+
+struct QEvent {
+  enum Type { None };
+  QEvent(Type) {}
+};
+
+struct QCoreApplication : public QObject {
+  static void postEvent(QObject *receiver, QEvent *event);
+  static QCoreApplication *instance();
+};
+
+struct QApplication : public QCoreApplication {};

cfe/trunk/test/Analysis/qt_malloc.cpp

+// RUN: %clang_cc1 -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"
+
+void send(QObject *obj)
+{
+  QEvent *e1 = new QEvent(QEvent::None);
+  static_cast<QApplication *>(QCoreApplication::instance())->postEvent(obj, e1);
+  QEvent *e2 = new QEvent(QEvent::None);
+  QCoreApplication::instance()->postEvent(obj, e2);
+  QEvent *e3 = new QEvent(QEvent::None);
+  QCoreApplication::postEvent(obj, e3);
+  QEvent *e4 = new QEvent(QEvent::None);
+  QApplication::postEvent(obj, e4);
+}