Report an error when the kallsyms file could not be parsed

When a wrong kallsym path is given, or the file could not be
parsed correctly, e.g. when it was empty, then we want to
notify the user about this. Otherwise, it may not be clear why
symbol resolution for kernel addresses is broken.

Change-Id: Icf51fa3038810e69a91d332a33495e7678b3977a
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

5 files changed, 68 insertions, 10 deletions

diff --git a/app/perfkallsyms.cpp b/app/perfkallsyms.cpp
index d477cb4..84b8847 100644
--- a/app/perfkallsyms.cpp
+++ b/app/perfkallsyms.cpp
@@ -24,12 +24,15 @@
 
 #include <algorithm>
 
-PerfKallsyms::PerfKallsyms(const QString &path)
+bool PerfKallsyms::parseMapping(const QString &path)
 {
+    m_entries.clear();
+    m_errorString.clear();
+
     QFile file(path);
     if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
-        qWarning("Failed to open kallsyms file \"%s\".", qPrintable(path));
-        return;
+        m_errorString = file.errorString();
+        return false;
     }
 
     QTextStream stream(&file);
@@ -38,6 +41,7 @@ PerfKallsyms::PerfKallsyms(const QString &path)
     QByteArray address;
 
     // NOTE: don't use atEnd here, as /proc/kallsyms is has size of 0
+    bool valid = true;
     while (true) {
         PerfKallsymEntry entry;
         char type = 0;
@@ -50,7 +54,8 @@ PerfKallsyms::PerfKallsyms(const QString &path)
         bool ok = false;
         entry.address = address.toULongLong(&ok, 16);
         if (!ok) {
-            qWarning("Failed to parse kallsyms file, invalid address: %s.", address.constData());
+            m_errorString = tr("Invalid address: %1").arg(QString::fromUtf8(address));
+            valid = false;
             break;
         }
 
@@ -60,10 +65,17 @@ PerfKallsyms::PerfKallsyms(const QString &path)
         m_entries.push_back(entry);
     }
 
+    if (valid && m_entries.isEmpty()) {
+        m_errorString = tr("Mapping is empty.");
+        return false;
+    }
+
     std::sort(m_entries.begin(), m_entries.end(),
         [](const PerfKallsymEntry& lhs, const PerfKallsymEntry& rhs) -> bool {
             return lhs.address < rhs.address;
         });
+
+    return valid;
 }
 
 PerfKallsymEntry PerfKallsyms::findEntry(quint64 address) const
diff --git a/app/perfkallsyms.h b/app/perfkallsyms.h
index 8c1c89b..3fd16bc 100644
--- a/app/perfkallsyms.h
+++ b/app/perfkallsyms.h
@@ -20,6 +20,7 @@
 #ifndef PERFKALLSYMS_H
 #define PERFKALLSYMS_H
 
+#include <QCoreApplication>
 #include <QByteArray>
 #include <QVector>
 
@@ -36,13 +37,16 @@ QT_END_NAMESPACE
 
 class PerfKallsyms
 {
+    Q_DECLARE_TR_FUNCTIONS(PerfKallsyms)
 public:
-    PerfKallsyms(const QString &path);
+    bool parseMapping(const QString &path);
+    QString errorString() const { return m_errorString; }
 
     PerfKallsymEntry findEntry(quint64 address) const;
 
 private:
     QVector<PerfKallsymEntry> m_entries;
+    QString m_errorString;
 };
 
 #endif // PERFKALLSYMS_H
diff --git a/app/perfunwind.cpp b/app/perfunwind.cpp
index 7b94ab3..00ceb2b 100644
--- a/app/perfunwind.cpp
+++ b/app/perfunwind.cpp
@@ -79,7 +79,7 @@ PerfUnwind::PerfUnwind(QIODevice *output, const QString &systemRoot, const QStri
                        const QString &kallsymsPath, bool printStats, uint maxEventBufferSize,
                        int maxFrames) :
     m_output(output), m_architecture(PerfRegisterInfo::ARCH_INVALID), m_systemRoot(systemRoot),
