From 042e713cb7800f97c10d2d75a717ceb3e41ab29a Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 29 Jan 2025 09:40:49 -0800 Subject: [PATCH] QTest: fix bug dereferencing nullptr in toString() Amends commit 0756cc1eae5fd8981983319fef1d084762a67b8d. The generic instantiation of this function had a std::nullptr_t * parameter, but callers had special code to pass a nullptr there because we never needed a value of a nullptr (it's always a null pointer). For example, in compare_ptr_helper(): auto lhsFormatter = Internal::pointerToString; auto rhsFormatter = Internal::genericToString; return compare_helper(t1 == nullptr, "Compared QObject pointers are not the same", const_cast(t1), nullptr, lhsFormatter, rhsFormatter, actual, expected, file, line); But in debug mode, some compilers did emit a load from this memory location, causing a crash. So we just specialize this function to avoid such. We had a test for this... except it was never reached because the earlier QCOMPARE() had already failed. For the test, this amends commit ae021882330abc5f6fbaadca290e6e5670c89028. Fixes: QTBUG-133330 Pick-to: 6.8 Change-Id: I2cd3bb475788431c6a0dfffd28e730e8b613e033 Reviewed-by: Christian Ehrlicher Reviewed-by: Ivan Solovev (cherry picked from commit e19b633c468123526660b40ae110f46090682c76) Reviewed-by: Qt Cherry-pick Bot --- src/testlib/qtestcase.h | 5 ++++ .../testlib/selftests/cmptest/tst_cmptest.cpp | 8 +++---- .../selftests/expected_cmptest.lightxml | 10 ++++++++ .../testlib/selftests/expected_cmptest.tap | 24 +++++++++++++++++++ .../selftests/expected_cmptest.teamcity | 4 ++++ .../testlib/selftests/expected_cmptest.txt | 8 +++++++ .../testlib/selftests/expected_cmptest.xml | 10 ++++++++ 7 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/testlib/qtestcase.h b/src/testlib/qtestcase.h index a4ff2a32b0e..7ae317be96a 100644 --- a/src/testlib/qtestcase.h +++ b/src/testlib/qtestcase.h @@ -384,6 +384,11 @@ namespace QTest return toString(static_cast(arg)); } + template <> inline const char *genericToString(const void *) + { + return QTest::toString(nullptr); + } + template const char *pointerToString(const void *arg) { using QTest::toString; diff --git a/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp b/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp index df8dc9f9c95..4d299c6f0f7 100644 --- a/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp +++ b/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp @@ -282,10 +282,10 @@ void tst_Cmptest::compareQObjects() object1.setObjectName(QStringLiteral("object1")); QObject object2; object2.setObjectName(QStringLiteral("object2")); - QCOMPARE(&object1, &object1); - QCOMPARE(&object1, &object2); - QCOMPARE(&object1, nullptr); - QCOMPARE(nullptr, &object2); + [&] { QCOMPARE(&object1, &object1); }(); + [&] { QCOMPARE(&object1, &object2); }(); + [&] { QCOMPARE(&object1, nullptr); }(); + [&] { QCOMPARE(nullptr, &object2); }(); } void tst_Cmptest::compare_stringLiterals() diff --git a/tests/auto/testlib/selftests/expected_cmptest.lightxml b/tests/auto/testlib/selftests/expected_cmptest.lightxml index 8d10234490c..3e5438e6cf3 100644 --- a/tests/auto/testlib/selftests/expected_cmptest.lightxml +++ b/tests/auto/testlib/selftests/expected_cmptest.lightxml @@ -172,6 +172,16 @@ + + + + + + diff --git a/tests/auto/testlib/selftests/expected_cmptest.tap b/tests/auto/testlib/selftests/expected_cmptest.tap index 2236bc54031..8fad22b99dd 100644 --- a/tests/auto/testlib/selftests/expected_cmptest.tap +++ b/tests/auto/testlib/selftests/expected_cmptest.tap @@ -268,6 +268,30 @@ not ok 22 - compareQObjects() file: qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp line: 0 ... +not ok 22 - compareQObjects() + --- + type: QCOMPARE + message: Compared QObject pointers are not the same + wanted: "nullptr" (nullptr) + found: QObject/"object1" (&object1) + expected: "nullptr" (nullptr) + actual: QObject/"object1" (&object1) + at: tst_Cmptest::compareQObjects() (qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp:0) + file: qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp + line: 0 + ... +not ok 22 - compareQObjects() + --- + type: QCOMPARE + message: Compared QObject pointers are not the same + wanted: QObject/"object2" (&object2) + found: "nullptr" (nullptr) + expected: QObject/"object2" (&object2) + actual: "nullptr" (nullptr) + at: tst_Cmptest::compareQObjects() (qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp:0) + file: qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp + line: 0 + ... ok 23 - compareQStringLists(empty lists) ok 24 - compareQStringLists(equal lists) not ok 25 - compareQStringLists(last item different) diff --git a/tests/auto/testlib/selftests/expected_cmptest.teamcity b/tests/auto/testlib/selftests/expected_cmptest.teamcity index c610eb0089a..38cb1b4ca39 100644 --- a/tests/auto/testlib/selftests/expected_cmptest.teamcity +++ b/tests/auto/testlib/selftests/expected_cmptest.teamcity @@ -72,6 +72,10 @@ ##teamcity[testStarted name='compareQObjects()' flowId='tst_Cmptest'] ##teamcity[testFailed name='compareQObjects()' message='Failure! |[Loc: qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp(0)|]' details='Compared QObject pointers are not the same|n Actual (&object1): QObject/"object1"|n Expected (&object2): QObject/"object2"' flowId='tst_Cmptest'] ##teamcity[testFinished name='compareQObjects()' flowId='tst_Cmptest'] +##teamcity[testFailed name='compareQObjects()' message='Failure! |[Loc: qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp(0)|]' details='Compared QObject pointers are not the same|n Actual (&object1): QObject/"object1"|n Expected (nullptr) : "nullptr"' flowId='tst_Cmptest'] +##teamcity[testFinished name='compareQObjects()' flowId='tst_Cmptest'] +##teamcity[testFailed name='compareQObjects()' message='Failure! |[Loc: qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp(0)|]' details='Compared QObject pointers are not the same|n Actual (nullptr) : "nullptr"|n Expected (&object2): QObject/"object2"' flowId='tst_Cmptest'] +##teamcity[testFinished name='compareQObjects()' flowId='tst_Cmptest'] ##teamcity[testStarted name='compareQStringLists(empty lists)' flowId='tst_Cmptest'] ##teamcity[testFinished name='compareQStringLists(empty lists)' flowId='tst_Cmptest'] ##teamcity[testStarted name='compareQStringLists(equal lists)' flowId='tst_Cmptest'] diff --git a/tests/auto/testlib/selftests/expected_cmptest.txt b/tests/auto/testlib/selftests/expected_cmptest.txt index 042625243c0..b3e28551213 100644 --- a/tests/auto/testlib/selftests/expected_cmptest.txt +++ b/tests/auto/testlib/selftests/expected_cmptest.txt @@ -95,6 +95,14 @@ FAIL! : tst_Cmptest::compareQObjects() Compared QObject pointers are not the sa Actual (&object1): QObject/"object1" Expected (&object2): QObject/"object2" Loc: [qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp(0)] +FAIL! : tst_Cmptest::compareQObjects() Compared QObject pointers are not the same + Actual (&object1): QObject/"object1" + Expected (nullptr) : "nullptr" + Loc: [qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp(0)] +FAIL! : tst_Cmptest::compareQObjects() Compared QObject pointers are not the same + Actual (nullptr) : "nullptr" + Expected (&object2): QObject/"object2" + Loc: [qtbase/tests/auto/testlib/selftests/cmptest/tst_cmptest.cpp(0)] PASS : tst_Cmptest::compareQStringLists(empty lists) PASS : tst_Cmptest::compareQStringLists(equal lists) FAIL! : tst_Cmptest::compareQStringLists(last item different) Compared lists differ at index 2. diff --git a/tests/auto/testlib/selftests/expected_cmptest.xml b/tests/auto/testlib/selftests/expected_cmptest.xml index e34c38e5bc9..272211370e5 100644 --- a/tests/auto/testlib/selftests/expected_cmptest.xml +++ b/tests/auto/testlib/selftests/expected_cmptest.xml @@ -174,6 +174,16 @@ + + + + + +