Valgrind: Fix Memcheck startup on remote linux

Remote CallGrind is still dysfunctional.

Change-Id: Ib9ab537dc068c94c7e61ac48b1a4b9d655ccb60f
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/plugins/debugger/debuggerruncontrol.cpp

@@ -468,12 +468,24 @@ static DebuggerRunConfigurationAspect *debuggerAspect(const RunControl *runContr
     return runControl->runConfiguration()->extraAspect<DebuggerRunConfigurationAspect>();
 }
 
+static bool cppDebugging(const RunControl *runControl)
+{
+    auto aspect = debuggerAspect(runControl);
+    return aspect ? aspect->useCppDebugger() : true; // For cases like valgrind-with-gdb.
+}
+
+static bool qmlDebugging(const RunControl *runControl)
+{
+    auto aspect = debuggerAspect(runControl);
+    return aspect ? aspect->useCppDebugger() : false; // For cases like valgrind-with-gdb.
+}
+
 /// DebuggerRunTool
 
 DebuggerRunTool::DebuggerRunTool(RunControl *runControl)
     : RunWorker(runControl),
-      m_isCppDebugging(debuggerAspect(runControl)->useCppDebugger()),
-      m_isQmlDebugging(debuggerAspect(runControl)->useQmlDebugger())
+      m_isCppDebugging(cppDebugging(runControl)),
+      m_isQmlDebugging(qmlDebugging(runControl))
 {
     setDisplayName("DebuggerRunTool");
 }

src/plugins/valgrind/callgrind/callgrindcontroller.cpp

