From a66f51fe8298cd334be8cd11fcab3892b3cf3a10 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Tue, 28 Jan 2025 14:33:48 +0100 Subject: [PATCH] QPicture: (almost) work around overflow in bounding box calculations QPicture keeps a bounding box of its drawing, and updates it as it processes paint commands, extending it if necessary. The problem is that these calculations are prone to overflows: * drawing (and thus the bounding box) is floating point based, but QPicture only stores a QRect. If the drawing's BB overflows the integer range, QPicture overflows. * Since the stored QRect is `int` based (signed), if the drawing spans "too far" (e.g. from INT_MIN/2 to INT_MAX/2), it'll again overflow the range. This impacts serialization, as the protocol serializes the bounding box's topleft+size. For the former, clamp the qreal-based into the integer range before the conversion. For the latter, as a dirty work around, when calculating a new bounding box, directly set the bounding box corners instead of topleft+size. This is NOT A FIX by any means. I'm trading one source of UB for another; the only objective is hiding an assertion from a test. Some design change is needed to properly fix this (keeping in mind the stability of the serialized format). Task-number: QTBUG-133293 Change-Id: Ia741e1e73bdf7eee21f66b4ea5c1fae3430b468d Reviewed-by: Edward Welbourne Reviewed-by: Allan Sandfeld Jensen --- src/gui/image/qpaintengine_pic.cpp | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/gui/image/qpaintengine_pic.cpp b/src/gui/image/qpaintengine_pic.cpp index 438305742cb..ba4888cef75 100644 --- a/src/gui/image/qpaintengine_pic.cpp +++ b/src/gui/image/qpaintengine_pic.cpp @@ -317,19 +317,25 @@ void QPicturePaintEngine::writeCmdLength(int pos, const QRectF &r, bool corr) } if (br.width() > 0.0 || br.height() > 0.0) { - int minx = qFloor(br.left()); - int miny = qFloor(br.top()); - int maxx = qCeil(br.right()); - int maxy = qCeil(br.bottom()); + const auto clampToIntRange = [](qreal v) + { + return qBound(qreal((std::numeric_limits::min)()), + v, + qreal((std::numeric_limits::max)())); + }; + int minx = qFloor(clampToIntRange(br.left())); + int miny = qFloor(clampToIntRange(br.top())); + int maxx = qCeil(clampToIntRange(br.right())); + int maxy = qCeil(clampToIntRange(br.bottom())); if (d->pic_d->brect.width() > 0 || d->pic_d->brect.height() > 0) { minx = qMin(minx, d->pic_d->brect.left()); miny = qMin(miny, d->pic_d->brect.top()); maxx = qMax(maxx, d->pic_d->brect.x() + d->pic_d->brect.width()); maxy = qMax(maxy, d->pic_d->brect.y() + d->pic_d->brect.height()); - d->pic_d->brect = QRect(minx, miny, maxx - minx, maxy - miny); + d->pic_d->brect.setCoords(minx, miny, maxx - 1, maxy - 1); } else { - d->pic_d->brect = QRect(minx, miny, maxx - minx, maxy - miny); + d->pic_d->brect.setCoords(minx, miny, maxx - 1, maxy - 1); } } }