diff --git a/src/corelib/global/qtypeinfo.h b/src/corelib/global/qtypeinfo.h index 73bf6140c7a..4602257ff5f 100644 --- a/src/corelib/global/qtypeinfo.h +++ b/src/corelib/global/qtypeinfo.h @@ -22,6 +22,16 @@ class QDebug; template inline constexpr bool qIsRelocatable = std::is_trivially_copyable_v && std::is_trivially_destructible_v; +// Denotes types that are trivially default constructible, and for which +// value-initialization can be achieved by filling their storage with 0 bits. +// There is no type trait we can use for this, so we hardcode a list of +// possibilities that we know are OK on the architectures that we support. +// The most notable exception are pointers to data members, which for instance +// on the Itanium ABI are initialized to -1. +template +inline constexpr bool qIsValueInitializationBitwiseZero = + std::is_scalar_v && !std::is_member_object_pointer_v; + /* The catch-all template. */ @@ -35,6 +45,7 @@ public: isIntegral = std::is_integral_v, isComplex = !std::is_trivial_v, isRelocatable = qIsRelocatable, + isValueInitializationBitwiseZero = qIsValueInitializationBitwiseZero, }; }; @@ -47,6 +58,7 @@ public: isIntegral = false, isComplex = false, isRelocatable = false, + isValueInitializationBitwiseZero = false, }; }; @@ -79,6 +91,7 @@ public: static constexpr bool isRelocatable = ((QTypeInfo::isRelocatable) && ...); static constexpr bool isPointer = false; static constexpr bool isIntegral = false; + static constexpr bool isValueInitializationBitwiseZero = false; }; #define Q_DECLARE_MOVABLE_CONTAINER(CONTAINER) \ @@ -91,6 +104,7 @@ public: \ isIntegral = false, \ isComplex = true, \ isRelocatable = true, \ + isValueInitializationBitwiseZero = false, \ }; \ } @@ -131,6 +145,7 @@ public: \ isRelocatable = !isComplex || ((FLAGS) & Q_RELOCATABLE_TYPE) || qIsRelocatable, \ isPointer = false, \ isIntegral = std::is_integral< TYPE >::value, \ + isValueInitializationBitwiseZero = qIsValueInitializationBitwiseZero, \ }; \ } diff --git a/src/corelib/kernel/qmetatype.cpp b/src/corelib/kernel/qmetatype.cpp index 81647eb5dc5..3606b298dd1 100644 --- a/src/corelib/kernel/qmetatype.cpp +++ b/src/corelib/kernel/qmetatype.cpp @@ -433,7 +433,7 @@ const char *QtMetaTypePrivate::typedefNameForType(const QtPrivate::QMetaTypeInte The enum describes attributes of a type supported by QMetaType. - \value NeedsConstruction This type has a non-trivial default constructor. If the flag is not set, instances can be safely initialized with memset to 0. + \value NeedsConstruction This type has a default constructor. If the flag is not set, instances can be safely initialized with memset to 0. \value NeedsCopyConstruction (since 6.5) This type has a non-trivial copy construtcor. If the flag is not set, instances can be copied with memcpy. \value NeedsMoveConstruction (since 6.5) This type has a non-trivial move constructor. If the flag is not set, instances can be moved with memcpy. \value NeedsDestruction This type has a non-trivial destructor. If the flag is not set calls to the destructor are not necessary before discarding objects. diff --git a/src/corelib/kernel/qmetatype.h b/src/corelib/kernel/qmetatype.h index 4c92a52f9e7..97e0eae7f97 100644 --- a/src/corelib/kernel/qmetatype.h +++ b/src/corelib/kernel/qmetatype.h @@ -1251,7 +1251,7 @@ namespace QtPrivate { struct QMetaTypeTypeFlags { enum { Flags = (QTypeInfo::isRelocatable ? QMetaType::RelocatableType : 0) - | (!std::is_trivially_default_constructible_v ? QMetaType::NeedsConstruction : 0) + | ((!std::is_default_constructible_v || !QTypeInfo::isValueInitializationBitwiseZero) ? QMetaType::NeedsConstruction : 0) | (!std::is_trivially_destructible_v ? QMetaType::NeedsDestruction : 0) | (!std::is_trivially_copy_constructible_v ? QMetaType::NeedsCopyConstruction : 0) | (!std::is_trivially_move_constructible_v ? QMetaType::NeedsMoveConstruction : 0) @@ -2394,7 +2394,7 @@ public: static constexpr QMetaTypeInterface::DefaultCtrFn getDefaultCtr() { - if constexpr (std::is_default_constructible_v && !std::is_trivially_default_constructible_v) { + if constexpr (std::is_default_constructible_v && !QTypeInfo::isValueInitializationBitwiseZero) { return [](const QMetaTypeInterface *, void *addr) { new (addr) S(); }; } else { return nullptr; diff --git a/src/corelib/kernel/qvariant.cpp b/src/corelib/kernel/qvariant.cpp index 75bdef2383c..ddb0ccf75dd 100644 --- a/src/corelib/kernel/qvariant.cpp +++ b/src/corelib/kernel/qvariant.cpp @@ -266,7 +266,7 @@ static void customConstruct(const QtPrivate::QMetaTypeInterface *iface, QVariant if (QVariant::Private::canUseInternalSpace(iface)) { d->is_shared = false; if (!copy && !iface->defaultCtr) - return; // trivial default constructor, we've already memset + return; // default constructor and it's OK to build in 0-filled storage, which we've already done construct(iface, d->data.data, copy); } else { d->data.shared = customConstructShared(iface->size, iface->alignment, [=](void *where) { diff --git a/tests/auto/corelib/kernel/qmetatype/tst_qmetatype.cpp b/tests/auto/corelib/kernel/qmetatype/tst_qmetatype.cpp index b611fb146c0..f0efff34365 100644 --- a/tests/auto/corelib/kernel/qmetatype/tst_qmetatype.cpp +++ b/tests/auto/corelib/kernel/qmetatype/tst_qmetatype.cpp @@ -1025,7 +1025,7 @@ template void addFlagsRow(const char *name, int id = qMetaTypeId QTest::newRow(name) << id << bool(QTypeInfo::isRelocatable) - << bool(!std::is_trivially_default_constructible_v) + << bool(!std::is_default_constructible_v || !QTypeInfo::isValueInitializationBitwiseZero) << bool(!std::is_trivially_copy_constructible_v) << bool(!std::is_trivially_destructible_v) << bool(QtPrivate::IsPointerToTypeDerivedFromQObject::Value) @@ -1322,6 +1322,132 @@ FOR_EACH_CORE_METATYPE(RETURN_CONSTRUCT_FUNCTION) TypeTestFunctionGetter::get(type)(); } + +namespace TriviallyConstructibleTests { + +enum Enum0 {}; +enum class Enum1 {}; + +static_assert(QTypeInfo::isValueInitializationBitwiseZero); +static_assert(QTypeInfo::isValueInitializationBitwiseZero); +static_assert(QTypeInfo::isValueInitializationBitwiseZero); +static_assert(QTypeInfo::isValueInitializationBitwiseZero); +static_assert(QTypeInfo::isValueInitializationBitwiseZero); +static_assert(QTypeInfo::isValueInitializationBitwiseZero); +static_assert(QTypeInfo::isValueInitializationBitwiseZero); + +struct A {}; +struct B { B() {} }; +struct C { ~C() {} }; +struct D { D(int) {} }; +struct E { E() {} ~E() {} }; +struct F { int i; }; +struct G { G() : i(0) {} int i; }; +struct H { constexpr H() : i(0) {} int i; }; +struct I { I() : i(42) {} int i; }; +struct J { constexpr J() : i(42) {} int i; }; +struct K { K() : i(0) {} ~K() {} int i; }; + +static_assert(!QTypeInfo::isValueInitializationBitwiseZero); +static_assert(!QTypeInfo::isValueInitializationBitwiseZero); +static_assert(!QTypeInfo::isValueInitializationBitwiseZero); +static_assert(!QTypeInfo::isValueInitializationBitwiseZero); +static_assert(!QTypeInfo::isValueInitializationBitwiseZero); +static_assert(!QTypeInfo::isValueInitializationBitwiseZero); +static_assert(!QTypeInfo::isValueInitializationBitwiseZero); +static_assert(!QTypeInfo::isValueInitializationBitwiseZero); +static_assert(!QTypeInfo::isValueInitializationBitwiseZero); +static_assert(!QTypeInfo::isValueInitializationBitwiseZero); +static_assert(!QTypeInfo::isValueInitializationBitwiseZero); + +} // namespace TriviallyConstructibleTests + +// Value-initializing these trivially constructible types cannot be achieved by +// memset(0) into their storage. For instance, on Itanium, a pointer to a data +// member needs to be value-initialized by setting it to -1. + +// Fits into QVariant +struct TrivialTypeNotZeroInitableSmall { + int TrivialTypeNotZeroInitableSmall::*pdm; +}; + +static_assert(std::is_trivially_default_constructible_v); +static_assert(!QTypeInfo::isValueInitializationBitwiseZero); +static_assert(sizeof(TrivialTypeNotZeroInitableSmall) < sizeof(QVariant)); // also checked more thoroughly below + +// Does not fit into QVariant internal storage +struct TrivialTypeNotZeroInitableBig { + int a; + double b; + char c; + int array[42]; + void (TrivialTypeNotZeroInitableBig::*pmf)(); + int TrivialTypeNotZeroInitableBig::*pdm; +}; + +static_assert(std::is_trivially_default_constructible_v); +static_assert(!QTypeInfo::isValueInitializationBitwiseZero); +static_assert(sizeof(TrivialTypeNotZeroInitableBig) > sizeof(QVariant)); // also checked more thoroughly below + +void tst_QMetaType::defaultConstructTrivial_QTBUG_109594() +{ + // MSVC miscompiles value-initialization of pointers to data members, + // https://developercommunity.visualstudio.com/t/Pointer-to-data-member-is-not-initialize/10238905 + { + QMetaType mt = QMetaType::fromType(); + QVERIFY(mt.isDefaultConstructible()); + auto ptr = static_cast(mt.create()); + const auto cleanup = qScopeGuard([=] { + mt.destroy(ptr); + }); +#ifdef Q_CC_MSVC_ONLY + QEXPECT_FAIL("", "MSVC compiler bug", Continue); +#endif + QCOMPARE(ptr->pdm, nullptr); + + QVariant v(mt); + QVERIFY(QVariant::Private::canUseInternalSpace(mt.iface())); + auto obj = v.value(); +#ifdef Q_CC_MSVC_ONLY + QEXPECT_FAIL("", "MSVC compiler bug", Continue); +#endif + QCOMPARE(obj.pdm, nullptr); + } + + { + QMetaType mt = QMetaType::fromType(); + QVERIFY(mt.isDefaultConstructible()); + auto ptr = static_cast(mt.create()); + const auto cleanup = qScopeGuard([=] { + mt.destroy(ptr); + }); + QCOMPARE(ptr->a, 0); + QCOMPARE(ptr->b, 0.0); + QCOMPARE(ptr->c, '\0'); + QCOMPARE(ptr->pmf, nullptr); + for (int i : ptr->array) + QCOMPARE(i, 0); +#ifdef Q_CC_MSVC_ONLY + QEXPECT_FAIL("", "MSVC compiler bug", Continue); +#endif + QCOMPARE(ptr->pdm, nullptr); + + QVariant v(mt); + QVERIFY(!QVariant::Private::canUseInternalSpace(mt.iface())); + auto obj = v.value(); + QCOMPARE(obj.a, 0); + QCOMPARE(obj.b, 0.0); + QCOMPARE(obj.c, '\0'); + QCOMPARE(obj.pmf, nullptr); + for (int i : obj.array) + QCOMPARE(i, 0); +#ifdef Q_CC_MSVC_ONLY + QEXPECT_FAIL("", "MSVC compiler bug", Continue); +#endif + QCOMPARE(obj.pdm, nullptr); + } +} + void tst_QMetaType::typedConstruct() { auto testMetaObjectWriteOnGadget = [](QVariant &gadget, const QList &properties) diff --git a/tests/auto/corelib/kernel/qmetatype/tst_qmetatype.h b/tests/auto/corelib/kernel/qmetatype/tst_qmetatype.h index 77feff806fc..75bb3635492 100644 --- a/tests/auto/corelib/kernel/qmetatype/tst_qmetatype.h +++ b/tests/auto/corelib/kernel/qmetatype/tst_qmetatype.h @@ -77,6 +77,7 @@ private slots: void flagsBinaryCompatibility6_0(); void construct_data(); void construct(); + void defaultConstructTrivial_QTBUG_109594(); void typedConstruct(); void constructCopy_data(); void constructCopy();