Introduce thread-with-slots

slots in a QThread derived classe is usually a code smell, because
they'll run in the thread where the QThread QObject lives, and not in
the thread itself.

Disabled by default, you'll have to explicitly enable it, and check
case by case if you have races.

6 files changed, 217 insertions, 24 deletions

diff --git a/src/QtUtils.h b/src/QtUtils.h
index af0fa9bf..30c61f92 100644
--- a/src/QtUtils.h
+++ b/src/QtUtils.h
@@ -168,26 +168,6 @@ inline bool isTooBigForQList(clang::QualType qt, const clang::ASTContext *contex
 }
 
 /**
- * Returns the varDecl for the 1st argument in a connect call
- */
-inline clang::ValueDecl *signalSenderForConnect(clang::CallExpr *call)
-{
-    return FunctionUtils::valueDeclForCallArgument(call, 0);
-}
-
-/**
- * Returns the varDecl for 3rd argument in connects that are passed an explicit
- * receiver or context QObject.
- */
-inline clang::ValueDecl *signalReceiverForConnect(clang::CallExpr *call)
-{
-    if (!call || call->getNumArgs() < 5)
-        return nullptr;
-
-    return FunctionUtils::valueDeclForCallArgument(call, 3);
-}
-
-/**
  * Returns true if a class has a ctor that has a parameter of type paramType.
  * ok will be false if an error occurred, or if the record is a fwd declaration, which isn't enough
  * for we to find out the signature.
@@ -241,6 +221,41 @@ CLAZYLIB_EXPORT clang::CXXMethodDecl* pmfFromConnect(clang::CallExpr *funcCall,
 CLAZYLIB_EXPORT clang::CXXMethodDecl* pmfFromUnary(clang::Expr *e);
 CLAZYLIB_EXPORT clang::CXXMethodDecl* pmfFromUnary(clang::UnaryOperator *uo);
 
+/**
+ * Returns the varDecl for the 1st argument in a connect call
+ */
+inline clang::ValueDecl *signalSenderForConnect(clang::CallExpr *call)
+{
+    return FunctionUtils::valueDeclForCallArgument(call, 0);
+}
+
+/**
+ * Returns the varDecl for 3rd argument in connects that are passed an explicit
+ * receiver or context QObject.
+ */
+inline clang::ValueDecl *signalReceiverForConnect(clang::CallExpr *call)
+{
+    if (!call || call->getNumArgs() < 5)
+        return nullptr;
+
+    return FunctionUtils::valueDeclForCallArgument(call, 3);
+}
+
+/**
+ * Returns the receiver method, in a PMF connect statement.
+ * The method can be a slot or a signal. If it's a lambda or functor nullptr is returned
+ */
+inline clang::CXXMethodDecl* receiverMethodForConnect(clang::CallExpr *call)
+{
+
+    clang::CXXMethodDecl *receiverMethod = QtUtils::pmfFromConnect(call, 2);
+    if (receiverMethod)
+        return receiverMethod;
+
+    // It's either third or fourth argument
+    return QtUtils::pmfFromConnect(call, 3);
+}
+
 }
 
 #endif
diff --git a/src/TypeUtils.h b/src/TypeUtils.h
index 3a6c002d..9c77bf9b 100644
--- a/src/TypeUtils.h
+++ b/src/TypeUtils.h
@@ -173,6 +173,14 @@ namespace TypeUtils
         return typeAsRecord(pointeeQualType(expr->getType()));
     }
 