@@ -192,15 +192,15 @@ void CallgrindController::getLocalDataFile()
     QString fileName = workingDir.isEmpty() ? baseFileName : (workingDir + QLatin1Char('/') + baseFileName);
 
     if (!m_valgrindProc->isLocal()) {
-        ///TODO: error handling
-        emit statusMessage(tr("Downloading remote profile data..."));
-        m_ssh = m_valgrindProc->connection();
-        // if there are files like callgrind.out.PID.NUM, set it to the most recent one of those
-        QString cmd = QString::fromLatin1("ls -t %1* | head -n 1").arg(fileName);
-        m_findRemoteFile = m_ssh->createRemoteProcess(cmd.toUtf8());
-        connect(m_findRemoteFile.data(), &QSsh::SshRemoteProcess::readyReadStandardOutput,
-                this, &CallgrindController::foundRemoteFile);
-        m_findRemoteFile->start();
+//        ///TODO: error handling
+//        emit statusMessage(tr("Downloading remote profile data..."));
+//        m_ssh = m_valgrindProc->connection();
+//        // if there are files like callgrind.out.PID.NUM, set it to the most recent one of those
+//        QString cmd = QString::fromLatin1("ls -t %1* | head -n 1").arg(fileName);
+//        m_findRemoteFile = m_ssh->createRemoteProcess(cmd.toUtf8());
+//        connect(m_findRemoteFile.data(), &QSsh::SshRemoteProcess::readyReadStandardOutput,
+//                this, &CallgrindController::foundRemoteFile);
+//        m_findRemoteFile->start();
     } else {
         QDir dir(workingDir, QString::fromLatin1("%1.*").arg(baseFileName), QDir::Time);
         QStringList outputFiles = dir.entryList();

src/plugins/valgrind/memcheckengine.cpp

@@ -51,8 +51,33 @@ using namespace Valgrind::XmlProtocol;
 namespace Valgrind {
 namespace Internal {
 
+class LocalAddressFinder : public RunWorker
+{
+public:
+    LocalAddressFinder(RunControl *runControl, QHostAddress *localServerAddress)
+        : RunWorker(runControl), connection(device()->sshParameters())
+    {
+        connect(&connection, &QSsh::SshConnection::connected, this, [this, localServerAddress] {
+            *localServerAddress = connection.connectionInfo().localAddress;
+            reportStarted();
+        });
+        connect(&connection, &QSsh::SshConnection::error, this, [this] {
+            reportFailure();
+        });
+    }
+
+    void start() override
+    {
+        connection.connectToHost();
+    }
+
+    QSsh::SshConnection connection;
+};
+
 MemcheckToolRunner::MemcheckToolRunner(RunControl *runControl, bool withGdb)
-    : ValgrindToolRunner(runControl), m_withGdb(withGdb)
+    : ValgrindToolRunner(runControl),
+      m_withGdb(withGdb),
+      m_localServerAddress(QHostAddress::LocalHost)
 {
     setDisplayName("MemcheckToolRunner");
     connect(m_runner.parser(), &XmlProtocol::ThreadedParser::error,
@@ -65,11 +90,15 @@ MemcheckToolRunner::MemcheckToolRunner(RunControl *runControl, bool withGdb)
                 this, &MemcheckToolRunner::startDebugger);
         connect(&m_runner, &ValgrindRunner::logMessageReceived,
                 this, &MemcheckToolRunner::appendLog);
-        m_runner.disableXml();
+//        m_runner.disableXml();
     } else {
         connect(m_runner.parser(), &XmlProtocol::ThreadedParser::internalError,
                 this, &MemcheckToolRunner::internalParserError);
     }
+
+    // We need a real address to connect to from the outside.
+    if (device()->type() != ProjectExplorer::Constants::DESKTOP_DEVICE_TYPE)
+        addDependency(new LocalAddressFinder(runControl, &m_localServerAddress));
 }
 
 QString MemcheckToolRunner::progressTitle() const
@@ -79,10 +108,7 @@ QString MemcheckToolRunner::progressTitle() const
 
 void MemcheckToolRunner::start()
 {
-//    MemcheckTool::engineStarting(this);
-
-    appendMessage(tr("Analyzing memory of %1").arg(executable()) + QLatin1Char('\n'),
-                        Utils::NormalMessageFormat);
+    m_runner.setLocalServerAddress(m_localServerAddress);
     ValgrindToolRunner::start();
 }
 
@@ -149,12 +175,10 @@ void MemcheckToolRunner::startDebugger()
     sp.useContinueInsteadOfRun = true;
     sp.expectedSignals.append("SIGTRAP");
 
-    QString errorMessage;
-    auto gdbRunControl = new RunControl(nullptr, ProjectExplorer::Constants::DEBUG_RUN_MODE);
-    (void) new Debugger::DebuggerRunTool(gdbRunControl, sp, &errorMessage);
-    connect(gdbRunControl, &RunControl::finished,
-            gdbRunControl, &RunControl::deleteLater);
-    gdbRunControl->initiateStart();
+    auto gdbWorker = new Debugger::DebuggerRunTool(runControl());
+    gdbWorker->setStartParameters(sp);
+    gdbWorker->initiateStart();
+    connect(runControl(), &RunControl::finished, gdbWorker, &RunControl::deleteLater);
 }
 
 void MemcheckToolRunner::appendLog(const QByteArray &data)

src/plugins/valgrind/memcheckengine.h

@@ -60,6 +60,7 @@ private:
     void appendLog(const QByteArray &data);
 
     const bool m_withGdb;
+    QHostAddress m_localServerAddress;
 };
 
 } // namespace Internal

src/plugins/valgrind/valgrindengine.cpp

@@ -106,6 +106,7 @@ void ValgrindToolRunner::stop()
 {
     m_isStopping = true;
     m_runner.stop();
+    reportStopped(); // FIXME: Restrict to non-running scenarios?
 }
 
 QString ValgrindToolRunner::executable() const

src/plugins/valgrind/valgrindprocess.h

@@ -66,7 +66,6 @@ public:
     ProjectExplorer::IDevice::ConstPtr device() const { return m_device; }
 
     qint64 pid() const;
-    QSsh::SshConnection *connection() const;
     bool isLocal() const;
 
 signals:
@@ -74,33 +73,24 @@ signals:
     void finished(int, QProcess::ExitStatus);
     void error(QProcess::ProcessError);
     void processOutput(const QString &, Utils::OutputFormat format);
