From f249e5c91c90a82115562bb6e8a6422a482e823d Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sat, 18 Jun 2022 12:09:52 -0700 Subject: [PATCH] moc: fix use of escape sequence of more than one character We had the code to calculate the length, but were improperly using it only for the offset, not the length of the string or its containing array. That resulted in the generated moc output containing: QT_MOC_LITERAL(111, 5), // "\xffz" QT_MOC_LITERAL(114, 5), // "\0012" QT_MOC_LITERAL(117, 23), // "slotWithAReallyLongName" The two strings are described as occupying 5 bytes (length 4 + null terminator), which is incorrect. The offset was correct: 114 - 111 = 3 and 117 - 114 = 3. The new output is: QT_MOC_LITERAL(111, 2), // "\xffz" QT_MOC_LITERAL(114, 2), // "\0012" QT_MOC_LITERAL(117, 23), // "slotWithAReallyLongName" The effect of the array size calculation would only be felt if moc decided it needed a second string array (for strings over 65535 bytes), which would cause the offsets in the second array to be all wrong. There was no such test until now. Drive-by fixing of the newline, indentation, and the stale comment referring to QByteArrayData (Qt 5). Change-Id: Id0fb9ab0089845ee8843fffd16f9cd01b3e0709a Reviewed-by: Fabian Kosmale (cherry picked from commit dda9c9e2bc4fd2efe9e3fb0e451a8c3512f9a4d2) Reviewed-by: Qt Cherry-pick Bot --- src/tools/moc/generator.cpp | 42 +++++++++++++++++--------------- tests/auto/tools/moc/tst_moc.cpp | 41 +++++++++++++++++-------------- 2 files changed, 46 insertions(+), 37 deletions(-) diff --git a/src/tools/moc/generator.cpp b/src/tools/moc/generator.cpp index aab6b74e63f..d86c58c7bc6 100644 --- a/src/tools/moc/generator.cpp +++ b/src/tools/moc/generator.cpp @@ -93,6 +93,19 @@ static inline int lengthOfEscapeSequence(const QByteArray &s, int i) return i - startPos; } +static inline uint lengthOfEscapedString(const QByteArray &str) +{ + int extra = 0; + for (int j = 0; j < str.length(); ++j) { + if (str.at(j) == '\\') { + int cnt = lengthOfEscapeSequence(str, j) - 1; + extra += cnt; + j += cnt; + } + } + return str.length() - extra; +} + void Generator::strreg(const QByteArray &s) { if (!strings.contains(s)) @@ -225,7 +238,7 @@ void Generator::generateCode() int stringDataLength = 0; int stringDataCounter = 0; for (int i = 0; i < strings.size(); ++i) { - int thisLength = strings.at(i).length() + 1; + int thisLength = lengthOfEscapedString(strings.at(i)) + 1; stringDataLength += thisLength; if (stringDataLength / constCharArraySizeLimit) { // save previous stringdata and start computing the next one. @@ -238,35 +251,26 @@ void Generator::generateCode() } fprintf(out, "};\n"); - // Macro that expands into a QByteArrayData. The offset member is - // calculated from 1) the offset of the actual characters in the - // stringdata.stringdata member, and 2) the stringdata.data index of the - // QByteArrayData being defined. This calculation relies on the - // QByteArrayData::data() implementation returning simply "this + offset". + // Macro that simplifies the string data listing. The offset is calculated + // from the top of the stringdata object (i.e., past the uints). fprintf(out, "#define QT_MOC_LITERAL(ofs, len) \\\n" " uint(offsetof(qt_meta_stringdata_%s_t, stringdata0) + ofs), len \n", qualifiedClassNameIdentifier.constData()); fprintf(out, "static const qt_meta_stringdata_%s_t qt_meta_stringdata_%s = {\n", qualifiedClassNameIdentifier.constData(), qualifiedClassNameIdentifier.constData()); - fprintf(out, " {\n"); + fprintf(out, " {"); { int idx = 0; for (int i = 0; i < strings.size(); ++i) { const QByteArray &str = strings.at(i); - fprintf(out, "QT_MOC_LITERAL(%d, %d)", idx, int(str.length())); - if (i != strings.size() - 1) - fputc(',', out); const QByteArray comment = str.length() > 32 ? str.left(29) + "..." : str; - fprintf(out, " // \"%s\"\n", comment.size() ? comment.constData() : ""); - idx += str.length() + 1; - for (int j = 0; j < str.length(); ++j) { - if (str.at(j) == '\\') { - int cnt = lengthOfEscapeSequence(str, j) - 1; - idx -= cnt; - j += cnt; - } - } + const char *comma = (i != strings.size() - 1 ? "," : " "); + int len = lengthOfEscapedString(str); + fprintf(out, "\n QT_MOC_LITERAL(%d, %d)%s // \"%s\"", idx, len, comma, + comment.constData()); + + idx += len + 1; } fprintf(out, "\n },\n"); } diff --git a/tests/auto/tools/moc/tst_moc.cpp b/tests/auto/tools/moc/tst_moc.cpp index f56e2c30243..2c8e143c202 100644 --- a/tests/auto/tools/moc/tst_moc.cpp +++ b/tests/auto/tools/moc/tst_moc.cpp @@ -299,6 +299,7 @@ class TestClassinfoWithEscapes: public QObject Q_CLASSINFO("cpp c*/omment", "f/*oo") Q_CLASSINFO("endswith\\", "Or?\?/") Q_CLASSINFO("newline\n inside\n", "Or \r") + Q_CLASSINFO("\xffz", "\0012") public slots: void slotWithAReallyLongName(int) { } @@ -969,13 +970,15 @@ void tst_Moc::classinfoWithEscapes() const QMetaObject *mobj = &TestClassinfoWithEscapes::staticMetaObject; QCOMPARE(mobj->methodCount() - mobj->methodOffset(), 1); - QCOMPARE(mobj->classInfoCount(), 5); + QCOMPARE(mobj->classInfoCount(), 6); QCOMPARE(mobj->classInfo(2).name(), "cpp c*/omment"); QCOMPARE(mobj->classInfo(2).value(), "f/*oo"); QCOMPARE(mobj->classInfo(3).name(), "endswith\\"); QCOMPARE(mobj->classInfo(3).value(), "Or?\?/"); QCOMPARE(mobj->classInfo(4).name(), "newline\n inside\n"); QCOMPARE(mobj->classInfo(4).value(), "Or \r"); + QCOMPARE(mobj->classInfo(5).name(), "\xff" "z"); + QCOMPARE(mobj->classInfo(5).value(), "\001" "2"); QMetaMethod mm = mobj->method(mobj->methodOffset()); QCOMPARE(mm.methodSignature(), QByteArray("slotWithAReallyLongName(int)")); @@ -3820,6 +3823,7 @@ class VeryLongStringData : public QObject #define repeat32768(V) repeat16384(V) repeat16384(V) #define repeat65534(V) repeat32768(V) repeat16384(V) repeat8192(V) repeat4096(V) repeat2048(V) repeat1024(V) repeat512(V) repeat256(V) repeat128(V) repeat64(V) repeat32(V) repeat16(V) repeat8(V) repeat4(V) repeat2(V) + Q_CLASSINFO("\1" "23\xff", "String with CRLF.\r\n") Q_CLASSINFO(repeat65534("n"), repeat65534("i")) Q_CLASSINFO(repeat65534("e"), repeat65534("r")) Q_CLASSINFO(repeat32768("o"), repeat32768("b")) @@ -3868,25 +3872,26 @@ void tst_Moc::unnamedNamespaceObjectsAndGadgets() void tst_Moc::veryLongStringData() { const QMetaObject *mobj = &VeryLongStringData::staticMetaObject; - QCOMPARE(mobj->classInfoCount(), 4); + int startAt = 1; // some other classinfo added to the beginning + QCOMPARE(mobj->classInfoCount(), startAt + 4); - QCOMPARE(mobj->classInfo(0).name()[0], 'n'); - QCOMPARE(mobj->classInfo(0).value()[0], 'i'); - QCOMPARE(mobj->classInfo(1).name()[0], 'e'); - QCOMPARE(mobj->classInfo(1).value()[0], 'r'); - QCOMPARE(mobj->classInfo(2).name()[0], 'o'); - QCOMPARE(mobj->classInfo(2).value()[0], 'b'); - QCOMPARE(mobj->classInfo(3).name()[0], ':'); - QCOMPARE(mobj->classInfo(3).value()[0], ')'); + QCOMPARE(mobj->classInfo(startAt + 0).name()[0], 'n'); + QCOMPARE(mobj->classInfo(startAt + 0).value()[0], 'i'); + QCOMPARE(mobj->classInfo(startAt + 1).name()[0], 'e'); + QCOMPARE(mobj->classInfo(startAt + 1).value()[0], 'r'); + QCOMPARE(mobj->classInfo(startAt + 2).name()[0], 'o'); + QCOMPARE(mobj->classInfo(startAt + 2).value()[0], 'b'); + QCOMPARE(mobj->classInfo(startAt + 3).name()[0], ':'); + QCOMPARE(mobj->classInfo(startAt + 3).value()[0], ')'); - QCOMPARE(strlen(mobj->classInfo(0).name()), static_cast(65534)); - QCOMPARE(strlen(mobj->classInfo(0).value()), static_cast(65534)); - QCOMPARE(strlen(mobj->classInfo(1).name()), static_cast(65534)); - QCOMPARE(strlen(mobj->classInfo(1).value()), static_cast(65534)); - QCOMPARE(strlen(mobj->classInfo(2).name()), static_cast(32768)); - QCOMPARE(strlen(mobj->classInfo(2).value()), static_cast(32768)); - QCOMPARE(strlen(mobj->classInfo(3).name()), static_cast(1)); - QCOMPARE(strlen(mobj->classInfo(3).value()), static_cast(1)); + QCOMPARE(strlen(mobj->classInfo(startAt + 0).name()), static_cast(65534)); + QCOMPARE(strlen(mobj->classInfo(startAt + 0).value()), static_cast(65534)); + QCOMPARE(strlen(mobj->classInfo(startAt + 1).name()), static_cast(65534)); + QCOMPARE(strlen(mobj->classInfo(startAt + 1).value()), static_cast(65534)); + QCOMPARE(strlen(mobj->classInfo(startAt + 2).name()), static_cast(32768)); + QCOMPARE(strlen(mobj->classInfo(startAt + 2).value()), static_cast(32768)); + QCOMPARE(strlen(mobj->classInfo(startAt + 3).name()), static_cast(1)); + QCOMPARE(strlen(mobj->classInfo(startAt + 3).value()), static_cast(1)); } void tst_Moc::gadgetHierarchy()