From 5b90a364a9693ed275ba5a99c7ce35f9ea3243b4 Mon Sep 17 00:00:00 2001
From: Kaylee Lubick <kjlubick@google.com>
Date: Wed, 29 Jul 2026 17:34:47 +0000
Subject: [PATCH] Address incorrect handling of a map pointer in SkRP

In pushChildCall, we held on to a pointer from a fChildEffectMap
and then later dereferenced it. However, in between those
points was a code path that could grow the map, invalidating
the pointer. This is demonstrated in the newly added test.

To fix it, we just dereference it earlier. While tracking this
down, I found a suspicious other usage of the map which works
in newer C++, but could break in older versions. It's trivial
to fix Generator::writeFunction, so I handled that as well.

Bug: https://issues.chromium.org/issues/540157141
Fixed: 540157141
Change-Id: I149b070c31d4cfefa65b30972f0b8b94441e66db
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1308776
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Commit-Queue: Kaylee Lubick <kjlubick@google.com>
---
 .../SkSLRasterPipelineCodeGenerator.cpp       | 20 +++++++------
 tests/RasterPipelineCodeGeneratorTest.cpp     | 28 +++++++++++++++++++
 2 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/src/sksl/codegen/SkSLRasterPipelineCodeGenerator.cpp b/src/sksl/codegen/SkSLRasterPipelineCodeGenerator.cpp
index 484a60970a..730d19bc6a 100644
--- a/src/sksl/codegen/SkSLRasterPipelineCodeGenerator.cpp
+++ b/src/sksl/codegen/SkSLRasterPipelineCodeGenerator.cpp
@@ -1422,10 +1422,13 @@ std::optional<SlotRange> Generator::writeFunction(
             // If we are passing a child effect to a function, we need to add its mapping to our
             // child map.
             if (arg.type().isEffectChild()) {
-                if (int* childIndex = fChildEffectMap.find(arg.as<VariableReference>()
-                                                              .variable())) {
+                if (int* childIndexPtr =
+                            fChildEffectMap.find(arg.as<VariableReference>().variable())) {
+                    // In earlier C++ versions, the map assignment could cause the map to be
+                    // resized, invalidating the pointer.
+                    int childIndex = *childIndexPtr;
                     SkASSERT(!fChildEffectMap.find(&param));
-                    fChildEffectMap[&param] = *childIndex;
+                    fChildEffectMap[&param] = childIndex;
                 }
                 continue;
             }
@@ -2809,8 +2812,9 @@ bool Generator::pushConstructorCompound(const AnyConstructor& c) {
 }
 
 bool Generator::pushChildCall(const ChildCall& c) {
-    int* childIdx = fChildEffectMap.find(&c.child());
-    SkASSERT(childIdx != nullptr);
+    int* childIdxPtr = fChildEffectMap.find(&c.child());
+    SkASSERT(childIdxPtr != nullptr);
+    int childIdx = *childIdxPtr;  // Save this in case pushExpression changes fChildEffectMap
     SkASSERT(!c.arguments().empty());
 
     // All child calls have at least one argument.
@@ -2832,7 +2836,7 @@ bool Generator::pushChildCall(const ChildCall& c) {
 
             // Move the argument into src.rgba while also preserving the execution mask.
             fBuilder.exchange_src();
-            fBuilder.invoke_shader(*childIdx);
+            fBuilder.invoke_shader(childIdx);
             break;
         }
         case Type::TypeKind::kColorFilter: {
@@ -2843,7 +2847,7 @@ bool Generator::pushChildCall(const ChildCall& c) {
 
             // Move the argument into src.rgba while also preserving the execution mask.
             fBuilder.exchange_src();
-            fBuilder.invoke_color_filter(*childIdx);
+            fBuilder.invoke_color_filter(childIdx);
             break;
         }
         case Type::TypeKind::kBlender: {
@@ -2861,7 +2865,7 @@ bool Generator::pushChildCall(const ChildCall& c) {
             }
             fBuilder.pop_dst_rgba();
             fBuilder.exchange_src();
-            fBuilder.invoke_blender(*childIdx);
+            fBuilder.invoke_blender(childIdx);
             break;
         }
         default: {
-- 
2.53.0

