From 0c8a88526da1fe107d2e2e1f5fa0e9495b53a4a5 Mon Sep 17 00:00:00 2001
From: Robert Phillips <robertphillips@google.com>
Date: Fri, 3 Jun 2022 09:38:16 -0400
Subject: [PATCH] [graphite] Add debug guards against reusing dead
 SkCombinationOptions

Bug: skia:12701
Change-Id: I4209a4741a957a4dc60ad6669fc164a2d5993676
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545720
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
---
 dm/DMSrcSink.cpp                    | 18 ++++++++++++++++++
 include/core/SkCombinationBuilder.h |  4 ++++
 src/core/SkCombinationBuilder.cpp   | 24 ++++++++++++++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 0e507fa8bb..105189ddb9 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -2245,6 +2245,24 @@ void precompile(skgpu::graphite::Context* context) {
 
         context->precompile(builder);
     }
+
+#ifdef SK_DEBUG
+    // TODO: move this to CombinationBuilderTest
+    builder.reset();
+
+    // Check that epochs are updated upon builder reset
+    {
+        SkCombinationOption solid_0 = builder.addOption(SkShaderType::kSolidColor);
+
+        int optionEpoch = solid_0.epoch();
+        SkASSERT(optionEpoch == builder.epoch());
+
+        builder.reset();
+
+        SkASSERT(optionEpoch != builder.epoch());
+    }
+#endif
+
 }
 
 } // anonymous namespace
diff --git a/include/core/SkCombinationBuilder.h b/include/core/SkCombinationBuilder.h
index be12c4281f..30efaf4f9f 100644
--- a/include/core/SkCombinationBuilder.h
+++ b/include/core/SkCombinationBuilder.h
@@ -97,6 +97,7 @@ public:
     bool isValid() const { return fDataInArena; }
     SkShaderType type() const;
     int numChildSlots() const;
+    SkDEBUGCODE(int epoch() const;)
 
 private:
     friend class SkCombinationBuilder; // for ctor
@@ -143,6 +144,7 @@ public:
 
 #ifdef SK_DEBUG
     void dump() const;
+    int epoch() const { return fEpoch; }
 #endif
 
 private:
@@ -167,6 +169,8 @@ private:
 
     // TODO: store the SkBlender-based blenders in the arena
     SkTHashSet<uint32_t> fBlendModes;
+
+    SkDEBUGCODE(int fEpoch = 0;)
 };
 
 #endif // SkCombinationBuilder_DEFINED
diff --git a/src/core/SkCombinationBuilder.cpp b/src/core/SkCombinationBuilder.cpp
index 11bcf34927..24ab7146fd 100644
--- a/src/core/SkCombinationBuilder.cpp
+++ b/src/core/SkCombinationBuilder.cpp
@@ -113,6 +113,14 @@ public:
     SkShaderType type() const { return fType; }
     int numSlots() const { return fNumSlots; }
 
+#ifdef SK_DEBUG
+    int epoch() const { return fEpoch; }
+    void setEpoch(int epoch) {
+        SkASSERT(fEpoch == kInvalidEpoch && epoch != kInvalidEpoch);
+        fEpoch = epoch;
+    }
+#endif
+
     void setSlotsArray(SkSlot* slots) {
         SkASSERT(!fSlots);
         fSlots = slots;
@@ -170,8 +178,11 @@ public:
 private:
     int numIntrinsicCombinations() const;
 
+    SkDEBUGCODE(static constexpr int kInvalidEpoch = -1;)
+
     const SkShaderType fType;
     const int          fNumSlots;
+    SkDEBUGCODE(int    fEpoch = kInvalidEpoch;)
     SkOption*          fNext = nullptr;
     SkSlot*            fSlots = nullptr;  // an array of 'fNumSlots' SkSlots
 };
@@ -304,6 +315,8 @@ SkCombinationOption SkCombinationOption::addChildOption(int childIndex, SkShader
         return SkCombinationOption(fBuilder, /*dataInArena=*/nullptr);
     }
 
+    SkASSERT(fDataInArena->epoch() == fBuilder->fEpoch);
+
     SkOption* child = fBuilder->addOptionInternal(type);
     if (child) {
         fDataInArena->addOption(childIndex, child);
@@ -318,6 +331,8 @@ SkCombinationOption SkCombinationOption::addChildOption(int childIndex, SkShader
         return SkCombinationOption(fBuilder, /*dataInArena=*/nullptr);
     }
 
+    SkASSERT(fDataInArena->epoch() == fBuilder->fEpoch);
+
     SkOption* child = fBuilder->addOptionInternal(type, minNumStops, maxNumStops);
     if (child) {
         fDataInArena->addOption(childIndex, child);
@@ -333,6 +348,8 @@ SkCombinationOption SkCombinationOption::addChildOption(
         return SkCombinationOption(fBuilder, /*dataInArena=*/nullptr);
     }
 
+    SkASSERT(fDataInArena->epoch() == fBuilder->fEpoch);
+
     SkOption* child = fBuilder->addOptionInternal(type, tileModes);
     if (child) {
         fDataInArena->addOption(childIndex, child);
@@ -343,6 +360,7 @@ SkCombinationOption SkCombinationOption::addChildOption(
 
 SkShaderType SkCombinationOption::type() const { return fDataInArena->type(); }
 int SkCombinationOption::numChildSlots() const { return fDataInArena->numSlots(); }
+SkDEBUGCODE(int SkCombinationOption::epoch() const { return fDataInArena->epoch(); })
 
 //--------------------------------------------------------------------------------------------------
 #ifdef SK_GRAPHITE_ENABLED
@@ -367,6 +385,11 @@ SkOption* SkCombinationBuilder::allocInArena(Args&&... args) {
         return nullptr;
     }
 
+    SkASSERT(arenaObject->type() == T::kType);
+    SkASSERT(arenaObject->numSlots() == T::kNumChildSlots);
+
+    SkDEBUGCODE(arenaObject->setEpoch(fEpoch));
+
     if (T::kNumChildSlots) {
         arenaObject->setSlotsArray(fArena->makeArrayDefault<SkOption::SkSlot>(T::kNumChildSlots));
     }
@@ -490,6 +513,7 @@ void SkCombinationBuilder::reset() {
     fShaderOptions.reset();
     fBlendModes.reset();
     fArena->reset();
+    SkDEBUGCODE(++fEpoch;)
 }
 
 int SkCombinationBuilder::numCombinations() {