Fix crash due to a stale pointer dereferencing

The HB face caching mechanism introduced in 227e9a4 wasn't complete due that fact that HB-NG doesn't parse the entire font table at once but rather references a table on-demand. This incompleteness caused a crash in case the engine doesn't get cached or when it removed from the cache and then re-used. Task-number: QTBUG-36099 Change-Id: I7816836107655ce7cf6eb9683bb5dc7f892f9cd1 Reviewed-by: Lisandro Damián Nicanor Pérez Meyer <[email protected]> Reviewed-by: Michael Krasnyk <[email protected]> Reviewed-by: Lars Knoll <[email protected]> Reviewed-by: Allan Sandfeld Jensen <[email protected]>
petrmanek · Jan 16, 2014 · 84be1bd · 84be1bd
1 parent 2565ef2
commit 84be1bd
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 18 deletions.
diff --git a/src/gui/text/qfontengine.cpp b/src/gui/text/qfontengine.cpp
@@ -171,7 +171,8 @@ static const HB_FontClass hb_fontClass = {
 static HB_Error hb_getSFntTable(void *font, HB_Tag tableTag, HB_Byte *buffer, HB_UInt *length)
 {
     QFontEngine *fe = (QFontEngine *)font;
-    if (!fe->getSfntTableData(tableTag, buffer, length))
+    Q_ASSERT(fe->faceData.get_font_table);
+    if (!fe->faceData.get_font_table(fe->faceData.user_data, tableTag, buffer, length))
         return HB_Err_Invalid_Argument;
     return HB_Err_Ok;
 }
@@ -182,6 +183,13 @@ static void hb_freeFace(void *face)
 }
 
 
+static bool qt_get_font_table_default(void *user_data, uint tag, uchar *buffer, uint *length)
+{
+    QFontEngine *fe = (QFontEngine *)user_data;
+    return fe->getSfntTableData(tag, buffer, length);
+}
+
+
 #ifdef QT_BUILD_INTERNAL
 // for testing purpose only, not thread-safe!
 static QList<QFontEngine *> *enginesCollector = 0;