-    void localHostAddressRetrieved(const QHostAddress &localHostAddress);
 
 private:
-    void handleRemoteStderr();
-    void handleRemoteStdout();
-    void handleError(QSsh::SshError);
+    void handleRemoteStderr(const QByteArray &b);
+    void handleRemoteStdout(const QByteArray &b);
 
-    void closed(int);
-    void connected();
+    void closed(bool success);
     void localProcessStarted();
     void remoteProcessStarted();
-    void findPIDOutputReceived();
+    void findPIDOutputReceived(const QByteArray &out);
 
     QString argumentString(Utils::OsType osType) const;
 
     ProjectExplorer::StandardRunnable m_debuggee;
-    ProjectExplorer::ApplicationLauncher m_localProcess;
-    qint64 m_pid;
+    ProjectExplorer::ApplicationLauncher m_valgrindProcess;
+    qint64 m_pid = 0;
     ProjectExplorer::IDevice::ConstPtr m_device;
 
-    struct Remote {
-        QSsh::SshConnection *m_connection;
-        QSsh::SshRemoteProcess::Ptr m_process;
-        QString m_errorString;
-        QProcess::ProcessError m_error;
-        QSsh::SshRemoteProcess::Ptr m_findPID;
-    } m_remote;
+    ProjectExplorer::ApplicationLauncher m_findPID;
 
     QSsh::SshConnectionParameters m_params;
     QString m_valgrindExecutable;

src/plugins/valgrind/valgrindrunner.cpp

@@ -39,6 +39,7 @@
 #include <ssh/sshremoteprocess.h>
 
 #include <QEventLoop>
+#include <QNetworkInterface>
 #include <QTcpServer>
 #include <QTcpSocket>
 
