peek into file before open

.gitignore

@@ -5,7 +5,7 @@ build*/
 elfutils.build.tar.bz2
 rustc_demangle.build.tar.bz2
 d-demangle.tar.gz
-*.actual
+*.actual*
 *.orig
 *.cache
 scripts/output

src/parsers/perf/perfparser.cpp

@@ -1476,6 +1476,7 @@ PerfParser::~PerfParser() = default;
 
 bool PerfParser::initParserArgs(const QString& path)
 {
+    // check for common file issues
     const auto info = QFileInfo(path);
     if (!info.exists()) {
         emit parsingFailed(tr("File '%1' does not exist.").arg(path));
@@ -1490,16 +1491,32 @@ bool PerfParser::initParserArgs(const QString& path)
         return false;
     }
 
+    // peek into file header
+    const auto filename = decompressIfNeeded(path);
+    QFile file(filename);
+    file.open(QIODevice::ReadOnly);
+    if (file.peek(8) != "PERFILE2" && file.peek(11) != "QPERFSTREAM") {
+        if (file.peek(8) == "PERFFILE") {
+            emit parsingFailed(tr("Failed to parse file %1: %2").arg(path, tr("Unsupported V1 perf data")));
+        } else {
+            emit parsingFailed(tr("Failed to parse file %1: %2").arg(path, tr("File format unknown")));
+        }
+        file.close();
+        return false;
+    }
+    file.close();
+
+    // check perfparser and set initial values
     auto parserBinary = Util::perfParserBinaryPath();
     if (parserBinary.isEmpty()) {
         emit parsingFailed(tr("Failed to find hotspot-perfparser binary."));
         return false;
     }
 
-    auto parserArgs = [this](const QString& filename) {
+    auto parserArgs = [](const QString& filename) {
         const auto settings = Settings::instance();
-        QStringList parserArgs = {QStringLiteral("--input"), decompressIfNeeded(filename),
-                                  QStringLiteral("--max-frames"), QStringLiteral("1024")};
+        QStringList parserArgs = {QStringLiteral("--input"), filename, QStringLiteral("--max-frames"),
+                                  QStringLiteral("1024")};
         const auto sysroot = settings->sysroot();
         if (!sysroot.isEmpty()) {
             parserArgs += {QStringLiteral("--sysroot"), sysroot};
@@ -1531,7 +1548,7 @@ bool PerfParser::initParserArgs(const QString& path)
         return parserArgs;
     };
 
-    m_parserArgs = parserArgs(path);
+    m_parserArgs = parserArgs(filename);
     m_parserBinary = parserBinary;
     return true;
 }
@@ -1584,22 +1601,23 @@ void PerfParser::startParseFile(const QString& path)
             emit parsingFinished();
         };
 
-        if (path.endsWith(QLatin1String(".perfparser"))) {
-            QFile file(path);
-            if (!file.open(QIODevice::ReadOnly)) {
-                emit parsingFailed(tr("Failed to open file %1: %2").arg(path, file.errorString()));
-                return;
-            }
+        // note: file is always readable and in supported format here,
+        //        already validated in initParserArgs()
+        QFile file(path);
+        file.open(QIODevice::ReadOnly);
+        if (file.peek(11) == "QPERFSTREAM") {
             d.setInput(&file);
             while (!file.atEnd() && !d.stopRequested) {
                 if (!d.tryParse()) {
-                    emit parsingFailed(tr("Failed to parse file"));
+                    // TODO: provide reason
+                    emit parsingFailed(tr("Failed to parse file %1: %2").arg(path, QStringLiteral("Unknown reason")));
                     return;
                 }
             }
             finalize();
             return;
         }
+        file.close();
 
         QProcess process;
         process.setProcessEnvironment(perfparserEnvironment(debuginfodUrls));

tests/integrationtests/tst_perfparser.cpp

@@ -19,13 +19,16 @@
 #include "perfparser.h"
 #include "perfrecord.h"
 #include "recordhost.h"
-#include "unistd.h"
 #include "util.h"
 
 #include "../testutils.h"
 #include <hotspot-config.h>
 
-#include <exception>
+#if QT_VERSION < QT_VERSION_CHECK(6, 3, 0)
+// workaround issues with string literals in QTest that we cannot workaround locally
+// this was fixed upstream, see: https://codereview.qt-project.org/c/qt/qtbase/+/354227
+// clazy:excludeall=qstring-allocations
+#endif
 
 namespace {
 template<typename T>
@@ -187,6 +190,102 @@ private slots:
         m_machineHostName = QSysInfo::machineHostName();
     }
 
+    void testFileErrorHandling_data()
+    {
+        QTest::addColumn<QString>("perfFile");
+        QTest::addColumn<QString>("errorMessagePart");
+
+        QTest::addRow("missing file") << QStringLiteral("not_here") << QStringLiteral("does not exist");
+        QTest::addRow("not a file") << QStringLiteral("../..") << QStringLiteral("is not a file");
+
+        QTest::addRow("permissions") << QString() << QStringLiteral("not readable");
+    }
+
+    void testFileErrorHandling()
+    {
+
+        PerfParser parser(this);
+        QSignalSpy parsingFailedSpy(&parser, &PerfParser::parsingFailed);
+
+        QFETCH(QString, perfFile);
+        QFETCH(QString, errorMessagePart);
+
+        QTemporaryFile tempFile;
+        if (perfFile.isEmpty()) {
+            tempFile.open();
+            tempFile.write("test content");
+            tempFile.close();
+            tempFile.setPermissions({}); // drop all permissons
+            perfFile = tempFile.fileName();
+        }
+
+        parser.initParserArgs(perfFile);
+        QCOMPARE(parsingFailedSpy.count(), 1);
+        auto message = parsingFailedSpy.takeFirst().at(0).toString();
+        QVERIFY(message.contains(perfFile));
+        QVERIFY(message.contains(errorMessagePart));
+    }
+
+    void testFileContent_data()
+    {
+        QTest::addColumn<QString>("perfFile");
+        QTest::addColumn<QString>("errorMessagePart");
+        QTest::addColumn<int>("waitTime");
+
+        const auto perfData = QFINDTESTDATA("file_content/true.perfparser");
+        QTest::addRow("pre-exported perfparser") << perfData << QString() << 2000;
+        const auto perfDataSomeName = QStringLiteral("fruitper");
+        QFile::copy(perfData, perfDataSomeName); // we can ignore errors (file exist) here
+        QTest::addRow("pre-exported perfparser \"bad extension\"") << perfDataSomeName << QString() << 2000;
+        QTest::addRow("no expected magic header")
+            << QFINDTESTDATA("tst_perfparser.cpp") << QStringLiteral("File format unknown") << 1000;
+        QTest::addRow("PERF v1") << QFINDTESTDATA("file_content/perf.data.true.v1") << QStringLiteral("V1 perf data")
+                                 << 1000;
+
+        // TODO: check why we need this long waittime
+        QTest::addRow("PERF v2") << QFINDTESTDATA("file_content/perf.data.true.v2") << QString() << 9000;
+#if KFArchive_FOUND
+        QTest::addRow("PERF v2, gzipped") << QFINDTESTDATA("file_content/perf.data.true.v2.gz") << QString() << 10000;
+#endif
+    }
+
+    void testFileContent()
+    {
+        // setting the application path as the checked perf files recorded a `true` binary which commonly
+        // is not available in the same place (and we don't get the any reasonable parser output in this case)
+        // the same place as the tests are run
+        Settings::instance()->setAppPath(
+            QFileInfo(QStandardPaths::findExecutable(QStringLiteral("true"))).dir().path());
+        // add extra paths to at least allow manually including the matched libc.so/ld.so during a test
+        Settings::instance()->setExtraLibPaths(QFINDTESTDATA("file_content"));
+
+        PerfParser parser(this);
+        QSignalSpy parsingFailedSpy(&parser, &PerfParser::parsingFailed);
+        QSignalSpy parsingFinishedSpy(&parser, &PerfParser::parsingFinished);
+
+        QFETCH(QString, perfFile);
+        QFETCH(QString, errorMessagePart);
+        QFETCH(int, waitTime);
+
+        QVERIFY(!perfFile.isEmpty() && QFile::exists(perfFile));
+        parser.startParseFile(perfFile);
+
+        if (errorMessagePart.isEmpty()) {
+            // if we don't expect an error message (Null String created by `QString()`)
+            // then expect a finish within the given time frame
+            QTRY_COMPARE_WITH_TIMEOUT(parsingFinishedSpy.count(), 1, waitTime);
+            QCOMPARE(parsingFailedSpy.count(), 0);
+        } else {
+            // otherwise wait for failed parsing, the check for if the required part is
+            // found in the error message (we only check a part to allow adjustments later)
+            QTRY_COMPARE_WITH_TIMEOUT(parsingFailedSpy.count(), 1, waitTime);
+            QCOMPARE(parsingFinishedSpy.count(), 0);
+            const auto message = parsingFailedSpy.takeFirst().at(0).toString();
+            QVERIFY(message.contains(errorMessagePart));
+            QVERIFY(message.contains(perfFile));
+        }
+    }
+
     void testCppInliningNoOptions()
     {
         const QStringList perfOptions;
@@ -744,11 +843,11 @@ private slots:
                 + QStringList {QStringLiteral("-c"), QStringLiteral("1000000"), QStringLiteral("--no-buildid-cache")},
             fileName, false, exePath, exeOptions);
 
-        VERIFY_OR_THROW(recordingFinishedSpy.wait(10000));
+        QVERIFY(recordingFinishedSpy.wait(10000));
 
-        COMPARE_OR_THROW(recordingFailedSpy.count(), 0);
-        COMPARE_OR_THROW(recordingFinishedSpy.count(), 1);
-        COMPARE_OR_THROW(QFileInfo::exists(fileName), true);
+        QCOMPARE(recordingFailedSpy.count(), 0);
+        QCOMPARE(recordingFinishedSpy.count(), 1);
+        QCOMPARE(QFileInfo::exists(fileName), true);
 
         m_perfCommand = perf.perfCommand();
     }
@@ -771,7 +870,7 @@ private slots:
                     r = p;
                 }
             }
