[HarfBuzz] Use of uninitialised in Harfbuzz

[Behdad Esfahbod]

Hi Adam,

I'm not familiar with that codebase.  If someone who knows the code can verify 
that the fixes are correct I'd be more than happy to commit.

Cheers,
behdad

On 08/07/2009 05:41 PM, Adam Langley wrote:
> 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
> _______________________________________________
> HarfBuzz mailing list
> HarfBuzz at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/harfbuzz
>