[HarfBuzz] Use of uninitialised in Harfbuzz

[Adam Langley]

Some recent changes to the WebKit layout tests[1] started triggering
this for us:

==8875== Conditional jump or move depends on uninitialised value(s)
==8875==    at 0x8EE7406: HB_HeuristicPosition
chromium/third_party/harfbuzz/src/harfbuzz-shaper.cpp:417
==8875==    by 0x8EECCA2: HB_HebrewShape
chromium/third_party/harfbuzz/src/harfbuzz-hebrew.c:183
==8875==    by 0x8EE6D66: HB_ShapeItem
chromium/third_party/harfbuzz/src/harfbuzz-shaper.cpp:1308
==8875==    by 0x88CBE11: WebCore::TextRunWalker::shapeGlyphs()
chromium/third_party/WebKit/WebCore/platform/graphics/chromium/FontLinux.cpp:352
==8875==    by 0x88CBF27: WebCore::TextRunWalker::nextScriptRun()
chromium/third_party/WebKit/WebCore/platform/graphics/chromium/FontLinux.cpp:218
==8875==    by 0x88CBFCC: WebCore::TextRunWalker::widthOfFullRun()
chromium/third_party/WebKit/WebCore/platform/graphics/chromium/FontLinux.cpp:279

Below is the patch that I wrote that appears to fix the problem. I've
landed it for Chromium already because I wanted to fix the
intermittent test failures, but I really have no idea when I'm doing
here!

I believe the changes to the assertions are correct. I believe that
item->num_glyphs at this point contains the number of output elements
in the various arrays and therefore should be >= length, but again,
not at all sure.


Cheers

AGL

diff --git a/third_party/harfbuzz/src/harfbuzz-hebrew.c
b/third_party/harfbuzz/src/harfbuzz-hebrew.c
index 533a063..2bda386 100644
--- a/third_party/harfbuzz/src/harfbuzz-hebrew.c
+++ b/third_party/harfbuzz/src/harfbuzz-hebrew.c
@@ -56,6 +56,8 @@ HB_Bool HB_HebrewShape(HB_ShaperItem *shaper_item)

     assert(shaper_item->item.script == HB_Script_Hebrew);

+    HB_HeuristicSetGlyphAttributes(shaper_item);
+
 #ifndef NO_OPENTYPE
     if (HB_SelectScript(shaper_item, hebrew_features)) {

@@ -64,7 +66,6 @@ HB_Bool HB_HebrewShape(HB_ShaperItem *shaper_item)
             return FALSE;


-        HB_HeuristicSetGlyphAttributes(shaper_item);
         HB_OpenTypeShape(shaper_item, /*properties*/0);
         return HB_OpenTypePosition(shaper_item, availableGlyphs,
/*doLogClusters*/TRUE);
     }
diff --git a/third_party/harfbuzz/src/harfbuzz-shaper.cpp
b/third_party/harfbuzz/src/harfbuzz-shaper.cpp
index 36b9282..3628c88 100644
--- a/third_party/harfbuzz/src/harfbuzz-shaper.cpp
+++ b/third_party/harfbuzz/src/harfbuzz-shaper.cpp
@@ -433,7 +433,7 @@ void HB_HeuristicSetGlyphAttributes(HB_ShaperItem *item)

     // ### zeroWidth and justification are missing here!!!!!

-    assert(item->num_glyphs <= length);
+    assert(length <= item->num_glyphs);

 //     qDebug("QScriptEngine::heuristicSetGlyphAttributes,
num_glyphs=%d", item->num_glyphs);
     HB_GlyphAttributes *attributes = item->attributes;
@@ -451,7 +451,6 @@ void HB_HeuristicSetGlyphAttributes(HB_ShaperItem *item)
         }
         ++glyph_pos;
     }
-    assert(glyph_pos == item->num_glyphs);

     // first char in a run is never (treated as) a mark
     int cStart = 0;


[1] http://webkit.org/quality/testwriting.html