Valgrind: Make callgrind work on docker

Callgrind on Remote Linux did not work for years, so its safe to ignore
the remains from the previous remote support and start over.

This here now handles the local case and the case where callgrind
runs together with the debuggee in the same docker container.

Guessing at the output file name is replaced by specifiying it in
advance.

Cross-setups are not yet supported.

Change-Id: I97edeb4eab7475727eddb26d25f8bec650a7eeb9
Reviewed-by: Christian Stenger <christian.stenger@qt.io>

src/plugins/valgrind/callgrind/callgrindcontroller.cpp

@@ -205,56 +205,33 @@ void CallgrindController::controllerProcessClosed(bool success)
 
 void CallgrindController::getLocalDataFile()
 {
-    // we look for callgrind.out.PID, but there may be updated ones called ~.PID.NUM
-    const QString baseFileName = QString("callgrind.out.%1").arg(m_pid);
-    const QString workingDir = m_valgrindRunnable.workingDirectory.toString();
-    // first, set the to-be-parsed file to callgrind.out.PID
-    QString fileName = workingDir.isEmpty() ? baseFileName : (workingDir + '/' + baseFileName);
+    cleanupTempFile();
+    {
+        TemporaryFile dataFile("callgrind.out");
+        dataFile.open();
+        m_hostOutputFile = FilePath::fromString(dataFile.fileName());
+    }
 
-    if (m_valgrindRunnable.device
-            && m_valgrindRunnable.device->type() != ProjectExplorer::Constants::DESKTOP_DEVICE_TYPE) {
 //        ///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 {
-        const QDir dir(workingDir, QString::fromLatin1("%1.*").arg(baseFileName), QDir::Time);
-        const QStringList outputFiles = dir.entryList();
-        // if there are files like callgrind.out.PID.NUM, set it to the most recent one of those
-        if (!outputFiles.isEmpty())
-            fileName = workingDir + '/' + outputFiles.first();
+// [...]
+//    m_sftp = m_ssh->createSftpSession();
+//    connect(m_sftp.get(), &QSsh::SftpSession::commandFinished,
+//            this, &CallgrindController::sftpJobFinished);
+//    connect(m_sftp.get(), &QSsh::SftpSession::started,
+//            this, &CallgrindController::sftpInitialized);
+//    m_sftp->start();
 
-        emit localParseDataAvailable(fileName);
-    }
-}
+    const bool res = m_valgrindOutputFile.copyFile(m_hostOutputFile);
+    QTC_CHECK(res);
 
-void CallgrindController::foundRemoteFile()
-{
-    m_remoteFile = m_findRemoteFile->readAllStandardOutput().trimmed();
-
-    m_sftp = m_ssh->createSftpSession();
-    connect(m_sftp.get(), &QSsh::SftpSession::commandFinished,
-            this, &CallgrindController::sftpJobFinished);
-    connect(m_sftp.get(), &QSsh::SftpSession::started,
-            this, &CallgrindController::sftpInitialized);
-    m_sftp->start();
+    emit localParseDataAvailable(m_hostOutputFile);
 }
 
 void CallgrindController::sftpInitialized()
 {
-    cleanupTempFile();
-    Utils::TemporaryFile dataFile("callgrind.out.");
-    QTC_ASSERT(dataFile.open(), return);
-    m_tempDataFile = dataFile.fileName();
-    dataFile.setAutoRemove(false);
-    dataFile.close();
-
-    m_downloadJob = m_sftp->downloadFile(QString::fromUtf8(m_remoteFile), m_tempDataFile);
+    m_downloadJob = m_sftp->downloadFile(m_valgrindOutputFile.path(), m_hostOutputFile.path());
 }
 
 void CallgrindController::sftpJobFinished(QSsh::SftpJobId job, const QString &error)
@@ -264,15 +241,15 @@ void CallgrindController::sftpJobFinished(QSsh::SftpJobId job, const QString &er
     m_sftp->quit();
 
     if (error.isEmpty())
-        emit localParseDataAvailable(m_tempDataFile);
+        emit localParseDataAvailable(m_hostOutputFile);
 }
 
 void CallgrindController::cleanupTempFile()
 {
-    if (!m_tempDataFile.isEmpty() && QFile::exists(m_tempDataFile))
-        QFile::remove(m_tempDataFile);
+    if (!m_hostOutputFile.isEmpty() && m_hostOutputFile.exists())
+        m_hostOutputFile.removeFile();
 
-    m_tempDataFile.clear();
+    m_hostOutputFile.clear();
 }
 
 void CallgrindController::setValgrindRunnable(const Runnable &runnable)

src/plugins/valgrind/callgrind/callgrindcontroller.h

@@ -62,16 +62,16 @@ public:
     void getLocalDataFile();
     void setValgrindPid(qint64 pid);
     void setValgrindRunnable(const ProjectExplorer::Runnable &runnable);
+    void setValgrindOutputFile(const Utils::FilePath &output) { m_valgrindOutputFile = output; }
 
 signals:
     void finished(Valgrind::Callgrind::CallgrindController::Option option);
