The integer-based geometry classes all risk triggering UB via signed integer overflow. Their representation and APIs exclusively use `int`. This means that for instance one can build a QLine spanning from x1=INT_MIN to x2=INT_MAX, and they try asking its delta-x (dx()), causing an integer overflow in the computation. So far, this has always been "undefined behavior that works in practice", effectively wrapping values around on all of our supported implementation. This choice however makes it impossible to have hardened / ubsan builds of Qt. It also causes some bizarre behaviors: for instance, a maximal QRect (going from (INT_MIN, INT_MIN) to (INT_MAX, INT_MAX)) reports "false" when asked if it contains() a 10x10 rectangle. (That's because QRect::contains() overflows.). Users may not readily notice what is going wrong here. After some thoughts, I've decided that the burden here lies on Qt. While it's true that these geometry classes are nothing but glorified int holders, their representation and handling is entirely chosen and controlled by Qt. The user is not supposed to know "in advance" that QRect holds a couple of points and not, say, a point and a size. Qt also chose the shape of the APIs, taking and returning ints, even when some of these APIs cannot return meaningful results due to the chosen representation. The best we can do at this point is to trigger assertions if some calculation overflows. Therefore, deploy the newly introduced private checked integer class, that will assert if something goes wrong. In release builds they will keep the pre-existing behavior (wraparound), but they will remove the associated UB. The changes are mostly mechanical: 1) change the representation of the geometry classes to use the checked integer (instead of plain `int`); 2) change getters/setters to deal with conversions from and to the checked integer; plus some minimal fallout. Operator overloading on the checked integer keeps the code 99% similar, and doesn't decrease readability. Some care is necessary to try and make all internal calculations use the checked int, instead of falling back to operations on `int`. I've tried to keep the behavior identical as much as possible (except of course for detecting overflow). In particular I haven't reengineered the existing implementations to so something "smarter" or use 64 bit arithmetic if possible or so. The only exception is the QRectF constructor from QRect. Since a QRectF can faithfully represent any QRect, in the conversion I'm taking special care not to call width() or height() as they may overflow. Instead, I calculate them using int64. This allows for a maximal QRect to be converted to a QRectF without loss of information (instead of what was happening before). Some other remarks: * operator/ checks that the divisor is not zero. This was done in several places already and just missing here and there. * QRect::isNull had a bizarre behavior (autotested) where a maximal rectangle was considered null (because of wraparounds). I've decided to keep this behavior. * Several test rows in tst_QRect were relying on overflow behaviors. However, many others were already commented out, with a comment saying "it would cause an overflow". Therefore I've commented them out as well. * The integral classes sometime have functions taking qreal (e.g. `operator*`). In there, qRound is applied to keep the result integral. In principle qRound can also overflow and cause UB, but I've decided to tackle that separately because I'm not sure what should happen (QTBUG-133261). This also happens for the fp->integral conversion functions (e.g. QRectF::toRect()). * The functions are marked constexpr, so add some minimal testing to make sure I'm not breaking it. * Some functions are also marked noexcept. We need to have a broader discussion whether that's actually wrong, and how to recover from there. Task-number: QTBUG-98965 Task-number: QTBUG-132947 Task-number: QTBUG-133261 Change-Id: Idde1daa5c6ca3e37ba0e402617576c5d130615f5 Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>