From c8f3becb6da40d7dd8a059de0d2db265d38a495d Mon Sep 17 00:00:00 2001 From: Andrew Hayzen Date: Tue, 14 Feb 2023 17:35:56 +0000 Subject: [PATCH] cxx-qt-lib: ensure correct sizes for wasm 32bit Related to #414 --- CHANGELOG.md | 1 + crates/cxx-qt-lib/src/core/qmodelindex.cpp | 7 ++- crates/cxx-qt-lib/src/core/qmodelindex.rs | 5 ++- crates/cxx-qt-lib/src/core/qvariant/mod.rs | 16 ++++++- .../cxx-qt-lib/src/core/qvariant/qvariant.cpp | 43 ++++++++++++++----- crates/cxx-qt-lib/src/gui/qcolor.cpp | 8 ++-- crates/cxx-qt-lib/src/gui/qcolor.rs | 4 +- 7 files changed, 62 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4610e7cbb..42172a2b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Support for generating correct C++ code for `Pin` Rust types - Support namespace attribute on shared types, QObject struct, and extern blocks +- Asserts for 32bit platforms such as Wasm ## [0.4.1](https://github.com/KDAB/cxx-qt/compare/v0.4.0...v0.4.1) - 2022-11-18 diff --git a/crates/cxx-qt-lib/src/core/qmodelindex.cpp b/crates/cxx-qt-lib/src/core/qmodelindex.cpp index e55f09053..ce08ed23c 100644 --- a/crates/cxx-qt-lib/src/core/qmodelindex.cpp +++ b/crates/cxx-qt-lib/src/core/qmodelindex.cpp @@ -8,13 +8,12 @@ #include "../assertion_utils.h" -// QModelIndex has two ints, a uint pointer, and a pointer. -// This results in 4 + 4 + 4 + 8 = 20, then due to compiler padding this results -// in size of 24 or three pointers. +// QModelIndex has two ints, a quint pointer (same as size_t), and a pointer. // https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/itemmodels/qabstractitemmodel.h?h=v5.15.6-lts-lgpl#n93 // https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/itemmodels/qabstractitemmodel.h?h=v6.2.4#n195 assert_alignment_and_size(QModelIndex, alignof(::std::size_t), - sizeof(::std::size_t[3])); + (sizeof(::std::int32_t) * 2) + sizeof(::std::size_t) + + sizeof(::std::size_t)); static_assert(::std::is_trivially_copyable::value); diff --git a/crates/cxx-qt-lib/src/core/qmodelindex.rs b/crates/cxx-qt-lib/src/core/qmodelindex.rs index 751a52c37..26532f490 100644 --- a/crates/cxx-qt-lib/src/core/qmodelindex.rs +++ b/crates/cxx-qt-lib/src/core/qmodelindex.rs @@ -56,7 +56,10 @@ mod ffi { #[derive(Clone)] #[repr(C)] pub struct QModelIndex { - _space: MaybeUninit<[usize; 3]>, + _r: MaybeUninit, + _c: MaybeUninit, + _i: MaybeUninit, + _m: MaybeUninit, } impl Default for QModelIndex { diff --git a/crates/cxx-qt-lib/src/core/qvariant/mod.rs b/crates/cxx-qt-lib/src/core/qvariant/mod.rs index 985bffaff..7e33df9c5 100644 --- a/crates/cxx-qt-lib/src/core/qvariant/mod.rs +++ b/crates/cxx-qt-lib/src/core/qvariant/mod.rs @@ -51,10 +51,22 @@ pub struct QVariant { /// /// Qt5 QVariant has one member, which contains three uints (but they are optimised to a size of 8) and a union /// Qt6 QVariant has one member, which contains three pointers and a union (pointer largest) + _data: MaybeUninit, + + // Compiler optimisations reduce the size of the uint to a ushort + #[cfg(qt_version_major = "5")] + _type: MaybeUninit, + #[cfg(qt_version_major = "5")] + _is_shared: MaybeUninit, #[cfg(qt_version_major = "5")] - _space: MaybeUninit<[usize; 2]>, + _is_null: MaybeUninit, + + #[cfg(qt_version_major = "6")] + _is_shared: MaybeUninit, + #[cfg(qt_version_major = "6")] + _is_null: MaybeUninit, #[cfg(qt_version_major = "6")] - _space: MaybeUninit<[usize; 4]>, + _packed_type: MaybeUninit, } impl Clone for QVariant { diff --git a/crates/cxx-qt-lib/src/core/qvariant/qvariant.cpp b/crates/cxx-qt-lib/src/core/qvariant/qvariant.cpp index cb3f95e5d..6c42a0a4e 100644 --- a/crates/cxx-qt-lib/src/core/qvariant/qvariant.cpp +++ b/crates/cxx-qt-lib/src/core/qvariant/qvariant.cpp @@ -13,24 +13,45 @@ // The layout has changed between Qt 5 and Qt 6 // -// Qt5 QVariant has one member, which contains three uints and a union. -// The three uints are optimised to a reduced size, resulting in a combined size -// of two pointers. -// https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/kernel/qvariant.h?h=v5.15.6-lts-lgpl#n491 -// https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/kernel/qvariant.h?h=v5.15.6-lts-lgpl#n411 -// // Qt6 QVariant has one member, which contains three pointers and a union -// (with a pointer as the largest member) +// (with a pointer / double as the largest member) // https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/kernel/qvariant.h?h=v6.2.4#n540 // https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/kernel/qvariant.h?h=v6.2.4#n474 +// +// Qt5 QVariant has one member, which contains three uints and a union +// (with a pointer / double as the largest member) +// The three uints are optimised to a reduced size of ushorts +// https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/kernel/qvariant.h?h=v5.15.6-lts-lgpl#n491 +// https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/kernel/qvariant.h?h=v5.15.6-lts-lgpl#n411 #if (QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)) + +#if (QT_POINTER_SIZE == 4) +// 32bit is 3 * 32bit ptr (12) + union with double (8) + 4 bytes padding +// alignment is 8 byte on 32bit systems as well due to the double assert_alignment_and_size(QVariant, - alignof(::std::size_t), - sizeof(::std::size_t[4])); + alignof(double), + (sizeof(::std::size_t) * 3) + sizeof(double) + + 4 /* compiler padding */); #else +// 64bit is 3 * 64ptr ptr (16) + union with double (8) +// alignment is 8 bytes from the double or the pointer on 64bit systems assert_alignment_and_size(QVariant, - alignof(::std::size_t), - sizeof(::std::size_t[2])); + alignof(double), + (sizeof(::std::size_t) * 3) + sizeof(double)); +#endif + +#else + +// 3 * uint (12) + union with double (8) +// but due to compiler optimisation it ends up as +// 3 * ushort (6) + union with double (8) + 2 bytes padding +// alignment is 8 byte on 32bit systems as well due to the double +assert_alignment_and_size( + QVariant, + alignof(double), + (sizeof(::std::uint16_t /* compiler optimised from ::std::uint32_t */) * 3) + + sizeof(double) + 2 /* compiler padding */); + #endif static_assert(!::std::is_trivially_copy_assignable::value); diff --git a/crates/cxx-qt-lib/src/gui/qcolor.cpp b/crates/cxx-qt-lib/src/gui/qcolor.cpp index 0af9d25b3..1ed496906 100644 --- a/crates/cxx-qt-lib/src/gui/qcolor.cpp +++ b/crates/cxx-qt-lib/src/gui/qcolor.cpp @@ -12,13 +12,15 @@ #include "../assertion_utils.h" // QColor has an enum with six values and a union with the largest being five -// ushorts. This results in (5 * std::uint16) + std::uint32_t = 14, then due to -// compiler padding this results in a sizeof 16 or two pointers. +// ushorts. This results in std::int32_t + (5 * std::uint16) = 14, then due to +// compiler padding this results in a sizeof 16. // https://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/painting/qcolor.h?h=v5.15.6-lts-lgpl#n262 // https://code.qt.io/cgit/qt/qtbase.git/tree/src/gui/painting/qcolor.h?h=v6.2.4#n237 assert_alignment_and_size(QColor, alignof(::std::size_t), - sizeof(::std::size_t[2])); + sizeof(::std::int32_t) + + (sizeof(::std::uint16_t) * 5) + + 2 /* compiler padding */); // QColor still had copy & move constructors in Qt 5 but they were basically // trivial. diff --git a/crates/cxx-qt-lib/src/gui/qcolor.rs b/crates/cxx-qt-lib/src/gui/qcolor.rs index f85e7fbed..783d237b9 100644 --- a/crates/cxx-qt-lib/src/gui/qcolor.rs +++ b/crates/cxx-qt-lib/src/gui/qcolor.rs @@ -58,7 +58,9 @@ mod ffi { #[derive(Clone)] #[repr(C)] pub struct QColor { - _space: MaybeUninit<[usize; 2]>, + _cspec: MaybeUninit, + _ct: MaybeUninit<[u16; 5]>, + _padding: MaybeUninit, } impl QColor {