Q{Elf,Mach}Parser: simplify the return codes

The multi-state return code was a legacy of how Arvid wrote the ELF
parser code back in the day, the fact that it scanned for two different
types of plugins in Qt 4 and that the metadata could exist in different
places.  None of that matters nowadays: who cares if the file is a
corrupt binary, not a valid binary, does not have the right
architecture, or has no suitable section? It's not a plugin, period.

The Qt 4 plugin mechanism was removed for Qt 5.0 in commit
7443895857fdaee132c8efc643e471f02b3d0fa4 ("Remove support for Qt 4 style
plugins").

Change-Id: I42eb903a916645db9900fffd16a442d800399b98
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

src/corelib/plugin/qelfparser_p.cpp

@@ -63,8 +63,8 @@ const char *QElfParser::parseSectionHeader(const char *data, ElfSectionHeader *s
     return data;
 }
 
-auto QElfParser::parse(const char *dataStart, ulong fdlen, const QString &library,
+QLibraryScanResult QElfParser::parse(const char *dataStart, ulong fdlen, const QString &library,
-                       QLibraryPrivate *lib, qsizetype *pos, qsizetype *sectionlen) -> ScanResult
+                                     QLibraryPrivate *lib)
 {
 #if defined(QELFPARSER_DEBUG)
     qDebug() << "QElfParser::parse " << library;
@@ -73,19 +73,19 @@ auto QElfParser::parse(const char *dataStart, ulong fdlen, const QString &librar
     if (fdlen < 64) {
         if (lib)
             lib->errorString = QLibrary::tr("'%1' is not an ELF object (%2)").arg(library, QLibrary::tr("file too small"));
-        return NotElf;
+        return {};
     }
     const char *data = dataStart;
     if (qstrncmp(data, "\177ELF", 4) != 0) {
         if (lib)
             lib->errorString = QLibrary::tr("'%1' is not an ELF object").arg(library);
-        return NotElf;
+        return {};
     }
     // 32 or 64 bit
     if (data[4] != 1 && data[4] != 2) {
         if (lib)
             lib->errorString = QLibrary::tr("'%1' is an invalid ELF object (%2)").arg(library, QLibrary::tr("odd cpu architecture"));
-        return Corrupt;
+        return {};
     }
 
     /*  If you remove this check, to read ELF objects of a different arch, please make sure you modify the typedefs
@@ -95,7 +95,7 @@ auto QElfParser::parse(const char *dataStart, ulong fdlen, const QString &librar
     if (data[4] != ExpectedClass) {
         if (lib)
             lib->errorString = QLibrary::tr("'%1' is an invalid ELF object (%2)").arg(library, QLibrary::tr("wrong cpu architecture"));
-        return Corrupt;
+        return {};
     }
 
     // endian
@@ -103,7 +103,7 @@ auto QElfParser::parse(const char *dataStart, ulong fdlen, const QString &librar
     if (data[5] != ExpectedEndianness) {
         if (lib)
             lib->errorString = QLibrary::tr("'%1' is an invalid ELF object (%2)").arg(library, QLibrary::tr("odd endianness"));
-        return Corrupt;
+        return {};
     }
 
     data += 16                  // e_ident
@@ -122,7 +122,7 @@ auto QElfParser::parse(const char *dataStart, ulong fdlen, const QString &librar
     if (e_shsize > fdlen) {
         if (lib)
             lib->errorString = QLibrary::tr("'%1' is an invalid ELF object (%2)").arg(library, QLibrary::tr("unexpected e_shsize"));
-        return Corrupt;
+        return {};
     }
 
     data += sizeof(qelfhalf_t)  // e_ehsize
@@ -134,7 +134,7 @@ auto QElfParser::parse(const char *dataStart, ulong fdlen, const QString &librar
     if (e_shentsize % 4) {
         if (lib)
             lib->errorString = QLibrary::tr("'%1' is an invalid ELF object (%2)").arg(library, QLibrary::tr("unexpected e_shentsize"));
-        return Corrupt;
+        return {};
     }
     data += sizeof(qelfhalf_t); // e_shentsize
     qelfhalf_t e_shnum     = qFromUnaligned<qelfhalf_t> (data);
@@ -149,7 +149,7 @@ auto QElfParser::parse(const char *dataStart, ulong fdlen, const QString &librar
                              nullptr, int(e_shnum)).arg(e_shentsize);
             lib->errorString = QLibrary::tr("'%1' is an invalid ELF object (%2)").arg(library, message);
         }
-        return Corrupt;
+        return {};
     }
 
 #if defined(QELFPARSER_DEBUG)
@@ -164,7 +164,7 @@ auto QElfParser::parse(const char *dataStart, ulong fdlen, const QString &librar
             lib->errorString = QLibrary::tr("'%1' is an invalid ELF object (%2)")
                                .arg(library, QLibrary::tr("shstrtab section header seems to be at %1")
                                              .arg(QString::number(soff, 16)));
-        return Corrupt;
+        return {};
     }
 
     parseSectionHeader(dataStart + soff, &strtab);
@@ -175,7 +175,7 @@ auto QElfParser::parse(const char *dataStart, ulong fdlen, const QString &librar
             lib->errorString = QLibrary::tr("'%1' is an invalid ELF object (%2)")
                                .arg(library, QLibrary::tr("string table seems to be at %1")
                                              .arg(QString::number(strtab.offset, 16)));
-        return Corrupt;
+        return {};
     }
 
 #if defined(QELFPARSER_DEBUG)
@@ -197,7 +197,7 @@ auto QElfParser::parse(const char *dataStart, ulong fdlen, const QString &librar
                 lib->errorString = QLibrary::tr("'%1' is an invalid ELF object (%2)")
                     .arg(library, QLibrary::tr("section name %1 of %2 behind end of file")
                                   .arg(i).arg(e_shnum));
-            return Corrupt;
+            return {};
         }
 
 #if defined(QELFPARSER_DEBUG)
@@ -210,7 +210,7 @@ auto QElfParser::parse(const char *dataStart, ulong fdlen, const QString &librar
                     if (lib)
                         lib->errorString = QLibrary::tr("'%1' is an invalid ELF object (%2)")
                             .arg(library, QLibrary::tr("empty .rodata. not a library."));
-                    return Corrupt;
+                    return {};
                 }
 #if defined(QELFPARSER_DEBUG)
                 qDebug()<<"section is not program data. skipped.";
@@ -223,15 +223,13 @@ auto QElfParser::parse(const char *dataStart, ulong fdlen, const QString &librar
                 if (lib)
                     lib->errorString = QLibrary::tr("'%1' is an invalid ELF object (%2)")
                         .arg(library, QLibrary::tr("missing section data. This is not a library."));
-                return Corrupt;
+                return {};
             }
-            *pos = sh.offset;
+            return { qsizetype(sh.offset), qsizetype(sh.size) };
-            *sectionlen = sh.size;
-            return QtMetaDataSection;
         }
         s += e_shentsize;
     }
-    return NoQtSection;
+    return {};
 }
 
 QT_END_NAMESPACE

src/corelib/plugin/qelfparser_p.h

@@ -52,7 +52,7 @@
 //
 
 #include <qendian.h>
-#include <private/qglobal_p.h>
+#include "qlibrary_p.h"
 
 QT_REQUIRE_CONFIG(library);
 
@@ -71,7 +71,6 @@ typedef quintptr qelfaddr_t;
 class QElfParser
 {
 public:
-    enum ScanResult { QtMetaDataSection, NoQtSection, NotElf, Corrupt };
     enum { ElfLittleEndian = 0, ElfBigEndian = 1 };
 
     struct ElfSectionHeader
@@ -85,7 +84,7 @@ public:
     qelfoff_t m_stringTableFileOffset;
 
     const char *parseSectionHeader(const char* s, ElfSectionHeader *sh);
-    ScanResult parse(const char *m_s, ulong fdlen, const QString &library, QLibraryPrivate *lib, qsizetype *pos, qsizetype *sectionlen);
+    QLibraryScanResult parse(const char *m_s, ulong fdlen, const QString &library, QLibraryPrivate *lib);
 };
 
 QT_END_NAMESPACE

src/corelib/plugin/qlibrary.cpp

@@ -254,8 +254,10 @@ static bool findPatternUnloaded(const QString &library, QLibraryPrivate *lib)
     constexpr qint64 MaxMemoryMapSize =
             Q_INT64_C(1) << (sizeof(qsizetype) > 4 ? 40 : 29);
 
-    qsizetype fdlen = qMin(file.size(), MaxMemoryMapSize);
+    QLibraryScanResult r;
-    const char *filedata = reinterpret_cast<char *>(file.map(0, fdlen));
+    r.pos = 0;
+    r.length = qMin(file.size(), MaxMemoryMapSize);
+    const char *filedata = reinterpret_cast<char *>(file.map(0, r.length));
 
 #ifdef Q_OS_UNIX
     if (filedata == nullptr) {
@@ -274,65 +276,49 @@ static bool findPatternUnloaded(const QString &library, QLibraryPrivate *lib)
         // the side of doing a regular read into memory (up to 64 MB).
         data = file.read(64 * 1024 * 1024);
         filedata = data.constData();
-        fdlen = data.size();
+        r.length = data.size();
     }
 #endif
 
     /*
-       ELF and Mach-O binaries with GCC have .qplugin sections.
+       ELF and Mach-O binaries with GCC have .qtmetadata sections. Find them.
     */
     bool hasMetaData = false;
-    qsizetype pos = 0;
     char pattern[] = "qTMETADATA ";
     pattern[0] = 'Q'; // Ensure the pattern "QTMETADATA" is not found in this library should QPluginLoader ever encounter it.
     const ulong plen = ulong(qstrlen(pattern));
 #if defined (Q_OF_ELF)
-    QElfParser::ScanResult r = QElfParser().parse(filedata, fdlen, library, lib, &pos, &fdlen);
+    r = QElfParser().parse(filedata, r.length, library, lib);
-    if (r == QElfParser::Corrupt || r == QElfParser::NotElf) {
+    if (r.length == 0) {
-            if (lib && qt_debug_component()) {
+        if (lib && qt_debug_component())
-                qWarning("QElfParser: %ls", qUtf16Printable(lib->errorString));
+            qWarning("QElfParser: %ls", qUtf16Printable(lib->errorString));
-            }
+        return false;
-            return false;
-    } else if (r == QElfParser::QtMetaDataSection) {
-        qsizetype rel = qt_find_pattern(filedata + pos, fdlen, pattern, plen);
-        if (rel < 0)
-            pos = -1;
-        else
-            pos += rel;
-        hasMetaData = true;
     }
 #elif defined(Q_OF_MACH_O)
     {
         QString errorString;
-        int r = QMachOParser::parse(filedata, fdlen, library, &errorString, &pos, &fdlen);
+        r = QMachOParser::parse(filedata, r.length, library, &errorString);
-        if (r == QMachOParser::NotSuitable) {
+        if (r.length == 0) {
             if (qt_debug_component())
                 qWarning("QMachOParser: %ls", qUtf16Printable(errorString));
             if (lib)
                 lib->errorString = errorString;
             return false;
         }
-        // even if the metadata section was not found, the Mach-O parser will
+    }
-        // at least return the boundaries of the right architecture
+#endif // defined(Q_OF_ELF) && defined(Q_CC_GNU)
-        qsizetype rel = qt_find_pattern(filedata + pos, fdlen, pattern, plen);
+    if (qsizetype rel = qt_find_pattern(filedata + r.pos, r.length, pattern, plen);
-        if (rel < 0)
+            rel >= 0) {
-            pos = -1;
+        r.pos += rel;
-        else
-            pos += rel;
         hasMetaData = true;
     }
-#else
-    pos = qt_find_pattern(filedata, fdlen, pattern, plen);
-    if (pos > 0)
-        hasMetaData = true;
-#endif // defined(Q_OF_ELF) && defined(Q_CC_GNU)
 
     bool ret = false;
 
-    if (pos >= 0 && hasMetaData) {
+    if (r.pos >= 0 && hasMetaData) {
-        const char *data = filedata + pos;
+        const char *data = filedata + r.pos;
         QString errMsg;
-        QJsonDocument doc = qJsonFromRawLibraryMetaData(data, fdlen, &errMsg);
+        QJsonDocument doc = qJsonFromRawLibraryMetaData(data, r.length, &errMsg);
         if (doc.isNull()) {
             qWarning("Found invalid metadata in lib %ls: %ls",
                      qUtf16Printable(library), qUtf16Printable(errMsg));

src/corelib/plugin/qlibrary_p.h

@@ -68,6 +68,12 @@ QT_BEGIN_NAMESPACE
 
 bool qt_debug_component();
 
+struct QLibraryScanResult
+{
+    qsizetype pos;
+    qsizetype length;
+};
+
 class QLibraryStore;
 class QLibraryPrivate
 {

src/corelib/plugin/qmachparser.cpp

@@ -42,7 +42,6 @@
 #if defined(Q_OF_MACH_O)
 
 #include <qendian.h>
-#include "qlibrary_p.h"
 
 #include <mach-o/loader.h>
 #include <mach-o/fat.h>
@@ -81,15 +80,16 @@ typedef section my_section;
 static const uint32_t my_magic = MH_MAGIC;
 #endif
 
-static int ns(const QString &reason, const QString &library, QString *errorString)
+Q_DECL_COLD_FUNCTION
+static QLibraryScanResult ns(const QString &reason, const QString &library, QString *errorString)
 {
     if (errorString)
         *errorString = QLibrary::tr("'%1' is not a valid Mach-O binary (%2)")
                 .arg(library, reason.isEmpty() ? QLibrary::tr("file is corrupt") : reason);
-    return QMachOParser::NotSuitable;
+    return {};
 }
 
-int QMachOParser::parse(const char *m_s, ulong fdlen, const QString &library, QString *errorString, qsizetype *pos, qsizetype *sectionlen)
+QLibraryScanResult  QMachOParser::parse(const char *m_s, ulong fdlen, const QString &library, QString *errorString)
 {
     // The minimum size of a Mach-O binary we're interested in.
     // It must have a full Mach header, at least one segment and at least one
@@ -146,9 +146,7 @@ int QMachOParser::parse(const char *m_s, ulong fdlen, const QString &library, QS
                       library, errorString);
     }
 
-    // from this point on, fdlen is specific to this architecture
     // from this point on, everything is in host byte order
-    *pos = reinterpret_cast<const char *>(header) - m_s;
 
     // (re-)check the CPU type
     // ### should we check the CPU subtype? Maybe on ARM?
@@ -197,9 +195,8 @@ int QMachOParser::parse(const char *m_s, ulong fdlen, const QString &library, QS
                         || Q_UNLIKELY(fdlen < sect[j].offset + sect[j].size))
                     return ns(QString(), library, errorString);
 
-                *pos += sect[j].offset;
+                qsizetype pos = reinterpret_cast<const char *>(header) - m_s + sect[j].offset;
-                *sectionlen = sect[j].size;
+                return { pos, qsizetype(sect[j].size) };
-                return QtMetaDataSection;
             }
         }
 
@@ -207,11 +204,10 @@ int QMachOParser::parse(const char *m_s, ulong fdlen, const QString &library, QS
         seg = reinterpret_cast<const my_segment_command *>(reinterpret_cast<const char *>(seg) + seg->cmdsize);
     }
 
-//    // No Qt section was found, but at least we know that where the proper architecture's boundaries are
+    // No .qtmetadata section was found
-//    return NoQtSection;
     if (errorString)
         *errorString = QLibrary::tr("'%1' is not a Qt plugin").arg(library);
-    return NotSuitable;
+    return {};
 }
 
 QT_END_NAMESPACE

src/corelib/plugin/qmachparser_p.h

@@ -51,8 +51,7 @@
 // We mean it.
 //
 
-#include <qendian.h>
+#include "qlibrary_p.h"
-#include <private/qglobal_p.h>
 
 QT_REQUIRE_CONFIG(library);
 
@@ -66,8 +65,8 @@ class QLibraryPrivate;
 class Q_AUTOTEST_EXPORT QMachOParser
 {
 public:
-    enum { QtMetaDataSection, NoQtSection, NotSuitable };
+    static QLibraryScanResult parse(const char *m_s, ulong fdlen, const QString &library,
-    static int parse(const char *m_s, ulong fdlen, const QString &library, QString *errorString, qsizetype *pos, qsizetype *sectionlen);
+                                    QString *errorString);
 };
 
 QT_END_NAMESPACE

tests/auto/corelib/plugin/qpluginloader/tst_qpluginloader.cpp

@@ -334,36 +334,36 @@ void tst_QPluginLoader::loadCorruptElf()
 void tst_QPluginLoader::loadMachO_data()
 {
 #if defined(QT_BUILD_INTERNAL) && defined(Q_OF_MACH_O)
-    QTest::addColumn<int>("parseResult");
+    QTest::addColumn<bool>("success");
 
-    QTest::newRow("/dev/null") << int(QMachOParser::NotSuitable);
+    QTest::newRow("/dev/null") << false;
-    QTest::newRow("elftest/debugobj.so") << int(QMachOParser::NotSuitable);
+    QTest::newRow("elftest/debugobj.so") << false;
-    QTest::newRow("tst_qpluginloader.cpp") << int(QMachOParser::NotSuitable);
+    QTest::newRow("tst_qpluginloader.cpp") << false;
-    QTest::newRow("tst_qpluginloader") << int(QMachOParser::NotSuitable);
+    QTest::newRow("tst_qpluginloader") << false;
 
 #  ifdef Q_PROCESSOR_X86_64
-    QTest::newRow("machtest/good.x86_64.dylib") << int(QMachOParser::QtMetaDataSection);
+    QTest::newRow("machtest/good.x86_64.dylib") << true;
-    QTest::newRow("machtest/good.i386.dylib") << int(QMachOParser::NotSuitable);
+    QTest::newRow("machtest/good.i386.dylib") << false;
-    QTest::newRow("machtest/good.fat.no-x86_64.dylib") << int(QMachOParser::NotSuitable);
+    QTest::newRow("machtest/good.fat.no-x86_64.dylib") << false;
-    QTest::newRow("machtest/good.fat.no-i386.dylib") << int(QMachOParser::QtMetaDataSection);
+    QTest::newRow("machtest/good.fat.no-i386.dylib") << true;
 #  elif defined(Q_PROCESSOR_X86_32)
-    QTest::newRow("machtest/good.i386.dylib") << int(QMachOParser::QtMetaDataSection);
+    QTest::newRow("machtest/good.i386.dylib") << true;
-    QTest::newRow("machtest/good.x86_64.dylib") << int(QMachOParser::NotSuitable);
+    QTest::newRow("machtest/good.x86_64.dylib") << false;
-    QTest::newRow("machtest/good.fat.no-i386.dylib") << int(QMachOParser::NotSuitable);
+    QTest::newRow("machtest/good.fat.no-i386.dylib") << false;
-    QTest::newRow("machtest/good.fat.no-x86_64.dylib") << int(QMachOParser::QtMetaDataSection);
+    QTest::newRow("machtest/good.fat.no-x86_64.dylib") << true;
 #  endif
 #  ifndef Q_PROCESSOR_POWER_64
-    QTest::newRow("machtest/good.ppc64.dylib") << int(QMachOParser::NotSuitable);
+    QTest::newRow("machtest/good.ppc64.dylib") << false;
 #  endif
 
-    QTest::newRow("machtest/good.fat.all.dylib") << int(QMachOParser::QtMetaDataSection);
+    QTest::newRow("machtest/good.fat.all.dylib") << true;
-    QTest::newRow("machtest/good.fat.stub-x86_64.dylib") << int(QMachOParser::NotSuitable);
+    QTest::newRow("machtest/good.fat.stub-x86_64.dylib") << false;
-    QTest::newRow("machtest/good.fat.stub-i386.dylib") << int(QMachOParser::NotSuitable);
+    QTest::newRow("machtest/good.fat.stub-i386.dylib") << false;
 
     QDir d(QFINDTESTDATA("machtest"));
     QStringList badlist = d.entryList(QStringList() << "bad*.dylib");
     foreach (const QString &bad, badlist)
-        QTest::newRow(qPrintable("machtest/" + bad)) << int(QMachOParser::NotSuitable);
+        QTest::newRow(qPrintable("machtest/" + bad)) << false;
 #endif
 }
 
@@ -374,31 +374,31 @@ void tst_QPluginLoader::loadMachO()
     QVERIFY(f.open(QIODevice::ReadOnly));
     QByteArray data = f.readAll();
 
-    qsizetype pos;
-    qsizetype len;
     QString errorString;
-    int r = QMachOParser::parse(data.constData(), data.size(), f.fileName(), &errorString, &pos, &len);
+    QLibraryScanResult r = QMachOParser::parse(data.constData(), data.size(), f.fileName(), &errorString);
 
-    QFETCH(int, parseResult);
+    QFETCH(bool, success);
-    QCOMPARE(r, parseResult);
+    if (success) {
-
+        QVERIFY(r.length != 0);
-    if (r == QMachOParser::NotSuitable)
+    } else {
+        QCOMPARE(r.length, 0);
         return;
+    }
 
-    QVERIFY(pos > 0);
+    QVERIFY(r.pos > 0);
-    QVERIFY(size_t(len) >= sizeof(void*));
+    QVERIFY(size_t(r.length) >= sizeof(void*));
-    QVERIFY(pos + long(len) < data.size());
+    QVERIFY(r.pos + r.length < data.size());
-    QCOMPARE(pos & (sizeof(void*) - 1), 0UL);
+    QCOMPARE(r.pos & (sizeof(void*) - 1), 0UL);
 
-    void *value = *(void**)(data.constData() + pos);
+    void *value = *(void**)(data.constData() + r.pos);
     QCOMPARE(value, sizeof(void*) > 4 ? (void*)(0xc0ffeec0ffeeL) : (void*)0xc0ffee);
 
     // now that we know it's valid, let's try to make it invalid
-    ulong offeredlen = pos;
+    ulong offeredlen = r.pos;
     do {
         --offeredlen;
-        r = QMachOParser::parse(data.constData(), offeredlen, f.fileName(), &errorString, &pos, &len);
+        r = QMachOParser::parse(data.constData(), offeredlen, f.fileName(), &errorString);
-        QVERIFY2(r == QMachOParser::NotSuitable, qPrintable(QString("Failed at size 0x%1").arg(offeredlen, 0, 16)));
+        QVERIFY2(r.length == 0, qPrintable(QString("Failed at size 0x%1").arg(offeredlen, 0, 16)));
     } while (offeredlen);
 #endif
 }