Fix use-of-uninitialized-value in SkPictureShader::onMakeContext

SkPictureShader::refBitmapShader is expected to always initialize the
scale adjust vector when returning a non-null shader.  But the code path
returning EmptyShader does not do that.

Instead of hauling around a separate scale adjustment, we can refactor
to avoid this problem by adjusting the local matrix directly, if needed,
in refBitmapShader.  The local matrix is conveniently already stored in
a SkTCopyOnFirstWrite.

Bug: chromium:835048, oss-fuzz:7738
Change-Id: I2df3bde7d6237f01bc71857c2fe254e86b186dc0
Reviewed-on: https://skia-review.googlesource.com/122544
Auto-Submit: Florin Malita <fmalita@chromium.org>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>

2 files changed, 12 insertions, 38 deletions

diff --git a/src/shaders/SkPictureShader.cpp b/src/shaders/SkPictureShader.cpp
index bbed6d695a..42f5c3e497 100644
--- a/src/shaders/SkPictureShader.cpp
+++ b/src/shaders/SkPictureShader.cpp
@@ -171,24 +171,15 @@ void SkPictureShader::flatten(SkWriteBuffer& buffer) const {
     fPicture->flatten(buffer);
 }
 
-// This helper returns two artifacts:
-//
-// 1) a cached image shader, which wraps a single picture tile at the given CTM/local matrix
-//
-// 2) a tile scale adjustment, to be applied downstream when dispatching createContext(),
-//    appendStages() and asFragmentProcessor() in callers
-//
-// The composite local matrix includes the actual local matrix, any inherited/outer local matrix
-// and a scale component (to mape the actual tile bitmap size -> fTile size).
-//
+// Returns a cached image shader, which wraps a single picture tile at the given
+// CTM/local matrix.  Also adjusts the local matrix for tile scaling.
 sk_sp<SkShader> SkPictureShader::refBitmapShader(const SkMatrix& viewMatrix,
-                                                 const SkMatrix& localMatrix,
+                                                 SkTCopyOnFirstWrite<SkMatrix>* localMatrix,
                                                  SkColorSpace* dstColorSpace,
-                                                 SkVector* scaleAdjust,
                                                  const int maxTextureSize) const {
     SkASSERT(fPicture && !fPicture->cullRect().isEmpty());
 
-    const SkMatrix m = SkMatrix::Concat(viewMatrix, localMatrix);
+    const SkMatrix m = SkMatrix::Concat(viewMatrix, **localMatrix);
 
     // Use a rotation-invariant scale
     SkPoint scale;
@@ -270,27 +261,24 @@ sk_sp<SkShader> SkPictureShader::refBitmapShader(const SkMatrix& viewMatrix,
         fAddedToCache.store(true);
     }
 
-    scaleAdjust->set(1 / tileScale.width(), 1 / tileScale.height());
+    if (tileScale.width() != 1 || tileScale.height() != 1) {
+        localMatrix->writable()->preScale(1 / tileScale.width(), 1 / tileScale.height());
+    }
 
     return tileShader;
 }
 
 bool SkPictureShader::onAppendStages(const StageRec& rec) const {
     auto lm = this->totalLocalMatrix(rec.fLocalM);
-    SkVector scaleAdjust;
 
     // Keep bitmapShader alive by using alloc instead of stack memory
     auto& bitmapShader = *rec.fAlloc->make<sk_sp<SkShader>>();
-    bitmapShader = this->refBitmapShader(rec.fCTM, *lm, rec.fDstCS, &scaleAdjust);
+    bitmapShader = this->refBitmapShader(rec.fCTM, &lm, rec.fDstCS);
 
     if (!bitmapShader) {
         return false;
     }
 
-    if (scaleAdjust != SkVector::Make(1, 1)) {
-        lm.writable()->preScale(scaleAdjust.fX, scaleAdjust.fY);
-    }
-
     StageRec localRec = rec;
     localRec.fLocalM = lm->isIdentity() ? nullptr : lm.get();
 
@@ -301,19 +289,11 @@ bool SkPictureShader::onAppendStages(const StageRec& rec) const {
 SkShaderBase::Context* SkPictureShader::onMakeContext(const ContextRec& rec, SkArenaAlloc* alloc)
 const {
     auto lm = this->totalLocalMatrix(rec.fLocalMatrix);
-    SkVector scaleAdjust;
-    sk_sp<SkShader> bitmapShader = this->refBitmapShader(*rec.fMatrix,
-                                                         *lm,
-                                                         rec.fDstColorSpace,
-                                                         &scaleAdjust);
+    sk_sp<SkShader> bitmapShader = this->refBitmapShader(*rec.fMatrix, &lm, rec.fDstColorSpace);
     if (!bitmapShader) {
         return nullptr;
     }
 
-    if (scaleAdjust != SkVector::Make(1, 1)) {
-        lm.writable()->preScale(scaleAdjust.fX, scaleAdjust.fY);
-    }
-
     ContextRec localRec = rec;
     localRec.fLocalMatrix = lm->isIdentity() ? nullptr : lm.get();
 
@@ -382,18 +362,13 @@ std::unique_ptr<GrFragmentProcessor> SkPictureShader::asFragmentProcessor(
     }
 
     auto lm = this->totalLocalMatrix(args.fPreLocalMatrix, args.fPostLocalMatrix);
-    SkVector scaleAdjust;
-    sk_sp<SkShader> bitmapShader(this->refBitmapShader(*args.fViewMatrix,*lm,
+    sk_sp<SkShader> bitmapShader(this->refBitmapShader(*args.fViewMatrix, &lm,
                                                        args.fDstColorSpaceInfo->colorSpace(),
-                                                       &scaleAdjust, maxTextureSize));
+                                                       maxTextureSize));
     if (!bitmapShader) {
         return nullptr;
     }
 
-    if (scaleAdjust != SkVector::Make(1, 1)) {
-        lm.writable()->preScale(scaleAdjust.fX, scaleAdjust.fY);
-    }
-
     // We want to *reset* args.fPreLocalMatrix, not compose it.
     GrFPArgs newArgs(args.fContext, args.fViewMatrix, args.fFilterQuality, args.fDstColorSpaceInfo);
     newArgs.fPreLocalMatrix = lm.get();
diff --git a/src/shaders/SkPictureShader.h b/src/shaders/SkPictureShader.h
index 5f28b6074b..6e5201e8fb 100644
--- a/src/shaders/SkPictureShader.h
+++ b/src/shaders/SkPictureShader.h
@@ -46,9 +46,8 @@ private:
     SkPictureShader(sk_sp<SkPicture>, TileMode, TileMode, const SkMatrix*, const SkRect*,
                     sk_sp<SkColorSpace>);
 
-    sk_sp<SkShader> refBitmapShader(const SkMatrix&, const SkMatrix& localMatrix,
+    sk_sp<SkShader> refBitmapShader(const SkMatrix&, SkTCopyOnFirstWrite<SkMatrix>* localMatrix,
                                     SkColorSpace* dstColorSpace,
-                                    SkVector* scaleAdjust,
                                     const int maxTextureSize = 0) const;
 
     class PictureShaderContext : public Context {