QVector2D/QVector3D/QVector4D: ensure well-defined behavior

The purpose of QVectorND classes is to store N floats packed
together.  One of their usecases is to build arrays of them, then
using them as an array of floats (e.g. when uploading data to
OpenGL).

The design of the class however has a major problem: using separate
members does not guarantee that the compiler does not insert padding
between them (although that could be static-asserted). What's worse,
the implementation of operator[] just does pointer arithmetic on the
first member of the class; that's undefined behavior, and will
trigger ASAN warnings in the future [1].

Solve both problems by using an array of floats instead of
individual x/y/z/w members. Now the compiler is not allowed to
insert hidden padding any more, and makes operator[] well-defined.
However this might be a BIC (IF the compiler added paddings in the
past): hence, add static_asserts checking that the memory layout of
the classes hasn't changed.

For good measure, also add static_asserts checking

1) that the class is standard_layout, so it's safe to
reinterpret_cast it to its first (and only) member;

2) that there's no padding at the _end_ of the class.

Note: an alternative solution to the operator[] problem could've
been leaving the class untouched, and reimplementing the operator
via a switch. Unfortunately none between GCC, clang and MSVC
compile away the switch, dramatically pessimizing the code.

[1] https://github.com/google/sanitizers/wiki/AddressSanitizerIntraObjectOverflow

Change-Id: Iec00ffb6044c58cf728de1754a780068f88152cb
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

1 files changed, 80 insertions, 44 deletions

diff --git a/src/gui/math3d/qvector4d.cpp b/src/gui/math3d/qvector4d.cpp
index 331144ba33..3a68bd6cb7 100644
--- a/src/gui/math3d/qvector4d.cpp
+++ b/src/gui/math3d/qvector4d.cpp
@@ -49,6 +49,42 @@ QT_BEGIN_NAMESPACE
 
 #ifndef QT_NO_VECTOR4D
 
+Q_STATIC_ASSERT_X(std::is_standard_layout<QVector4D>::value, "QVector4D is supposed to be standard layout");
+Q_STATIC_ASSERT_X(sizeof(QVector4D) == sizeof(float) * 4, "QVector4D is not supposed to have padding at the end");
+
+// QVector4D used to be defined as class QVector4D { float x, y, z, w; };,
+// now instead it is defined as classs QVector4D { float v[4]; };.
+// Check that binary compatibility is preserved.
+// ### Qt 6: remove all of these checks.
+
+namespace {
+
+struct QVector4DOld
+{
+    float x, y, z, w;
+};
+
+struct QVector4DNew
+{
+    float v[4];
+};
+
+Q_STATIC_ASSERT_X(std::is_standard_layout<QVector4DOld>::value, "Binary compatibility break in QVector4D");
+Q_STATIC_ASSERT_X(std::is_standard_layout<QVector4DNew>::value, "Binary compatibility break in QVector4D");
+
+Q_STATIC_ASSERT_X(sizeof(QVector4DOld) == sizeof(QVector4DNew), "Binary compatibility break in QVector4D");
+
+// requires a constexpr offsetof
+#if !defined(Q_CC_MSVC) || (_MSC_VER >= 1910)
+Q_STATIC_ASSERT_X(offsetof(QVector4DOld, x) == offsetof(QVector4DNew, v) + sizeof(QVector4DNew::v[0]) * 0, "Binary compatibility break in QVector4D");
+Q_STATIC_ASSERT_X(offsetof(QVector4DOld, y) == offsetof(QVector4DNew, v) + sizeof(QVector4DNew::v[0]) * 1, "Binary compatibility break in QVector4D");
+Q_STATIC_ASSERT_X(offsetof(QVector4DOld, z) == offsetof(QVector4DNew, v) + sizeof(QVector4DNew::v[0]) * 2, "Binary compatibility break in QVector4D");
+Q_STATIC_ASSERT_X(offsetof(QVector4DOld, w) == offsetof(QVector4DNew, v) + sizeof(QVector4DNew::v[0]) * 3, "Binary compatibility break in QVector4D");
+#endif
+
+
+} // anonymous namespace
+
 /*!
     \class QVector4D
     \brief The QVector4D class represents a vector or vertex in 4D space.
@@ -106,10 +142,10 @@ QT_BEGIN_NAMESPACE
 */
 QVector4D::QVector4D(const QVector2D& vector)
 {
-    xp = vector.xp;
-    yp = vector.yp;
-    zp = 0.0f;
-    wp = 0.0f;
+    v[0] = vector.v[0];
+    v[1] = vector.v[1];
+    v[2] = 0.0f;
+    v[3] = 0.0f;
 }
 
 /*!
@@ -120,10 +156,10 @@ QVector4D::QVector4D(const QVector2D& vector)
 */
 QVector4D::QVector4D(const QVector2D& vector, float zpos, float wpos)
 {
-    xp = vector.xp;
-    yp = vector.yp;
-    zp = zpos;
-    wp = wpos;
+    v[0] = vector.v[0];
+    v[1] = vector.v[1];
+    v[2] = zpos;
+    v[3] = wpos;
 }
 
 #endif
@@ -138,10 +174,10 @@ QVector4D::QVector4D(const QVector2D& vector, float zpos, float wpos)
 */
 QVector4D::QVector4D(const QVector3D& vector)
 {
-    xp = vector.xp;
-    yp = vector.yp;
-    zp = vector.zp;
-    wp = 0.0f;
+    v[0] = vector.v[0];
+    v[1] = vector.v[1];
+    v[2] = vector.v[2];
+    v[3] = 0.0f;
 }
 
 /*!
@@ -152,10 +188,10 @@ QVector4D::QVector4D(const QVector3D& vector)
 */
 QVector4D::QVector4D(const QVector3D& vector, float wpos)
 {
-    xp = vector.xp;
-    yp = vector.yp;
-    zp = vector.zp;
-    wp = wpos;
+    v[0] = vector.v[0];
+    v[1] = vector.v[1];
+    v[2] = vector.v[2];
+    v[3] = wpos;
 }
 
 #endif
