Skip to content

Commit 4079702

Browse files
Jeff BrownAndroid (Google) Code Review
authored andcommitted
Merge "Fix memory corruption issues in TextLayoutCache. (DO NOT MERGE)" into ics-mr1
2 parents 17f9fe1 + f98c2a6 commit 4079702

File tree

1 file changed

+18
-5
lines changed

1 file changed

+18
-5
lines changed

core/jni/android/graphics/TextLayoutCache.cpp

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -368,17 +368,13 @@ void TextLayoutCacheValue::initShaperItem(HB_ShaperItem& shaperItem, HB_FontRec*
368368
// padding and fallback if we find that we are wrong.
369369
createGlyphArrays(shaperItem, (contextCount + 2) * 2);
370370

371-
// Create log clusters array
372-
shaperItem.log_clusters = new unsigned short[contextCount];
373-
374371
// Set the string properties
375372
shaperItem.string = chars;
376373
shaperItem.stringLength = contextCount;
377374
}
378375

379376
void TextLayoutCacheValue::freeShaperItem(HB_ShaperItem& shaperItem) {
380377
deleteGlyphArrays(shaperItem);
381-
delete[] shaperItem.log_clusters;
382378
HB_FreeFace(shaperItem.face);
383379
}
384380

@@ -392,6 +388,7 @@ void TextLayoutCacheValue::shapeRun(HB_ShaperItem& shaperItem, size_t start, siz
392388
shaperItem.item.script = isRTL ? HB_Script_Arabic : HB_Script_Common;
393389

394390
// Shape
391+
assert(shaperItem.item.length > 0); // Harfbuzz will overwrite other memory if length is 0.
395392
while (!HB_ShapeItem(&shaperItem)) {
396393
// We overflowed our arrays. Resize and retry.
397394
// HB_ShapeItem fills in shaperItem.num_glyphs with the needed size.
@@ -404,6 +401,10 @@ void TextLayoutCacheValue::computeValuesWithHarfbuzz(SkPaint* paint, const UChar
404401
size_t start, size_t count, size_t contextCount, int dirFlags,
405402
Vector<jfloat>* const outAdvances, jfloat* outTotalAdvance,
406403
Vector<jchar>* const outGlyphs) {
404+
if (!count) {
405+
*outTotalAdvance = 0;
406+
return;
407+
}
407408

408409
UBiDiLevel bidiReq = 0;
409410
bool forceLTR = false;
@@ -544,6 +545,10 @@ void TextLayoutCacheValue::computeRunValuesWithHarfbuzz(HB_ShaperItem& shaperIte
544545
size_t start, size_t count, bool isRTL,
545546
Vector<jfloat>* const outAdvances, jfloat* outTotalAdvance,
546547
Vector<jchar>* const outGlyphs) {
548+
if (!count) {
549+
*outTotalAdvance = 0;
550+
return;
551+
}
547552

548553
shapeRun(shaperItem, start, count, isRTL);
549554

@@ -607,14 +612,22 @@ void TextLayoutCacheValue::deleteGlyphArrays(HB_ShaperItem& shaperItem) {
607612
delete[] shaperItem.attributes;
608613
delete[] shaperItem.advances;
609614
delete[] shaperItem.offsets;
615+
delete[] shaperItem.log_clusters;
610616
}
611617

612618
void TextLayoutCacheValue::createGlyphArrays(HB_ShaperItem& shaperItem, int size) {
619+
shaperItem.num_glyphs = size;
620+
621+
// These arrays are all indexed by glyph
613622
shaperItem.glyphs = new HB_Glyph[size];
614623
shaperItem.attributes = new HB_GlyphAttributes[size];
615624
shaperItem.advances = new HB_Fixed[size];
616625
shaperItem.offsets = new HB_FixedPoint[size];
617-
shaperItem.num_glyphs = size;
626+
627+
// Although the log_clusters array is indexed by character, Harfbuzz expects that
628+
// it is big enough to hold one element per glyph. So we allocate log_clusters along
629+
// with the other glyph arrays above.
630+
shaperItem.log_clusters = new unsigned short[size];
618631
}
619632

620633
} // namespace android

0 commit comments

Comments
 (0)