From 065527825e19a8a5eb2efa0ab481980dc86ad58a Mon Sep 17 00:00:00 2001
From: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>
Date: Thu, 7 Jan 2021 08:20:42 +0100
Subject: [PATCH] Remove false Q_UNREACHABLE from shaping code

This was added by 9ff76c27b9031ae7c49c4c9e8b5a3bea1e0e3c78 on
the basis that it signifies a shaping error and would later assert
or crash.

But the line is easily reachable by user code. If Harfbuzz returns
0 glyphs, it just means it is unable to shape the string, for instance
if the input string only contains default ignorables (like a ZWJ)
and does not have any appropriate glyph to use for replacement.

Qt expects there to always be at least one glyph in the output
(num_glyphs == 0 is used to indicate shaping is not yet done), so
to avoid asserts later on, we simply populate the output with a
single 0 token, which is a required entry in the font that is
reserved for representing unrepresentable characters.

This also adds a test and therefore a zero-width joiner to the test
font to reproduce the issue.

[ChangeLog][QtGui][Text] Fixed a possible crash with certain fonts
when shaping strings consisting only of control characters.

Fixes: QTBUG-89155
Change-Id: Ia0dd6a04844c9be90dcab6c464bebe339a3dab11
Reviewed-by: Konstantin Ritt <ritt.ks@gmail.com>
(cherry picked from commit fccd419dd632306a4bd85928223e0a56a59510ef)
---
 src/gui/text/qtextengine.cpp                  |  13 +++++++++++--
 .../auto/gui/text/qglyphrun/tst_qglyphrun.cpp |  16 ++++++++++++++++
 tests/auto/shared/resources/test.ttf          | Bin 2008 -> 2128 bytes
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp
index f8ce0ba8af5..28802c9bed3 100644
--- a/src/gui/text/qtextengine.cpp
+++ b/src/gui/text/qtextengine.cpp
@@ -1546,12 +1546,21 @@ void QTextEngine::shapeText(int item) const
 
         si.num_glyphs = glyph_pos;
     }
+
     if (Q_UNLIKELY(si.num_glyphs == 0)) {
-        Q_UNREACHABLE(); // ### report shaping errors somehow
+        if (Q_UNLIKELY(!ensureSpace(si.glyph_data_offset + 1))) {
+            qWarning() << "Unable to allocate space for place-holder glyph";
+            return;
+        }
+
+        si.num_glyphs = 1;
+
+        // Overwrite with 0 token to indicate failure
+        QGlyphLayout g = availableGlyphs(&si);
+        g.glyphs[0] = 0;
         return;
     }
 
-
     layoutData->used += si.num_glyphs;
 
     QGlyphLayout glyphs = shapedGlyphs(&si);
diff --git a/tests/auto/gui/text/qglyphrun/tst_qglyphrun.cpp b/tests/auto/gui/text/qglyphrun/tst_qglyphrun.cpp
index cf038775167..973a615d5ec 100644
--- a/tests/auto/gui/text/qglyphrun/tst_qglyphrun.cpp
+++ b/tests/auto/gui/text/qglyphrun/tst_qglyphrun.cpp
@@ -63,6 +63,7 @@ private slots:
     void boundingRect();
     void mixedScripts();
     void multiLineBoundingRect();
+    void defaultIgnorables();
 
 private:
     int m_testFontId;
@@ -631,6 +632,21 @@ void tst_QGlyphRun::multiLineBoundingRect()
     QVERIFY(firstLineGlyphRun.boundingRect().height() < allGlyphRun.boundingRect().height());
 }
 
+void tst_QGlyphRun::defaultIgnorables()
+{
+    QTextLayout layout;
+    layout.setFont(QFont("QtsSpecialTestFont"));
+    layout.setText(QChar(0x200D));
+    layout.beginLayout();
+    layout.createLine();
+    layout.endLayout();
+
+    QList<QGlyphRun> runs = layout.glyphRuns();
+    QCOMPARE(runs.size(), 1);
+    QCOMPARE(runs.at(0).glyphIndexes().size(), 1);
+    QCOMPARE(runs.at(0).glyphIndexes()[0], 0);
+}
+
 #endif // QT_NO_RAWFONT
 
 QTEST_MAIN(tst_QGlyphRun)
