From 84be1bd4d3ed8d2d9e65301649bc841ea4197fe2 Mon Sep 17 00:00:00 2001 From: Konstantin Ritt Date: Tue, 14 Jan 2014 18:27:33 +0200 Subject: Fix crash due to a stale pointer dereferencing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The HB face caching mechanism introduced in 227e9a40cfeb7e00658cd3 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 Reviewed-by: Michael Krasnyk Reviewed-by: Lars Knoll Reviewed-by: Allan Sandfeld Jensen --- src/gui/text/qfontengine.cpp | 13 ++++++++++++- src/gui/text/qfontengine_ft.cpp | 29 +++++++++++++++++++---------- src/gui/text/qfontengine_p.h | 5 +++++ src/gui/text/qharfbuzzng.cpp | 22 +++++++++++++++------- 4 files changed, 51 insertions(+), 18 deletions(-) diff --git a/src/gui/text/qfontengine.cpp b/src/gui/text/qfontengine.cpp index 303c85ce75..9eea2e786f 100644 --- 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 *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 index 42cf15ee3b..05bd014bd7 100644 --- 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 index c181d61d73..a04f4bd0ac 100644 --- 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 @@ public: 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 index b4ab5856df..d2e7df9c10 100644 --- 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(buffer), &length))) + if (Q_UNLIKELY(!get_font_table(data->user_data, tag, reinterpret_cast(buffer), &length))) length = 0; return hb_blob_create(const_cast(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; -- cgit v1.2.3