@@ -210,6 +218,9 @@ QFontEngine::QFontEngine()
       font_(0), font_destroy_func(0),
       face_(0), face_destroy_func(0)
 {
+    faceData.user_data = this;
+    faceData.get_font_table = qt_get_font_table_default;
+
     cache_cost = 0;
     fsType = 0;
     symbol = false;

diff --git a/src/gui/text/qfontengine_ft.cpp b/src/gui/text/qfontengine_ft.cpp
@@ -116,6 +116,21 @@ QT_BEGIN_NAMESPACE
 #define TRUNC(x)    ((x) >> 6)
 #define ROUND(x)    (((x)+32) & -64)
 
+static bool ft_getSfntTable(void *user_data, uint tag, uchar *buffer, uint *length)
+{
+    FT_Face face = (FT_Face)user_data;
+
+    bool result = false;
+    if (FT_IS_SFNT(face)) {
+        FT_ULong len = *length;
+        result = FT_Load_Sfnt_Table(face, tag, 0, buffer, &len) == FT_Err_Ok;
+        *length = len;
+    }
+
+    return result;
+}
+
+
 // -------------------------- Freetype support ------------------------------
 
 class QtFreetypeData
@@ -408,15 +423,7 @@ QFontEngine::Properties QFreetypeFace::properties() const
 
 bool QFreetypeFace::getSfntTable(uint tag, uchar *buffer, uint *length) const
 {
-    bool result = false;
-#if (FREETYPE_MAJOR*10000 + FREETYPE_MINOR*100 + FREETYPE_PATCH) > 20103
-    if (FT_IS_SFNT(face)) {
-        FT_ULong len = *length;
-        result = FT_Load_Sfnt_Table(face, tag, 0, buffer, &len) == FT_Err_Ok;
-        *length = len;
-    }
-#endif
-    return result;
+    return ft_getSfntTable(face, tag, buffer, length);
 }
 
 /* Some fonts (such as MingLiu rely on hinting to scale different
@@ -761,6 +768,8 @@ bool QFontEngineFT::init(FaceId faceId, bool antialias, GlyphFormat format,
     fontDef.styleName = QString::fromUtf8(face->style_name);
 
     if (!freetype->hbFace) {
+        faceData.user_data = face;
+        faceData.get_font_table = ft_getSfntTable;
         freetype->hbFace = harfbuzzFace();
         freetype->hbFace_destroy_func = face_destroy_func;
     } else {
@@ -1179,7 +1188,7 @@ QFixed QFontEngineFT::emSquareSize() const
 
 bool QFontEngineFT::getSfntTableData(uint tag, uchar *buffer, uint *length) const
 {
-    return freetype->getSfntTable(tag, buffer, length);
+    return ft_getSfntTable(freetype->face, tag, buffer, length);
 }
 
 int QFontEngineFT::synthesized() const

diff --git a/src/gui/text/qfontengine_p.h b/src/gui/text/qfontengine_p.h
@@ -85,6 +85,7 @@ enum HB_Compat_Error {
 };
 
 typedef void (*qt_destroy_func_t) (void *user_data);
+typedef bool (*qt_get_font_table_func_t) (void *user_data, uint tag, uchar *buffer, uint *length);
 
 class Q_GUI_EXPORT QFontEngine
 {
@@ -280,6 +281,10 @@ class Q_GUI_EXPORT QFontEngine
     mutable qt_destroy_func_t font_destroy_func;
     mutable void *face_;
     mutable qt_destroy_func_t face_destroy_func;
+    struct FaceData {
+        void *user_data;
+        qt_get_font_table_func_t get_font_table;
+    } faceData;
 
     uint cache_cost; // amount of mem used in kb by the font
     uint fsType : 16;

diff --git a/src/gui/text/qharfbuzzng.cpp b/src/gui/text/qharfbuzzng.cpp
@@ -623,19 +623,22 @@ hb_font_funcs_t *hb_qt_get_font_funcs()
 
 
 static hb_blob_t *
-_hb_qt_get_font_table(hb_face_t * /*face*/, hb_tag_t tag, void *user_data)
+_hb_qt_reference_table(hb_face_t * /*face*/, hb_tag_t tag, void *user_data)
 {
-    QFontEngine *fe = (QFontEngine *)user_data;
-    Q_ASSERT(fe);
+    QFontEngine::FaceData *data = (QFontEngine::FaceData *)user_data;
+    Q_ASSERT(data);
+
+    qt_get_font_table_func_t get_font_table = data->get_font_table;
+    Q_ASSERT(get_font_table);
 
     uint length = 0;
-    if (Q_UNLIKELY(!fe->getSfntTableData(tag, 0, &length) || length == 0))
+    if (Q_UNLIKELY(!get_font_table(data->user_data, tag, 0, &length) || length == 0))
         return hb_blob_get_empty();
 
     char *buffer = (char *)malloc(length);
     Q_CHECK_PTR(buffer);
 
-    if (Q_UNLIKELY(!fe->getSfntTableData(tag, reinterpret_cast<uchar *>(buffer), &length)))
+    if (Q_UNLIKELY(!get_font_table(data->user_data, tag, reinterpret_cast<uchar *>(buffer), &length)))
         length = 0;
 
     return hb_blob_create(const_cast<const char *>(buffer), length,
@@ -646,9 +649,14 @@ _hb_qt_get_font_table(hb_face_t * /*face*/, hb_tag_t tag, void *user_data)
 static inline hb_face_t *
 _hb_qt_face_create(QFontEngine *fe)
 {
-    hb_face_t *face;
+    Q_ASSERT(fe);
+
+    QFontEngine::FaceData *data = (QFontEngine::FaceData *)malloc(sizeof(QFontEngine::FaceData));
+    Q_CHECK_PTR(data);
+    data->user_data = fe->faceData.user_data;
+    data->get_font_table = fe->faceData.get_font_table;
 
-    face = hb_face_create_for_tables(_hb_qt_get_font_table, (void *)fe, NULL);
+    hb_face_t *face = hb_face_create_for_tables(_hb_qt_reference_table, (void *)data, free);
     if (Q_UNLIKELY(hb_face_is_immutable(face))) {
         hb_face_destroy(face);
         return NULL;