-    m_extraLibsPath(extraLibsPath), m_appPath(appPath), m_debugPath(debugPath), m_kallsyms(kallsymsPath),
+    m_extraLibsPath(extraLibsPath), m_appPath(appPath), m_debugPath(debugPath),
     m_maxEventBufferSize(maxEventBufferSize), m_eventBufferSize(0), m_lastFlushMaxTime(0)
 {
     m_stats.enabled = printStats;
@@ -103,6 +103,11 @@ PerfUnwind::PerfUnwind(QIODevice *output, const QString &systemRoot, const QStri
         qint32 dataStreamVersion = qToLittleEndian(QDataStream::Qt_DefaultCompiledVersion);
         output->write(reinterpret_cast<const char *>(&dataStreamVersion), sizeof(qint32));
     }
+
+    if (!m_kallsyms.parseMapping(kallsymsPath))
+        sendError(InvalidKallsyms,
+                  tr("Failed to parse kernel symbol mapping file \"%1\": %2")
+                    .arg(kallsymsPath, m_kallsyms.errorString()));
 }
 
 PerfUnwind::~PerfUnwind()
diff --git a/app/perfunwind.h b/app/perfunwind.h
index 1894161..bed8196 100644
--- a/app/perfunwind.h
+++ b/app/perfunwind.h
@@ -150,6 +150,7 @@ public:
     enum ErrorCode {
         TimeOrderViolation = 1,
         MissingElfFile = 2,
+        InvalidKallsyms = 3,
     };
     void sendError(ErrorCode error, const QString &message);
 
diff --git a/tests/auto/kallsyms/tst_kallsyms.cpp b/tests/auto/kallsyms/tst_kallsyms.cpp
index 4d8d756..d9f7c31 100644
--- a/tests/auto/kallsyms/tst_kallsyms.cpp
+++ b/tests/auto/kallsyms/tst_kallsyms.cpp
@@ -86,7 +86,9 @@ private slots:
         file.write(kallsymsContents);
         file.flush();
 
-        PerfKallsyms kallsyms(file.fileName());
+        PerfKallsyms kallsyms;
+        QVERIFY(kallsyms.parseMapping(file.fileName()));
+        QVERIFY(kallsyms.errorString().isEmpty());
 
         const auto entry = kallsyms.findEntry(address);
         QCOMPARE(entry.address, expectedAddress);
@@ -100,7 +102,9 @@ private slots:
         if (!QFile::exists(path))
             QSKIP("/proc/kallsysms not available");
 
-        PerfKallsyms kallsyms(path);
+        PerfKallsyms kallsyms;
+        QVERIFY(kallsyms.parseMapping(path));
+        QVERIFY(kallsyms.errorString().isEmpty());
 
         // just check that we find any entry
         const auto addr = std::numeric_limits<quint64>::max();
@@ -108,6 +112,37 @@ private slots:
         QVERIFY(!entry.symbol.isEmpty());
     }
 
+    void testParseErrors()
+    {
+        QTemporaryFile file;
+        QVERIFY(file.open());
+        const auto fileName = file.fileName();
+
+        {
+            PerfKallsyms kallsyms;
+            QVERIFY(!kallsyms.parseMapping(fileName));
+            qDebug() << kallsyms.errorString(); // file is empty
+            QVERIFY(!kallsyms.errorString().isEmpty());
+        }
+
+        file.write("this is garbage and not a valid mapping\n");
+        file.flush();
+        {
+            PerfKallsyms kallsyms;
+            QVERIFY(!kallsyms.parseMapping(fileName));
+            qDebug() << kallsyms.errorString(); // invalid address
+            QVERIFY(!kallsyms.errorString().isEmpty());
+        }
+
+        QVERIFY(file.remove());
+        {
+            PerfKallsyms kallsyms;
+            QVERIFY(!kallsyms.parseMapping(fileName));
+            qDebug() << kallsyms.errorString(); // file not found
+            QVERIFY(!kallsyms.errorString().isEmpty());
+        }
+    }
+
     void benchmarkProc()
     {
         const auto path = QStringLiteral("/proc/kallsyms");
@@ -115,8 +150,9 @@ private slots:
             QSKIP("/proc/kallsysms not available");
 
         QBENCHMARK {
-            PerfKallsyms kallsyms(path);
-            Q_UNUSED(kallsyms);
+            PerfKallsyms kallsyms;
+            bool parsed = kallsyms.parseMapping(path);
+            Q_UNUSED(parsed);
         }
     }
 };