@@ -49,6 +50,7 @@ namespace Valgrind {
 class ValgrindRunner::Private
 {
 public:
+    QHostAddress localServerAddress;
     ValgrindProcess *process = nullptr;
     QProcess::ProcessChannelMode channelMode = QProcess::SeparateChannels;
     bool finished = false;
@@ -125,6 +127,11 @@ void ValgrindRunner::setProcessChannelMode(QProcess::ProcessChannelMode mode)
     d->channelMode = mode;
 }
 
+void ValgrindRunner::setLocalServerAddress(const QHostAddress &localServerAddress)
+{
+    d->localServerAddress = localServerAddress;
+}
+
 void ValgrindRunner::setDevice(const IDevice::ConstPtr &device)
 {
     d->device = device;
@@ -152,10 +159,8 @@ void ValgrindRunner::setToolName(const QString &toolName)
 
 bool ValgrindRunner::start()
 {
-    // FIXME: Remove hack.
-    if (d->tool == "memcheck"
-            && device()->type() == ProjectExplorer::Constants::DESKTOP_DEVICE_TYPE) {
-        if (!startServers(QHostAddress::LocalHost))
+    if (!d->localServerAddress.isNull()) {
+        if (!startServers())
             return false;
         setValgrindArguments(memcheckLogArguments() + valgrindArguments());
     }
@@ -176,8 +181,6 @@ bool ValgrindRunner::start()
                      this, &ValgrindRunner::processFinished);
     QObject::connect(d->process, &ValgrindProcess::error,
                      this, &ValgrindRunner::processError);
-    QObject::connect(d->process, &ValgrindProcess::localHostAddressRetrieved,
-                     this, &ValgrindRunner::localHostAddressRetrieved);
 
     d->process->run(d->debuggee.runMode);
 
@@ -214,14 +217,6 @@ void ValgrindRunner::processFinished(int ret, QProcess::ExitStatus status)
         emit processErrorReceived(errorString(), d->process->processError());
 }
 
-void ValgrindRunner::localHostAddressRetrieved(const QHostAddress &localHostAddress)
-{
-    if (startServers(localHostAddress)) {
-        setValgrindArguments(memcheckLogArguments() + valgrindArguments());
-        valgrindProcess()->setValgrindArguments(fullValgrindArguments());
-    }
-}
-
 QString ValgrindRunner::errorString() const
 {
     return d->process ? d->process->errorString() : QString();
@@ -274,10 +269,10 @@ void ValgrindRunner::readLogSocket()
     emit logMessageReceived(d->logSocket->readAll());
 }
 
-bool ValgrindRunner::startServers(const QHostAddress &localHostAddress)
+bool ValgrindRunner::startServers()
 {
-    bool check = d->xmlServer.listen(localHostAddress);
-    const QString ip = localHostAddress.toString();
+    bool check = d->xmlServer.listen(d->localServerAddress);
+    const QString ip = d->localServerAddress.toString();
     if (!check) {
         emit processErrorReceived(tr("XmlServer on %1:").arg(ip) + ' '
                                   + d->xmlServer.errorString(), QProcess::FailedToStart );
@@ -286,7 +281,7 @@ bool ValgrindRunner::startServers(const QHostAddress &localHostAddress)
     d->xmlServer.setMaxPendingConnections(1);
     connect(&d->xmlServer, &QTcpServer::newConnection,
             this, &ValgrindRunner::xmlSocketConnected);
-    check = d->logServer.listen(localHostAddress);
+    check = d->logServer.listen(d->localServerAddress);
     if (!check) {
         emit processErrorReceived(tr("LogServer on %1:").arg(ip) + ' '
                                   + d->logServer.errorString(), QProcess::FailedToStart );
@@ -298,16 +293,33 @@ bool ValgrindRunner::startServers(const QHostAddress &localHostAddress)
     return true;
 }
 
+static void handleSocketParameter(const QString &prefix, const QTcpServer &tcpServer,
+                            bool *useXml, QStringList *arguments)
+{
+    QHostAddress serverAddress = tcpServer.serverAddress();
+    if (serverAddress.protocol() != QAbstractSocket::IPv4Protocol) {
+        // Report will end up in the Application Output pane, i.e. not have
+        // clickable items, but that's better than nothing.
+        qWarning("Need IPv4 for valgrind");
+        *useXml = false;
+    } else {
+        *arguments << QString("%1=%2:%3").arg(prefix).arg(serverAddress.toString())
+                                         .arg(tcpServer.serverPort());
+    }
+}
+
 QStringList ValgrindRunner::memcheckLogArguments() const
 {
-    QStringList arguments;
-    if (!d->disableXml)
-        arguments << QLatin1String("--xml=yes");
-    arguments << QString::fromLatin1("--xml-socket=%1:%2")
-                 .arg(d->xmlServer.serverAddress().toString()).arg(d->xmlServer.serverPort())
-              << QLatin1String("--child-silent-after-fork=yes")
-              << QString::fromLatin1("--log-socket=%1:%2")
-                 .arg(d->logServer.serverAddress().toString()).arg(d->logServer.serverPort());
+    bool enableXml = !d->disableXml;
+
+    QStringList arguments = {"--child-silent-after-fork=yes"};
+
+    handleSocketParameter("--xml-socket", d->xmlServer, &enableXml, &arguments);
+    handleSocketParameter("--log-socket", d->logServer, &enableXml, &arguments);
+
+    if (enableXml)
+        arguments << "--xml=yes";
+
     return arguments;
 }
 

src/plugins/valgrind/valgrindrunner.h

@@ -56,6 +56,7 @@ public:
     void setValgrindArguments(const QStringList &toolArguments);
     void setDebuggee(const ProjectExplorer::StandardRunnable &debuggee) ;
     void setProcessChannelMode(QProcess::ProcessChannelMode mode);
+    void setLocalServerAddress(const QHostAddress &localServerAddress);
 
     void setDevice(const ProjectExplorer::IDevice::ConstPtr &device);
     ProjectExplorer::IDevice::ConstPtr device() const;
@@ -84,7 +85,7 @@ signals:
     void extraProcessFinished();
 
 private:
-    bool startServers(const QHostAddress &localHostAddress);
+    bool startServers();
     QStringList memcheckLogArguments() const;
 
     void processError(QProcess::ProcessError);