-            VERIFY_OR_THROW(hasCost);
+            QVERIFY(hasCost);
         }
         for (const auto& child : row.children) {
             validateCosts(costs, child);
@@ -793,39 +892,38 @@ private slots:
 
         parser.startParseFile(fileName);
 
-        VERIFY_OR_THROW(parsingFinishedSpy.wait(6000));
+        QVERIFY(parsingFinishedSpy.wait(6000));
 
         // Verify that the test passed
-        COMPARE_OR_THROW(parsingFailedSpy.count(), 0);
-        COMPARE_OR_THROW(parsingFinishedSpy.count(), 1);
+        QCOMPARE(parsingFailedSpy.count(), 0);
+        QCOMPARE(parsingFinishedSpy.count(), 1);
 
         // Verify the summary data isn't empty
-        COMPARE_OR_THROW(summaryDataSpy.count(), 1);
+        QCOMPARE(summaryDataSpy.count(), 1);
         QList<QVariant> summaryDataArgs = summaryDataSpy.takeFirst();
         m_summaryData = qvariant_cast<Data::Summary>(summaryDataArgs.at(0));
-        COMPARE_OR_THROW(m_perfCommand, m_summaryData.command);
-        VERIFY_OR_THROW(m_summaryData.sampleCount > 0);
-        VERIFY_OR_THROW(m_summaryData.applicationTime.delta() > 0);
-        VERIFY_OR_THROW(m_summaryData.cpusAvailable > 0);
-        COMPARE_OR_THROW(m_summaryData.processCount, quint32(1)); // for now we always have a single process
-        VERIFY_OR_THROW(m_summaryData.threadCount > 0); // and at least one thread
-        COMPARE_OR_THROW(m_summaryData.cpuArchitecture, m_cpuArchitecture);
-        COMPARE_OR_THROW(m_summaryData.linuxKernelVersion, m_linuxKernelVersion);
-        COMPARE_OR_THROW(m_summaryData.hostName, m_machineHostName);
+        QCOMPARE(m_perfCommand, m_summaryData.command);
+        QVERIFY(m_summaryData.sampleCount > 0);
+        QVERIFY(m_summaryData.applicationTime.delta() > 0);
+        QVERIFY(m_summaryData.cpusAvailable > 0);
+        QCOMPARE(m_summaryData.processCount, quint32(1)); // for now we always have a single process
+        QVERIFY(m_summaryData.threadCount > 0); // and at least one thread
+        QCOMPARE(m_summaryData.cpuArchitecture, m_cpuArchitecture);
+        QCOMPARE(m_summaryData.linuxKernelVersion, m_linuxKernelVersion);
+        QCOMPARE(m_summaryData.hostName, m_machineHostName);
 
         if (checkFrequency) {
             // Verify the sample frequency is acceptable, greater than 500Hz
             double frequency = (1E9 * m_summaryData.sampleCount) / m_summaryData.applicationTime.delta();
-            VERIFY_OR_THROW2(frequency > 500,
-                             qPrintable(QLatin1String("Low Frequency: ") + QString::number(frequency)));
+            QVERIFY2(frequency > 500, qPrintable(QLatin1String("Low Frequency: ") + QString::number(frequency)));
         }
 
         // Verify the top Bottom-Up symbol result contains the expected data
-        COMPARE_OR_THROW(bottomUpDataSpy.count(), 1);
+        QCOMPARE(bottomUpDataSpy.count(), 1);
         QList<QVariant> bottomUpDataArgs = bottomUpDataSpy.takeFirst();
         m_bottomUpData = bottomUpDataArgs.at(0).value<Data::BottomUpResults>();
         validateCosts(m_bottomUpData.costs, m_bottomUpData.root);
-        VERIFY_OR_THROW(m_bottomUpData.root.children.count() > 0);
+        QVERIFY(m_bottomUpData.root.children.count() > 0);
 
         if (topBottomUpSymbol.isValid()) {
             int bottomUpTopIndex = maxElementTopIndex(m_bottomUpData);
@@ -834,14 +932,14 @@ private slots:
             if (actualTopBottomUpSymbol == ComparableSymbol(QStringLiteral("__FRAME_END__"), {})) {
                 QEXPECT_FAIL("", "bad symbol offsets - bug in mmap handling or symbol cache?", Continue);
             }
-            COMPARE_OR_THROW(actualTopBottomUpSymbol, topBottomUpSymbol);
+            QCOMPARE(actualTopBottomUpSymbol, topBottomUpSymbol);
         }
 
         // Verify the top Top-Down symbol result contains the expected data
-        COMPARE_OR_THROW(topDownDataSpy.count(), 1);
+        QCOMPARE(topDownDataSpy.count(), 1);
         QList<QVariant> topDownDataArgs = topDownDataSpy.takeFirst();
         m_topDownData = topDownDataArgs.at(0).value<Data::TopDownResults>();
-        VERIFY_OR_THROW(m_topDownData.root.children.count() > 0);
+        QVERIFY(m_topDownData.root.children.count() > 0);
 
         if (topTopDownSymbol.isValid()
             && QLatin1String(QTest::currentTestFunction()) != QLatin1String("testCppRecursionCallGraphDwarf")) {
@@ -851,40 +949,40 @@ private slots:
             if (actualTopTopDownSymbol == ComparableSymbol(QStringLiteral("__FRAME_END__"), {})) {
                 QEXPECT_FAIL("", "bad symbol offsets - bug in mmap handling or symbol cache?", Continue);
             }
-            COMPARE_OR_THROW(actualTopTopDownSymbol, topTopDownSymbol);
+            QCOMPARE(actualTopTopDownSymbol, topTopDownSymbol);
         }
 
         // Verify the Caller/Callee data isn't empty
-        COMPARE_OR_THROW(callerCalleeDataSpy.count(), 1);
+        QCOMPARE(callerCalleeDataSpy.count(), 1);
         QList<QVariant> callerCalleeDataArgs = callerCalleeDataSpy.takeFirst();
         m_callerCalleeData = callerCalleeDataArgs.at(0).value<Data::CallerCalleeResults>();
-        VERIFY_OR_THROW(m_callerCalleeData.entries.count() > 0);
+        QVERIFY(m_callerCalleeData.entries.count() > 0);
 
         // Verify that no individual cost in the Caller/Callee data is greater than the total cost of all samples
         for (const auto& entry : std::as_const(m_callerCalleeData.entries)) {
-            VERIFY_OR_THROW(m_callerCalleeData.inclusiveCosts.cost(0, entry.id)
-                            <= static_cast<qint64>(m_summaryData.costs[0].totalPeriod));
+            QVERIFY(m_callerCalleeData.inclusiveCosts.cost(0, entry.id)
+                    <= static_cast<qint64>(m_summaryData.costs[0].totalPeriod));
         }
 
         // Verify that the events data is not empty and somewhat sane
-        COMPARE_OR_THROW(eventsDataSpy.count(), 1);
+        QCOMPARE(eventsDataSpy.count(), 1);
         m_eventData = eventsDataSpy.first().first().value<Data::EventResults>();
-        VERIFY_OR_THROW(!m_eventData.stacks.isEmpty());
-        VERIFY_OR_THROW(!m_eventData.threads.isEmpty());
-        COMPARE_OR_THROW(static_cast<quint32>(m_eventData.threads.size()), m_summaryData.threadCount);
+        QVERIFY(!m_eventData.stacks.isEmpty());
+        QVERIFY(!m_eventData.threads.isEmpty());
+        QCOMPARE(static_cast<quint32>(m_eventData.threads.size()), m_summaryData.threadCount);
         for (const auto& thread : std::as_const(m_eventData.threads)) {
-            VERIFY_OR_THROW(!thread.name.isEmpty());
-            VERIFY_OR_THROW(thread.pid != 0);
-            VERIFY_OR_THROW(thread.tid != 0);
-            VERIFY_OR_THROW(thread.time.isValid());
-            VERIFY_OR_THROW(thread.time.end > thread.time.start);
-            VERIFY_OR_THROW(thread.offCpuTime == 0 || thread.offCpuTime < thread.time.delta());
-        }
-        VERIFY_OR_THROW(!m_eventData.totalCosts.isEmpty());
+            QVERIFY(!thread.name.isEmpty());
+            QVERIFY(thread.pid != 0);
+            QVERIFY(thread.tid != 0);
+            QVERIFY(thread.time.isValid());
+            QVERIFY(thread.time.end > thread.time.start);
+            QVERIFY(thread.offCpuTime == 0 || thread.offCpuTime < thread.time.delta());
+        }
+        QVERIFY(!m_eventData.totalCosts.isEmpty());
         for (const auto& costs : std::as_const(m_eventData.totalCosts)) {
-            VERIFY_OR_THROW(!costs.label.isEmpty());
-            VERIFY_OR_THROW(costs.sampleCount > 0);
-            VERIFY_OR_THROW(costs.totalPeriod > 0);
+            QVERIFY(!costs.label.isEmpty());
+            QVERIFY(costs.sampleCount > 0);
+            QVERIFY(costs.totalPeriod > 0);
         }
     }
 };