+    inline clang::CXXRecordDecl* typeAsRecord(clang::ValueDecl *value)
+    {
+        if (!value)
+            return nullptr;
+
+        return typeAsRecord(pointeeQualType(value->getType()));
+    }
+
     /**
      * Returns the class that the typedef refered by qt is in.
      *
diff --git a/src/checks/level1/const-signal-or-slot.cpp b/src/checks/level1/const-signal-or-slot.cpp
index faa580ae..77fa0783 100644
--- a/src/checks/level1/const-signal-or-slot.cpp
+++ b/src/checks/level1/const-signal-or-slot.cpp
@@ -51,10 +51,7 @@ void ConstSignalOrSlot::VisitStmt(clang::Stmt *stmt)
     if (!QtUtils::isConnect(func) || !QtUtils::connectHasPMFStyle(func))
         return;
 
-    CXXMethodDecl *slot = QtUtils::pmfFromConnect(call, 2);
-    if (!slot) // The slot is either third or fourth argument
-        slot = QtUtils::pmfFromConnect(call, 3);
-
+    CXXMethodDecl *slot =  QtUtils::receiverMethodForConnect(call);
     if (!slot || !slot->isConst() || slot->getReturnType()->isVoidType()) // const and returning void must do something, so not a getter
         return;
 
diff --git a/src/checks/level3/README-thread-with-slots.md b/src/checks/level3/README-thread-with-slots.md
new file mode 100644
index 00000000..e4c0efa4
--- /dev/null
+++ b/src/checks/level3/README-thread-with-slots.md
@@ -0,0 +1,8 @@
+# thread-with-slots
+
+slots in a `QThread` derived classe are usually a code smell, because
+they'll run in the thread where the `QThread` `QObject` lives and not in
+the thread itself.
+
+Disabled by default since it's very hard to avoid for false-positives. You'll
+have to explicitly enable it and check case by case for races.
diff --git a/src/checks/level3/thread-with-slots.cpp b/src/checks/level3/thread-with-slots.cpp
new file mode 100644
index 00000000..590aec65
--- /dev/null
+++ b/src/checks/level3/thread-with-slots.cpp
@@ -0,0 +1,125 @@
+/*
+  This file is part of the clazy static checker.
+
+  Copyright (C) 2017 Sergio Martins <smartins@kde.org>
+
+  This library is free software; you can redistribute it and/or
+  modify it under the terms of the GNU Library General Public
+  License as published by the Free Software Foundation; either
+  version 2 of the License, or (at your option) any later version.
+
+  This library is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  Library General Public License for more details.
+
+  You should have received a copy of the GNU Library General Public License
+  along with this library; see the file COPYING.LIB.  If not, write to
+  the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+  Boston, MA 02110-1301, USA.
+*/
+
+#include "thread-with-slots.h"
+#include "Utils.h"
+#include "HierarchyUtils.h"
+#include "QtUtils.h"
+#include "TypeUtils.h"
+#include "checkmanager.h"
+#include "ClazyContext.h"
+#include "AccessSpecifierManager.h"
+
+#include <clang/AST/AST.h>
+
+using namespace clang;
+using namespace std;
+
+
+static bool hasMutexes(Stmt *body)
+{
+    auto declrefs = HierarchyUtils::getStatements<DeclRefExpr>(body);
+    for (auto declref : declrefs) {
+        ValueDecl *valueDecl = declref->getDecl();
+        if (CXXRecordDecl *record = TypeUtils::typeAsRecord(valueDecl->getType())) {
+            if (record->getName() == "QMutex" || record->getName() == "QBasicMutex") {
+                return true;
+            }
+        }
+    }
+
+    return false;
+}
+
+ThreadWithSlots::ThreadWithSlots(const std::string &name, ClazyContext *context)
+    : CheckBase(name, context)
+{
+    context->enableAccessSpecifierManager();
+}
+
+void ThreadWithSlots::VisitStmt(clang::Stmt *stmt)
+{
+    // Here we catch slots not marked as slots, we warn when the connect is made
+
+    auto callExpr = dyn_cast<CallExpr>(stmt);
+    if (!callExpr || !m_context->accessSpecifierManager)
+        return;
+
+    FunctionDecl *connectFunc = callExpr->getDirectCallee();
+    if (!QtUtils::isConnect(connectFunc))
+        return;
+
+    CXXMethodDecl *slot =  QtUtils::receiverMethodForConnect(callExpr);
+    if (!slot || !TypeUtils::derivesFrom(slot->getParent(), "QThread"))
+        return;
+
+    if (slot->getParent()->getName() == "QThread") // The slots in QThread are thread safe, we're only worried about derived classes
+        return;
+
+    QtAccessSpecifierType specifierType = m_context->accessSpecifierManager->qtAccessSpecifierType(slot);
+    if (specifierType == QtAccessSpecifier_Slot || specifierType == QtAccessSpecifier_Signal)
+        return; // For stuff explicitly marked as slots or signals we use VisitDecl
+
+    emitWarning(slot, "Slot " + slot->getQualifiedNameAsString() + " might not run in the expected thread");
+}
+
+void ThreadWithSlots::VisitDecl(Decl *decl)
+{
+    // Here we catch slots marked as such, and warn when they are declared
+
+    auto method = dyn_cast<CXXMethodDecl>(decl);
+    if (!method || !m_context->accessSpecifierManager || !method->isThisDeclarationADefinition() || !method->hasBody()
+        || !TypeUtils::derivesFrom(method->getParent(), "QThread"))
+        return;
+
+    // The slots in QThread are thread safe, we're only worried about derived classes:
+    if (method->getParent()->getName() == "QThread")
+        return;
+
+    // We're only interested in slots:
+    if (m_context->accessSpecifierManager->qtAccessSpecifierType(method) != QtAccessSpecifier_Slot)
+        return;
+
+    // Look for a mutex, or mutex locker, to avoid some false-positives
+    Stmt *body = method->getBody();
+    if (hasMutexes(body))
+        return;
+
+    // If we use member mutexes, let's not warn either
+    bool accessesNonMutexMember = false;
+    auto memberexprs = HierarchyUtils::getStatements<MemberExpr>(body);
+    for (auto memberexpr : memberexprs) {
+        ValueDecl *valueDecl = memberexpr->getMemberDecl();
+        if (CXXRecordDecl *record = TypeUtils::typeAsRecord(valueDecl->getType())) {
+            if (record->getName() == "QMutex" || record->getName() == "QBasicMutex") {
+                return;
+            }
+        }
+        accessesNonMutexMember = true;
+    }
+
+    if (!accessesNonMutexMember)
+        return;
+
+    emitWarning(method, "Slot " + method->getQualifiedNameAsString() + " might not run in the expected thread");
+}
+
+REGISTER_CHECK("thread-with-slots", ThreadWithSlots, CheckLevel3)
diff --git a/src/checks/level3/thread-with-slots.h b/src/checks/level3/thread-with-slots.h
new file mode 100644
index 00000000..c38cbf06
--- /dev/null
+++ b/src/checks/level3/thread-with-slots.h
@@ -0,0 +1,40 @@
+/*
+  This file is part of the clazy static checker.
+
+  Copyright (C) 2017 Sergio Martins <smartins@kde.org>
+
+  This library is free software; you can redistribute it and/or
+  modify it under the terms of the GNU Library General Public
+  License as published by the Free Software Foundation; either
+  version 2 of the License, or (at your option) any later version.
+
+  This library is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  Library General Public License for more details.
+
+  You should have received a copy of the GNU Library General Public License
+  along with this library; see the file COPYING.LIB.  If not, write to
+  the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+  Boston, MA 02110-1301, USA.
+*/
+
+#ifndef CLAZY_THREAD_WITH_SLOTS_H
+#define CLAZY_THREAD_WITH_SLOTS_H
+
+#include "checkbase.h"
+
+
+/**
+ * See README-thread-with-slots.md for more info.
+ */
+class ThreadWithSlots : public CheckBase
+{
+public:
+    explicit ThreadWithSlots(const std::string &name, ClazyContext *context);
+    void VisitStmt(clang::Stmt *stmt) override;
+    void VisitDecl(clang::Decl *decl) override;
+private:
+};
+
+#endif