diff --git a/tests/auto/shared/resources/test.ttf b/tests/auto/shared/resources/test.ttf
index 382b2547b040e4dee9e531118988caeb748177dc..b2bb6cd036eae9b4b127f88d018e7f30106787f6 100644
GIT binary patch
delta 683
zcmaiwPe>GT6vw~sH#;*X+FEP}(PRn<rL1*~-B6MKNQa6N>`*4S<2Jg&&hD^|u?xYL
z2hY(QlkgA(odO}ccJt67Iu!_Y^w2e;)AaqVi|nl#-tYH&-}m{w_j~VK`b%FE0-zVq
zU|{av{3UnOE58Qh0o6h=@WLH$d5HdJ+CuS3<RHTcQafDKO1K(W54ZDxF~a!zYPqrU
zD!=3~{+{8vlJ7k>zdzONJ}p<GV3wpwR%aP51yQr!Z6yT?A1P$Y)uN})sYxbyL%%2R
z>Y+L%5#v|rI~6bR`xZv|IGe7V46Ex=|Ma^W(4`YjhBZHAqVv4JO+VFYjt&0G|9!NS
zp8kyi<A6H$%n$u~-yc((2QMXWn=R6eXXLazj~8WB5O2x#MR}ny&gRetTNrSkik{mb
zOX!DnqYH}vIIcOpKanKVj?T%~<kOEoj<oMGq(8CZGioT#sx-|yp(}KLsw1k2|5?xO
zRkbBbMt1j%d3D>uUH!z=F%})h@%CBqq#Mz?fte#c)xnsk<4&8gv>uD?7p8Cv9xB-2
zT0;X4Tx4;fy}N973O7+D5#a_2pIk)0Mq(TjxJdfYT;w>i3F<jq#u%2dMlybDpB*qc
X6~&SnG0fR&6gBLP%9@+a&I11cMuu}#

delta 733
zcmYjNF=!J}82;YfC8?sO)`*IgyrC4uRCA%2AX1EICaKjFsYPhd_<A|GOOD(nv<^lc
zx+(@-EVw!fRdH|;myV^AgM)5vP7Y4-yYsB&c<=xI@BjY){`YN<e#!PB035_4=$N}v
zKVN!s&;1FA1+vQx&+_4YUu3?;e6`UFC2~a2Kk(94eA`Rk*(eiUrY5l6wUw9UCCNJV
z+FVmv_kVx(=K*}>xzHq$YKn){MpGu5Uf5q9=?#}jL<2|NcEi#p#TW;CWWL|CR(&le
zLe?wHWy|tZt(NzI^l@@e{dN%g*B%@JGOt-L`W@wS&{_7sWp2ce;{&H2Kg%s;Fa5$v
z{Req$rQV;2;`TAExA9z*Gk=xFU`Cu0JA=F8j1Yr$ab<xTJvrm?FyK0w$g^>dFEWns
z(-=)uzhOB1kA^$LPf{dZq>)A;t{#RyFtxnUHc#v8g1wt>jJno8fES5aU<{cAYeal%
z)XyL1cX?xvCWR?XCYp52b46nxnF(0Yy2Kg|p`KtJGrPR;Ph(S*!XjQJqI6WarR=V2
zb#8`1-B%6Aa_3bL*4nM`;%(IloOVl^=ZeL>SXR4sHwdL!DacaMoGwVG<%H~%9px%3
zP=ncwnw6-c3!5iE2ubrQG)JODOHEA2*g*>ps}6T{S?fSCDrN!i*mHU^e1{U<JeoD=
OlE$5LsjKZUT>ArfRD!zz