-    void localParseDataAvailable(const QString &file);
+    void localParseDataAvailable(const Utils::FilePath &file);
     void statusMessage(const QString &msg);
 
 private:
     void handleControllerProcessError(QProcess::ProcessError);
 
-    void foundRemoteFile();
     void sftpInitialized();
     void sftpJobFinished(QSsh::SftpJobId job, const QString &error);
     void cleanupTempFile();
@@ -88,11 +88,10 @@ private:
 
     // remote callgrind support
     QSsh::SshConnection *m_ssh = nullptr;
-    QString m_tempDataFile;
-    QSsh::SshRemoteProcessPtr m_findRemoteFile;
+    Utils::FilePath m_valgrindOutputFile; // On the device that runs valgrind
+    Utils::FilePath m_hostOutputFile; // On the device that runs creator
     QSsh::SftpSessionPtr m_sftp;
     QSsh::SftpJobId m_downloadJob = 0;
-    QByteArray m_remoteFile;
 };
 
 } // namespace Callgrind

src/plugins/valgrind/callgrind/callgrindparser.cpp

@@ -30,10 +30,10 @@
 #include "callgrindcostitem.h"
 #include "callgrindfunction.h"
 
+#include <utils/filepath.h>
 #include <utils/qtcassert.h>
 #include <utils/stringutils.h>
 
-#include <QFileDevice>
 #include <QHash>
 #include <QVector>
 #include <QStringList>
@@ -41,7 +41,10 @@
 
 // #define DEBUG_PARSER
 
