From 5830641d0f233f09001cdc388367b229b868c25f Mon Sep 17 00:00:00 2001 From: hjk Date: Thu, 26 Aug 2021 06:58:50 +0200 Subject: [PATCH] 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 --- .../callgrind/callgrindcontroller.cpp | 65 ++++++------------- .../valgrind/callgrind/callgrindcontroller.h | 9 ++- .../valgrind/callgrind/callgrindparser.cpp | 35 +++++----- .../valgrind/callgrind/callgrindparser.h | 6 +- src/plugins/valgrind/callgrindengine.cpp | 23 +++---- src/plugins/valgrind/callgrindengine.h | 3 +- src/plugins/valgrind/callgrindtool.cpp | 2 +- src/plugins/valgrind/valgrindrunner.cpp | 12 ++-- .../callgrind/callgrindparsertests.cpp | 9 +-- 9 files changed, 71 insertions(+), 93 deletions(-) diff --git a/src/plugins/valgrind/callgrind/callgrindcontroller.cpp b/src/plugins/valgrind/callgrind/callgrindcontroller.cpp index d76a43c5b7c..31da33e13a1 100644 --- a/src/plugins/valgrind/callgrind/callgrindcontroller.cpp +++ b/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) diff --git a/src/plugins/valgrind/callgrind/callgrindcontroller.h b/src/plugins/valgrind/callgrind/callgrindcontroller.h index 090d34290a3..30e2b6c0f0a 100644 --- a/src/plugins/valgrind/callgrind/callgrindcontroller.h +++ b/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 diff --git a/src/plugins/valgrind/callgrind/callgrindparser.cpp b/src/plugins/valgrind/callgrind/callgrindparser.cpp index 68492394364..1288bd16846 100644 --- a/src/plugins/valgrind/callgrind/callgrindparser.cpp +++ b/src/plugins/valgrind/callgrind/callgrindparser.cpp @@ -30,10 +30,10 @@ #include "callgrindcostitem.h" #include "callgrindfunction.h" +#include #include #include -#include #include #include #include @@ -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; @@ -197,20 +196,22 @@ public: QSet 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(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() diff --git a/src/plugins/valgrind/callgrind/callgrindparser.h b/src/plugins/valgrind/callgrind/callgrindparser.h index 122e3323387..b07313423b3 100644 --- a/src/plugins/valgrind/callgrind/callgrindparser.h +++ b/src/plugins/valgrind/callgrind/callgrindparser.h @@ -27,9 +27,7 @@ #include -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(); diff --git a/src/plugins/valgrind/callgrindengine.cpp b/src/plugins/valgrind/callgrindengine.cpp index aba8997a2fd..61ecb13a3e2 100644 --- a/src/plugins/valgrind/callgrindengine.cpp +++ b/src/plugins/valgrind/callgrindengine.cpp @@ -34,11 +34,13 @@ #include +#include #include #include 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) diff --git a/src/plugins/valgrind/callgrindengine.h b/src/plugins/valgrind/callgrindengine.h index 50c141efc94..dfee79d95b2 100644 --- a/src/plugins/valgrind/callgrindengine.h +++ b/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; diff --git a/src/plugins/valgrind/callgrindtool.cpp b/src/plugins/valgrind/callgrindtool.cpp index a169bee9e12..2973caa0932 100644 --- a/src/plugins/valgrind/callgrindtool.cpp +++ b/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()); } diff --git a/src/plugins/valgrind/valgrindrunner.cpp b/src/plugins/valgrind/valgrindrunner.cpp index cfcf63a350b..7ac0d506205 100644 --- a/src/plugins/valgrind/valgrindrunner.cpp +++ b/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; } diff --git a/tests/auto/valgrind/callgrind/callgrindparsertests.cpp b/tests/auto/valgrind/callgrind/callgrindparsertests.cpp index 7ee2c07d452..ca87cd1d458 100644 --- a/tests/auto/valgrind/callgrind/callgrindparsertests.cpp +++ b/tests/auto/valgrind/callgrind/callgrindparsertests.cpp @@ -32,8 +32,9 @@ #include #include +#include + #include -#include #include #include @@ -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(); }