Skip to content

Commit

Permalink
Fix QTextEngine regression with large-ish texts
Browse files Browse the repository at this point in the history
Change 997fd3b fixed integer overflows
with huge texts. This was done by using qsizetype for size calculations
instead of int. However, that change introduced a serious regression
due to an itermediate imultiplication result being "promoted" to unsigned,
and therefore a negative value being converted to a large positive.
The solution is to make sure all values in the expression are signed.

Fixes: QTBUG-123339
Task-number: QTBUG-119611
Pick-to: 6.7
Change-Id: I3f9189f77b383c6103cf5b35981cdb607b065f6f
Reviewed-by: Eskil Abrahamsen Blomfeldt <[email protected]>
  • Loading branch information
paulolav committed Mar 15, 2024
1 parent f944651 commit 7a84c58
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 7 deletions.
7 changes: 4 additions & 3 deletions src/gui/text/qtextengine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2658,9 +2658,10 @@ QTextEngine::LayoutData::LayoutData(const QString &str, void **stack_memory, qsi
{
allocated = _allocated;

qsizetype space_charAttributes = sizeof(QCharAttributes) * string.size() / sizeof(void*) + 1;
qsizetype space_logClusters = sizeof(unsigned short) * string.size() / sizeof(void*) + 1;
available_glyphs = (allocated - space_charAttributes - space_logClusters) * sizeof(void*) / QGlyphLayout::SpaceNeeded;
constexpr qsizetype voidSize = sizeof(void*);
qsizetype space_charAttributes = sizeof(QCharAttributes) * string.size() / voidSize + 1;
qsizetype space_logClusters = sizeof(unsigned short) * string.size() / voidSize + 1;
available_glyphs = (allocated - space_charAttributes - space_logClusters) * voidSize / QGlyphLayout::SpaceNeeded;

if (available_glyphs < str.size()) {
// need to allocate on the heap
Expand Down
6 changes: 2 additions & 4 deletions src/gui/text/qtextengine_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,8 @@ Q_DECLARE_TYPEINFO(QGlyphAttributes, Q_PRIMITIVE_TYPE);

struct QGlyphLayout
{
enum {
SpaceNeeded = sizeof(glyph_t) + sizeof(QFixed) + sizeof(QFixedPoint)
+ sizeof(QGlyphAttributes) + sizeof(QGlyphJustification)
};
static constexpr qsizetype SpaceNeeded = sizeof(glyph_t) + sizeof(QFixed) + sizeof(QFixedPoint)
+ sizeof(QGlyphAttributes) + sizeof(QGlyphJustification);

// init to 0 not needed, done when shaping
QFixedPoint *offsets; // 8 bytes per element
Expand Down
22 changes: 22 additions & 0 deletions tests/auto/gui/text/qfontmetrics/tst_qfontmetrics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ private slots:
void zeroWidthMetrics();
void verticalMetrics_data();
void verticalMetrics();
void largeText_data();
void largeText(); // QTBUG-123339
};

void tst_QFontMetrics::same()
Expand Down Expand Up @@ -388,5 +390,25 @@ void tst_QFontMetrics::verticalMetrics()
QVERIFY(fm.ascent() != 0 || fm.descent() != 0);
}

void tst_QFontMetrics::largeText_data()
{
QTest::addColumn<qsizetype>("size");
for (int i = 1; i < 20; ++i) {
qsizetype size = qsizetype(1) << i;
QByteArray rowText = QByteArray::number(size);
QTest::newRow(rowText.constData()) << size;
}
}

void tst_QFontMetrics::largeText()
{
QFont font;
QFontMetrics fm(font);
QFETCH(qsizetype, size);
QString string(size, QLatin1Char('A'));
QRect boundingRect = fm.boundingRect(string);
QVERIFY(boundingRect.isValid());
}

QTEST_MAIN(tst_QFontMetrics)
#include "tst_qfontmetrics.moc"

0 comments on commit 7a84c58

Please sign in to comment.