-namespace {
+using namespace Utils;
+
+namespace Valgrind {
+namespace Callgrind {
 
 static void skipSpace(const char **current, const char *end)
 {
@@ -130,10 +133,6 @@ static int parseNameShorthand(const char **current, const char *end)
     return -1; // invalid
 }
 
-}
-
-namespace Valgrind {
-namespace Callgrind {
 
 class Parser::Private
 {
@@ -150,7 +149,7 @@ public:
         delete data;
     }
 
-    void parse(QIODevice *device);
+    void parse(const FilePath &filePath);
     void parseHeader(QIODevice *device);
 
     using NamePair = QPair<qint64, QString>;
@@ -197,20 +196,22 @@ public:
     QSet<Function *> recursiveFunctions;
 };
 
-void Parser::Private::parse(QIODevice *device)
+void Parser::Private::parse(const FilePath &filePath)
 {
     // be sure to clean up existing data before re-allocating
     // the callee might not have taken the parse data
     delete data;
     data = nullptr;
 
-    QString file;
-    if (auto fileDevice = qobject_cast<QFileDevice *>(device))
-        file = fileDevice->fileName();
-    data = new ParseData(file);
-    parseHeader(device);
-    while (!device->atEnd()) {
-        QByteArray line = device->readLine();
+    const QString path = filePath.path(); // FIXME: Works only accidentally for docker
+    QFile file(path);
+    if (!file.open(QIODevice::ReadOnly))
+        qWarning() << "Could not open file for parsing:" << filePath.toUserOutput();
+
+    data = new ParseData(path);
+    parseHeader(&file);
+    while (!file.atEnd()) {
+        const QByteArray line = file.readLine();
         // empty lines actually have no meaning - only fn= starts a new function
         if (line.length() > 1)
             dispatchLine(line);
@@ -639,9 +640,9 @@ void Parser::Private::parseCalledObjectFile(const char *begin, const char *end)
 
 //BEGIN Parser
 
-void Parser::parse(QIODevice *device)
+void Parser::parse(const Utils::FilePath &filePath)
 {
-    d->parse(device);
+    d->parse(filePath);
 }
 
 Parser::Parser()

src/plugins/valgrind/callgrind/callgrindparser.h

@@ -27,9 +27,7 @@
 
 #include <QObject>
 
-QT_BEGIN_NAMESPACE
-class QIODevice;
-QT_END_NAMESPACE
+namespace Utils { class FilePath; }
 
 namespace Valgrind {
 namespace Callgrind {
@@ -56,7 +54,7 @@ public:
     // get and take ownership of the parsing results. If this function is not called the repository
     // will be destroyed when the parser is destroyed. Subsequent calls return null.
     ParseData *takeData();
-    void parse(QIODevice *stream);
+    void parse(const Utils::FilePath &filePath);
 
 signals:
     void parserDataReady();

src/plugins/valgrind/callgrindengine.cpp

@@ -34,11 +34,13 @@
 
 #include <debugger/analyzer/analyzermanager.h>
 
+#include <utils/filepath.h>
 #include <utils/qtcassert.h>
 #include <utils/qtcprocess.h>
 
 using namespace ProjectExplorer;
 using namespace Valgrind::Callgrind;
+using namespace Utils;
 
 namespace Valgrind {
 namespace Internal {
@@ -58,7 +60,7 @@ CallgrindToolRunner::CallgrindToolRunner(RunControl *runControl)
     connect(&m_controller, &CallgrindController::finished,
             this, &CallgrindToolRunner::controllerFinished);
     connect(&m_controller, &CallgrindController::localParseDataAvailable,
-            this, &CallgrindToolRunner::localParseDataAvailable);
+            this, &CallgrindToolRunner::handleLocalParseData);
     connect(&m_controller, &CallgrindController::statusMessage,
             this, &CallgrindToolRunner::showStatusMessage);
 
@@ -71,6 +73,10 @@ CallgrindToolRunner::CallgrindToolRunner(RunControl *runControl)
 
     m_controller.setValgrindRunnable(runnable());
 
+    static int fileCount = 100;
+    m_valgrindOutputFile = runnable().workingDirectory / QString("callgrind.out.f%1").arg(++fileCount);
+    m_controller.setValgrindOutputFile(m_valgrindOutputFile);
+
     setupCallgrindRunner(this);
 }
 
@@ -97,6 +103,8 @@ QStringList CallgrindToolRunner::toolArguments() const
     if (!m_argumentForToggleCollect.isEmpty())
         arguments << m_argumentForToggleCollect;
 
+    arguments << "--callgrind-out-file=" + m_valgrindOutputFile.path();
+
     arguments << Utils::ProcessArgs::splitArgs(m_settings.callgrindArguments.value());
 
     return arguments;
@@ -175,18 +183,11 @@ void CallgrindToolRunner::triggerParse()
     m_controller.getLocalDataFile();
 }
 
-void CallgrindToolRunner::localParseDataAvailable(const QString &file)
+void CallgrindToolRunner::handleLocalParseData(const FilePath &outputFile)
 {
-    // parse the callgrind file
-    QTC_ASSERT(!file.isEmpty(), return);
-    QFile outputFile(file);
     QTC_ASSERT(outputFile.exists(), return);
-    if (outputFile.open(QIODevice::ReadOnly)) {
-        showStatusMessage(tr("Parsing Profile Data..."));
-        m_parser.parse(&outputFile);
-    } else {
-        qWarning() << "Could not open file for parsing:" << outputFile.fileName();
-    }
+    showStatusMessage(tr("Parsing Profile Data..."));
+    m_parser.parse(outputFile);
 }
 
 void CallgrindToolRunner::controllerFinished(CallgrindController::Option option)

src/plugins/valgrind/callgrindengine.h

@@ -70,10 +70,11 @@ private:
     void showStatusMessage(const QString &message);
 
     void triggerParse();
-    void localParseDataAvailable(const QString &file);
+    void handleLocalParseData(const Utils::FilePath &filePath);
     void controllerFinished(Callgrind::CallgrindController::Option option);
 
     bool m_markAsPaused = false;
+    Utils::FilePath m_valgrindOutputFile;
     Callgrind::CallgrindController m_controller;
     Callgrind::Parser m_parser;
     bool m_paused = false;

src/plugins/valgrind/callgrindtool.cpp

@@ -895,7 +895,7 @@ void CallgrindToolPrivate::loadExternalLogFile()
     QCoreApplication::processEvents();
 
     Parser parser;
-    parser.parse(&logFile);
+    parser.parse(filePath);
     takeParserData(parser.takeData());
 }
 

src/plugins/valgrind/valgrindrunner.cpp

@@ -138,8 +138,8 @@ bool ValgrindRunner::Private::run()
     if (HostOsInfo::isMacHost())
         // May be slower to start but without it we get no filenames for symbols.
         cmd.addArg("--dsymutil=yes");
-    cmd.addArg(m_debuggee.command.executable().toString());
-    cmd.addArgs(m_debuggee.command.arguments(), CommandLine::Raw);
+
+    cmd.addCommandLineAsArgs(m_debuggee.command);
 
     emit q->valgrindExecuted(cmd.toUserOutput());
 
@@ -149,10 +149,14 @@ bool ValgrindRunner::Private::run()
     valgrind.environment = m_debuggee.environment;
     valgrind.device = m_device;
 
-    if (m_device->type() == ProjectExplorer::Constants::DESKTOP_DEVICE_TYPE)
+    if (m_device->type() == ProjectExplorer::Constants::DESKTOP_DEVICE_TYPE) {
         m_valgrindProcess.start(valgrind);
-    else
+    } else if (m_device->type() == "DockerDeviceType") {
+        valgrind.device = {};
+        m_valgrindProcess.start(valgrind);
+    } else {
         m_valgrindProcess.start(valgrind, m_device);
+    }
 
     return true;
 }

tests/auto/valgrind/callgrind/callgrindparsertests.cpp

@@ -32,8 +32,9 @@
 #include <valgrind/callgrind/callgrindcostitem.h>
 #include <valgrind/callgrind/callgrindparsedata.h>
 
+#include <utils/filepath.h>
+
 #include <QDebug>
-#include <QFile>
 #include <QString>
 #include <QTest>
 
@@ -103,12 +104,8 @@ void CallgrindParserTests::cleanup()
 
 ParseData* parseDataFile(const QString &dataFile)
 {
-    QFile file(dataFile);
-    Q_ASSERT(file.exists());
-    file.open(QIODevice::ReadOnly);
-
     Parser p;
-    p.parse(&file);
+    p.parse(Utils::FilePath::fromString(dataFile));
 
     return p.takeData();
 }