Skip to content

Commit

Permalink
peek into file before open
Browse files Browse the repository at this point in the history 
* check file open, providing nice and early error
* explicit check for perfparser (magic number QPERFSTREAM)
  opening directly without the need to fall back to file name
* explicit check for V1 perf data which isn't supported by perfparser
* explicit check for unknown file type

fixes KDAB#477
  • Loading branch information
@GitMensch
GitMensch committed Nov 5, 2023
1 parent 69ad59b commit c420b6b
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 17 deletions.
38 changes: 28 additions & 10 deletions src/parsers/perf/perfparser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1475,6 +1475,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));
Expand All @@ -1489,6 +1490,22 @@ 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."));
Expand All @@ -1497,8 +1514,8 @@ bool PerfParser::initParserArgs(const QString& path)

auto parserArgs = [this](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};
Expand Down Expand Up @@ -1530,7 +1547,7 @@ bool PerfParser::initParserArgs(const QString& path)
return parserArgs;
};

m_parserArgs = parserArgs(path);
m_parserArgs = parserArgs(filename);
m_parserBinary = parserBinary;
return true;
}
Expand Down Expand Up @@ -1583,22 +1600,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));
Expand Down
27 changes: 20 additions & 7 deletions tests/integrationtests/tst_perfparser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,23 +227,36 @@ private slots:
QCOMPARE(parsingFailedSpy.count(), 0);
QCOMPARE(parsingFinishedSpy.count(), 1);

// check for invalid data -> expected error
// check pre-exported perfparser with "bad extension"
const auto shell = QStandardPaths::findExecutable(QStringLiteral("bash"));
if (!shell.isEmpty()) {
const auto perfDataSomeName = QStringLiteral("fruitper");
QProcess::execute(shell,
{QStringLiteral("-c"), QLatin1String("cp -f %1 %2").arg(perfData, perfDataSomeName)});

parser.startParseFile(perfDataSomeName);
QVERIFY(parsingFinishedSpy.wait(2000));
QCOMPARE(parsingFailedSpy.count(), 0);
QCOMPARE(parsingFinishedSpy.count(), 2);
}

// check for unexpected (here: no) file magic -> expected error
const auto noPerf = QFINDTESTDATA("tst_perfparser.cpp");
QVERIFY(!noPerf.isEmpty() && QFile::exists(noPerf));
parser.startParseFile(noPerf);
QVERIFY(parsingFailedSpy.wait(1000));
QCOMPARE(parsingFailedSpy.count(), 1);
message = qvariant_cast<QString>(parsingFailedSpy.takeFirst().at(0));
QVERIFY(message.contains(QLatin1String("invalid perf data file")));
QVERIFY(message.contains(noPerf));
QVERIFY(message.contains(QLatin1String("File format unknown")));

// check for PERFv1 -> expected error
const auto perf1 = QFINDTESTDATA("file_content/perf.data.true.v1");
QVERIFY(!perf1.isEmpty() && QFile::exists(perf1));
parser.startParseFile(perf1);
QVERIFY(parsingFailedSpy.wait(1000));
QCOMPARE(parsingFailedSpy.count(), 1);
message = qvariant_cast<QString>(parsingFailedSpy.takeFirst().at(0));
QVERIFY(message.contains(QLatin1String("invalid perf data file")));
QVERIFY(message.contains(perf1));
QVERIFY(message.contains(QLatin1String("V1 perf data")));

// check for PERFv2
const auto perf2 = QFINDTESTDATA("file_content/perf.data.true.v2");
Expand All @@ -253,7 +266,7 @@ private slots:
8000)); // FIXME: this waittime and the one below must be so high as unavailable elf files are checked
// again and again via PerfSymbolTable::registerElf, as seen in the output
QCOMPARE(parsingFailedSpy.count(), 0);
QCOMPARE(parsingFinishedSpy.count(), 2);
QCOMPARE(parsingFinishedSpy.count(), 3);

#if KFArchive_FOUND
// check for PERFv2, gzipped
Expand All @@ -262,7 +275,7 @@ private slots:
parser.startParseFile(perf2gz);
QVERIFY(parsingFinishedSpy.wait(9000));
QCOMPARE(parsingFailedSpy.count(), 0);
QCOMPARE(parsingFinishedSpy.count(), 3);
QCOMPARE(parsingFinishedSpy.count(), 4);
#endif
}

Expand Down

0 comments on commit c420b6b

Please sign in to comment.