@@ -258,10 +294,10 @@ QVector4D::QVector4D(const QVector3D& vector, float wpos)
 float QVector4D::length() const
 {
     // Need some extra precision if the length is very small.
-    double len = double(xp) * double(xp) +
-                 double(yp) * double(yp) +
-                 double(zp) * double(zp) +
-                 double(wp) * double(wp);
+    double len = double(v[0]) * double(v[0]) +
+                 double(v[1]) * double(v[1]) +
+                 double(v[2]) * double(v[2]) +
+                 double(v[3]) * double(v[3]);
     return float(std::sqrt(len));
 }
 
@@ -273,7 +309,7 @@ float QVector4D::length() const
 */
 float QVector4D::lengthSquared() const
 {
-    return xp * xp + yp * yp + zp * zp + wp * wp;
+    return v[0] * v[0] + v[1] * v[1] + v[2] * v[2] + v[3] * v[3];
 }
 
 /*!
@@ -288,18 +324,18 @@ float QVector4D::lengthSquared() const
 QVector4D QVector4D::normalized() const
 {
     // Need some extra precision if the length is very small.
-    double len = double(xp) * double(xp) +
-                 double(yp) * double(yp) +
-                 double(zp) * double(zp) +
-                 double(wp) * double(wp);
+    double len = double(v[0]) * double(v[0]) +
+                 double(v[1]) * double(v[1]) +
+                 double(v[2]) * double(v[2]) +
+                 double(v[3]) * double(v[3]);
     if (qFuzzyIsNull(len - 1.0f)) {
         return *this;
     } else if (!qFuzzyIsNull(len)) {
         double sqrtLen = std::sqrt(len);
-        return QVector4D(float(double(xp) / sqrtLen),
-                         float(double(yp) / sqrtLen),
-                         float(double(zp) / sqrtLen),
-                         float(double(wp) / sqrtLen));
+        return QVector4D(float(double(v[0]) / sqrtLen),
+                         float(double(v[1]) / sqrtLen),
+                         float(double(v[2]) / sqrtLen),
+                         float(double(v[3]) / sqrtLen));
     } else {
         return QVector4D();
     }
@@ -314,19 +350,19 @@ QVector4D QVector4D::normalized() const
 void QVector4D::normalize()
 {
     // Need some extra precision if the length is very small.
-    double len = double(xp) * double(xp) +
-                 double(yp) * double(yp) +
-                 double(zp) * double(zp) +
-                 double(wp) * double(wp);
+    double len = double(v[0]) * double(v[0]) +
+                 double(v[1]) * double(v[1]) +
+                 double(v[2]) * double(v[2]) +
+                 double(v[3]) * double(v[3]);
     if (qFuzzyIsNull(len - 1.0f) || qFuzzyIsNull(len))
         return;
 
     len = std::sqrt(len);
 
-    xp = float(double(xp) / len);
-    yp = float(double(yp) / len);
-    zp = float(double(zp) / len);
-    wp = float(double(wp) / len);
+    v[0] = float(double(v[0]) / len);
+    v[1] = float(double(v[1]) / len);
+    v[2] = float(double(v[2]) / len);
+    v[3] = float(double(v[3]) / len);
 }
 
 /*!
@@ -387,7 +423,7 @@ void QVector4D::normalize()
 */
 float QVector4D::dotProduct(const QVector4D& v1, const QVector4D& v2)
 {
-    return v1.xp * v2.xp + v1.yp * v2.yp + v1.zp * v2.zp + v1.wp * v2.wp;
+    return v1.v[0] * v2.v[0] + v1.v[1] * v2.v[1] + v1.v[2] * v2.v[2] + v1.v[3] * v2.v[3];
 }
 
 /*!
@@ -503,7 +539,7 @@ float QVector4D::dotProduct(const QVector4D& v1, const QVector4D& v2)
 */
 QVector2D QVector4D::toVector2D() const
 {
-    return QVector2D(xp, yp);
+    return QVector2D(v[0], v[1]);
 }
 
 /*!
@@ -515,9 +551,9 @@ QVector2D QVector4D::toVector2D() const
 */
 QVector2D QVector4D::toVector2DAffine() const
 {
-    if (qIsNull(wp))
+    if (qIsNull(v[3]))
         return QVector2D();
-    return QVector2D(xp / wp, yp / wp);
+    return QVector2D(v[0] / v[3], v[1] / v[3]);
 }
 
 #endif
@@ -531,7 +567,7 @@ QVector2D QVector4D::toVector2DAffine() const
 */
 QVector3D QVector4D::toVector3D() const
 {
-    return QVector3D(xp, yp, zp);
+    return QVector3D(v[0], v[1], v[2]);
 }
 
 /*!
@@ -542,9 +578,9 @@ QVector3D QVector4D::toVector3D() const
 */
 QVector3D QVector4D::toVector3DAffine() const
 {
-    if (qIsNull(wp))
+    if (qIsNull(v[3]))
         return QVector3D();
-    return QVector3D(xp / wp, yp / wp, zp / wp);
+    return QVector3D(v[0] / v[3], v[1] / v[3], v[2] / v[3]);
 }
 
 #endif