fonts: Fix memory allocation for fallback glyphs.

When allocating the mask for a fallback glyph, we allocate it on the
arena on the SkScalerContext while the image belongs to a glyph on a
different cache. This can lead to use-after-free bugs if accessing the
image after the context owning that memory is destroyed. Fix this by
allocating on the arena from the owning cache.

R=herb@google.com, mtklein@google.com

Bug: 829622
Change-Id: Ife53e24f5bc868f36c43f2adcd7a2629ab5577fe
Reviewed-on: https://skia-review.googlesource.com/134182
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>

1 files changed, 46 insertions, 51 deletions

diff --git a/tests/SkRemoteGlyphCacheTest.cpp b/tests/SkRemoteGlyphCacheTest.cpp
index 359c506950..62036d2285 100644
--- a/tests/SkRemoteGlyphCacheTest.cpp
+++ b/tests/SkRemoteGlyphCacheTest.cpp
@@ -46,7 +46,7 @@ public:
     }
     const SkTHashSet<SkDiscardableHandleId>& lockedHandles() const { return fLockedHandles; }
     SkDiscardableHandleId handleCount() { return fNextHandleId; }
-    int cacheMissCount(SkStrikeClient::CacheMissType type) { return fCacheMissCount[type]; }
+    int cacheMissCount(uint32_t type) { return fCacheMissCount[type]; }
     bool hasCacheMiss() const {
         for (uint32_t i = 0; i <= SkStrikeClient::CacheMissType::kLast; ++i) {
             if (fCacheMissCount[i] > 0) return true;
@@ -410,14 +410,22 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkRemoteGlyphCache_CacheMissReporting, report
 }
 
 DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
+    // Build proxy typeface on the client for initializing the cache.
+    sk_sp<DiscardableManager> discardableManager = sk_make_sp<DiscardableManager>();
+    SkStrikeServer server(discardableManager.get());
+    SkStrikeClient client(discardableManager, false);
+
+    auto serverTf = SkTypeface::MakeFromName("monospace", SkFontStyle());
+    auto tfData = server.serializeTypeface(serverTf.get());
+    auto clientTf = client.deserializeTypeface(tfData->data(), tfData->size());
+    REPORTER_ASSERT(reporter, clientTf);
+
     SkPaint paint;
     paint.setAntiAlias(true);
-    auto typeface = SkTypeface::MakeFromName("monospace", SkFontStyle());
-    paint.setTypeface(typeface);
+    paint.setTypeface(clientTf);
     paint.setColor(SK_ColorRED);
 
     auto lostGlyphID = SkPackedGlyphID(1, SK_FixedHalf, SK_FixedHalf);
-
     const uint8_t glyphImage[] = {0xFF, 0xFF};
 
     // Build a fallback cache.
@@ -429,7 +437,7 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
         SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
         auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
 
-        auto fallbackCache = SkStrikeCache::FindOrCreateStrikeExclusive(*desc, effects, *typeface);
+        auto fallbackCache = SkStrikeCache::FindOrCreateStrikeExclusive(*desc, effects, *clientTf);
         auto glyph = fallbackCache->getRawGlyphByID(lostGlyphID);
         glyph->fMaskFormat = SkMask::kA8_Format;
         glyph->fHeight = 1;
@@ -450,67 +458,54 @@ DEF_TEST(SkRemoteGlyphCache_SearchOfDesperation, reporter) {
         REPORTER_ASSERT(reporter, !(testCache == nullptr));
     }
 
-    // Make sure we can't find the target cache.
+    // Create the target cache.
+    SkExclusiveStrikePtr testCache;
+    SkAutoDescriptor ad;
+    SkScalerContextRec rec;
+    SkScalerContextEffects effects;
+    SkScalerContextFlags flags = SkScalerContextFlags::kNone;
+    SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
+    auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
+    testCache = SkStrikeCache::FindStrikeExclusive(*desc);
+    REPORTER_ASSERT(reporter, testCache == nullptr);
+    testCache = SkStrikeCache::CreateStrikeExclusive(*desc,
+                                                     clientTf->createScalerContext(effects, desc));
+    static_cast<SkScalerContextProxy*>(testCache->getScalerContext())->initCache(testCache.get());
+
+    // Look for the lost glyph.
     {
-        SkAutoDescriptor ad;
-        SkScalerContextRec rec;
-        SkScalerContextEffects effects;
-        SkScalerContextFlags flags = SkScalerContextFlags::kNone;
-        SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
-        auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
-        auto testCache = SkStrikeCache::FindStrikeExclusive(*desc);
-        REPORTER_ASSERT(reporter, testCache == nullptr);
-    }
+        const auto& lostGlyph = testCache->getGlyphIDMetrics(
+                lostGlyphID.code(), lostGlyphID.getSubXFixed(), lostGlyphID.getSubYFixed());
+        testCache->findImage(lostGlyph);
 
-    {
-        SkGlyph lostGlyph;
-        lostGlyph.initWithGlyphID(lostGlyphID);
-
-        SkAutoDescriptor ad;
-        SkScalerContextRec rec;
-        SkScalerContextEffects effects;
-        SkScalerContextFlags flags = SkScalerContextFlags::kNone;
-        SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
-        auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
-        SkArenaAlloc alloc{100};
-        auto found = SkStrikeCache::DesperationSearchForImage(*desc, &lostGlyph, &alloc);
-        REPORTER_ASSERT(reporter, found);
         REPORTER_ASSERT(reporter, lostGlyph.fHeight == 1);
         REPORTER_ASSERT(reporter, lostGlyph.fWidth == 2);
         REPORTER_ASSERT(reporter, lostGlyph.fMaskFormat == SkMask::kA8_Format);
+        REPORTER_ASSERT(reporter, memcmp(lostGlyph.fImage, glyphImage, sizeof(glyphImage)) == 0);
     }
 
+    // Look for the lost glyph with a different sub-pix position.
     {
-        SkGlyph lostGlyph;
-        auto reallyLostGlyphID = SkPackedGlyphID(1, SK_FixedQuarter, SK_FixedQuarter);
-        lostGlyph.initWithGlyphID(reallyLostGlyphID);
+        const auto& lostGlyph =
+                testCache->getGlyphIDMetrics(lostGlyphID.code(), SK_FixedQuarter, SK_FixedQuarter);
+        testCache->findImage(lostGlyph);
 
-        SkAutoDescriptor ad;
-        SkScalerContextRec rec;
-        SkScalerContextEffects effects;
-        SkScalerContextFlags flags = SkScalerContextFlags::kNone;
-        SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
-        auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
-        SkArenaAlloc alloc{100};
-        auto found = SkStrikeCache::DesperationSearchForImage(*desc, &lostGlyph, &alloc);
-        REPORTER_ASSERT(reporter, found);
         REPORTER_ASSERT(reporter, lostGlyph.fHeight == 1);
         REPORTER_ASSERT(reporter, lostGlyph.fWidth == 2);
         REPORTER_ASSERT(reporter, lostGlyph.fMaskFormat == SkMask::kA8_Format);
+        REPORTER_ASSERT(reporter, memcmp(lostGlyph.fImage, glyphImage, sizeof(glyphImage)) == 0);
     }
 
-    // Make sure we can't find the target cache again.
-    {
-        SkAutoDescriptor ad;
-        SkScalerContextRec rec;
-        SkScalerContextEffects effects;
-        SkScalerContextFlags flags = SkScalerContextFlags::kNone;
-        SkScalerContext::MakeRecAndEffects(paint, nullptr, nullptr, flags, &rec, &effects, false);
-        auto desc = SkScalerContext::AutoDescriptorGivenRecAndEffects(rec, effects, &ad);
-        auto testCache = SkStrikeCache::FindStrikeExclusive(*desc);
-        REPORTER_ASSERT(reporter, testCache == nullptr);
+    for (uint32_t i = 0; i <= SkStrikeClient::CacheMissType::kLast; ++i) {
+        if (i == SkStrikeClient::CacheMissType::kGlyphMetricsFallback ||
+            i == SkStrikeClient::CacheMissType::kFontMetrics) {
+            REPORTER_ASSERT(reporter, discardableManager->cacheMissCount(i) == 2);
+        } else {
+            REPORTER_ASSERT(reporter, discardableManager->cacheMissCount(i) == 0);
+        }
     }
-
     SkStrikeCache::Validate();
 
+    // Must unlock everything on termination, otherwise valgrind complains about memory leaks.
+    discardableManager->unlockAndDeleteAll();
 }