From e4c6d73f925671a9c96558293472ed213861239c Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@kdab.com>
Date: Mon, 14 Mar 2016 15:18:11 +0100
Subject: [PATCH] QRect: fix UB (int overflow) in center()
QRect::center() should be defined for any
QRect(x1,y1,x2,x2), INT_MIN <= x1, x2, y1, y2 <= INT_MAX
because the average of two signed integers is always
representable as a signed integer.
But not when it's calculated as (x1+x2)/2, since that
expression overflows when x1 > INT_MAX - x2.
Instead of playing games with Hacker's Delight-style
expressions, or use Google's patented algorithm, which
requires two divisions, take advantage of the fact that
int is not intmax_t and perform the calculation in the
qint64 domain. The cast back to int is always well-
defined since, as mentioned, the result is always
representable in an int.
Fix a test-case that expected a nonsensical result due
to overflow.
[ChangeLog][QtCore][QRect] Fixed integer overflow in
center(). This fixes the result for some corner-cases
like a 1x1 rectangle at (INT_MIN, INT_MIN), for which
the previous implementation could return anything
(due to invoking undefined behavior), but commonly
returned (0, 0).
Change-Id: I1a885ca6dff770327dd31655c3eb473fcfeb8878
Reviewed-by: Lars Knoll <lars.knoll@theqtcompany.com>
---
src/corelib/tools/qrect.h | 2 +-
tests/auto/corelib/tools/qrect/tst_qrect.cpp | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/corelib/tools/qrect.h b/src/corelib/tools/qrect.h
index 59cecf9a01e..a0449ae134e 100644
--- a/src/corelib/tools/qrect.h
+++ b/src/corelib/tools/qrect.h
@@ -245,7 +245,7 @@ Q_DECL_CONSTEXPR inline QPoint QRect::bottomLeft() const Q_DECL_NOTHROW
{ return QPoint(x1, y2); }
Q_DECL_CONSTEXPR inline QPoint QRect::center() const Q_DECL_NOTHROW
-{ return QPoint((x1+x2)/2, (y1+y2)/2); }
+{ return QPoint(int((qint64(x1)+x2)/2), int((qint64(y1)+y2)/2)); } // cast avoids overflow on addition
Q_DECL_CONSTEXPR inline int QRect::width() const Q_DECL_NOTHROW
{ return x2 - x1 + 1; }
diff --git a/tests/auto/corelib/tools/qrect/tst_qrect.cpp b/tests/auto/corelib/tools/qrect/tst_qrect.cpp
index c6907dcac27..1377e801a72 100644
--- a/tests/auto/corelib/tools/qrect/tst_qrect.cpp
+++ b/tests/auto/corelib/tools/qrect/tst_qrect.cpp
@@ -2349,7 +2349,7 @@ void tst_QRect::center_data()
QTest::newRow( "SmallestQRect" ) << getQRectCase( SmallestQRect ) << QPoint(1,1);
QTest::newRow( "MiddleQRect" ) << getQRectCase( MiddleQRect ) << QPoint(0,0);
QTest::newRow( "LargestQRect" ) << getQRectCase( LargestQRect ) << QPoint(INT_MAX/2,INT_MAX/2);
- QTest::newRow( "SmallestCoordQRect" ) << getQRectCase( SmallestCoordQRect ) << QPoint(0,0);
+ QTest::newRow( "SmallestCoordQRect" ) << getQRectCase( SmallestCoordQRect ) << QPoint(INT_MIN, INT_MIN);
QTest::newRow( "LargestCoordQRect" ) << getQRectCase( LargestCoordQRect ) << QPoint(0,0);
QTest::newRow( "RandomQRect" ) << getQRectCase( RandomQRect ) << QPoint(105,207);
QTest::newRow( "NegativeSizeQRect" ) << getQRectCase( NegativeSizeQRect ) << QPoint(-4,-4);
--
GitLab