From aaeedc768c1e2110d1f650a364dcf004ec7bdf86 Mon Sep 17 00:00:00 2001 From: Pedro Pontes Date: Fri, 18 Oct 2024 08:21:50 -0700 Subject: [PATCH] chore: backport 2 changes from 2-M127 to 0-M129 (#44312) --- patches/config.json | 3 +- patches/skia/.patches | 1 + ..._when_computing_slot_allocation_size.patch | 198 ++++++++++++++++++ patches/v8/.patches | 1 + ...ix_non-materialized_receiver_closure.patch | 41 ++++ 5 files changed, 243 insertions(+), 1 deletion(-) create mode 100644 patches/skia/.patches create mode 100644 patches/skia/sksl_rp_prevent_overflow_when_computing_slot_allocation_size.patch create mode 100644 patches/v8/merged_maglev_fix_non-materialized_receiver_closure.patch diff --git a/patches/config.json b/patches/config.json index 8cf28d8d2be1..2f4813860fbb 100644 --- a/patches/config.json +++ b/patches/config.json @@ -11,5 +11,6 @@ { "patch_dir": "src/electron/patches/Mantle", "repo": "src/third_party/squirrel.mac/vendor/Mantle" }, { "patch_dir": "src/electron/patches/ReactiveObjC", "repo": "src/third_party/squirrel.mac/vendor/ReactiveObjC" }, { "patch_dir": "src/electron/patches/webrtc", "repo": "src/third_party/webrtc" }, - { "patch_dir": "src/electron/patches/reclient-configs", "repo": "src/third_party/engflow-reclient-configs" } + { "patch_dir": "src/electron/patches/reclient-configs", "repo": "src/third_party/engflow-reclient-configs" }, + { "patch_dir": "src/electron/patches/skia", "repo": "src/third_party/skia" } ] diff --git a/patches/skia/.patches b/patches/skia/.patches new file mode 100644 index 000000000000..35c07619677b --- /dev/null +++ b/patches/skia/.patches @@ -0,0 +1 @@ +sksl_rp_prevent_overflow_when_computing_slot_allocation_size.patch diff --git a/patches/skia/sksl_rp_prevent_overflow_when_computing_slot_allocation_size.patch b/patches/skia/sksl_rp_prevent_overflow_when_computing_slot_allocation_size.patch new file mode 100644 index 000000000000..976f35e2aacd --- /dev/null +++ b/patches/skia/sksl_rp_prevent_overflow_when_computing_slot_allocation_size.patch @@ -0,0 +1,198 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Brian Osman +Date: Fri, 9 Aug 2024 14:50:21 -0400 +Subject: Prevent overflow when computing slot allocation size + +Bug: 355465305 +Change-Id: Ife25289f7b3489701c67b7dc5d30e473019a1193 +Reviewed-on: https://skia-review.googlesource.com/c/skia/+/888376 +Reviewed-by: Julia Lavrova +Commit-Queue: Brian Osman + +diff --git a/src/sksl/codegen/SkSLRasterPipelineBuilder.cpp b/src/sksl/codegen/SkSLRasterPipelineBuilder.cpp +index 14790df1944b418c2471f652e78f8706cb58f1ec..fd721640e1f10eb0fcf04a74d6c6d61fdd13249b 100644 +--- a/src/sksl/codegen/SkSLRasterPipelineBuilder.cpp ++++ b/src/sksl/codegen/SkSLRasterPipelineBuilder.cpp +@@ -6,11 +6,15 @@ + */ + + #include "src/sksl/codegen/SkSLRasterPipelineBuilder.h" ++#include ++#include + + #include "include/core/SkStream.h" + #include "include/private/base/SkMalloc.h" ++#include "include/private/base/SkTFitsIn.h" + #include "include/private/base/SkTo.h" + #include "src/base/SkArenaAlloc.h" ++#include "src/base/SkSafeMath.h" + #include "src/core/SkOpts.h" + #include "src/core/SkRasterPipelineContextUtils.h" + #include "src/core/SkRasterPipelineOpContexts.h" +@@ -1664,13 +1668,17 @@ static void* context_bit_pun(intptr_t val) { + return sk_bit_cast(val); + } + +-Program::SlotData Program::allocateSlotData(SkArenaAlloc* alloc) const { ++std::optional Program::allocateSlotData(SkArenaAlloc* alloc) const { + // Allocate a contiguous slab of slot data for immutables, values, and stack entries. + const int N = SkOpts::raster_pipeline_highp_stride; + const int scalarWidth = 1 * sizeof(float); + const int vectorWidth = N * sizeof(float); +- const int allocSize = vectorWidth * (fNumValueSlots + fNumTempStackSlots) + +- scalarWidth * fNumImmutableSlots; ++ SkSafeMath safe; ++ size_t allocSize = safe.add(safe.mul(vectorWidth, safe.add(fNumValueSlots, fNumTempStackSlots)), ++ safe.mul(scalarWidth, fNumImmutableSlots)); ++ if (!safe || !SkTFitsIn(allocSize)) { ++ return std::nullopt; ++ } + float* slotPtr = static_cast(alloc->makeBytesAlignedTo(allocSize, vectorWidth)); + sk_bzero(slotPtr, allocSize); + +@@ -1691,8 +1699,11 @@ bool Program::appendStages(SkRasterPipeline* pipeline, + #else + // Convert our Instruction list to an array of ProgramOps. + TArray stages; +- SlotData slotData = this->allocateSlotData(alloc); +- this->makeStages(&stages, alloc, uniforms, slotData); ++ std::optional slotData = this->allocateSlotData(alloc); ++ if (!slotData) { ++ return false; ++ } ++ this->makeStages(&stages, alloc, uniforms, *slotData); + + // Allocate buffers for branch targets and labels; these are needed to convert labels into + // actual offsets into the pipeline and fix up branches. +@@ -1706,7 +1717,7 @@ bool Program::appendStages(SkRasterPipeline* pipeline, + auto resetBasePointer = [&]() { + // Whenever we hand off control to another shader, we have to assume that it might overwrite + // the base pointer (if it uses SkSL, it will!), so we reset it on return. +- pipeline->append(SkRasterPipelineOp::set_base_pointer, slotData.values.data()); ++ pipeline->append(SkRasterPipelineOp::set_base_pointer, (*slotData).values.data()); + }; + + resetBasePointer(); +@@ -2896,7 +2907,7 @@ void Program::Dumper::dump(SkWStream* out, bool writeInstructionCount) { + // executed. The program requires pointer ranges for managing its data, and ASAN will report + // errors if those pointers are pointing at unallocated memory. + SkArenaAlloc alloc(/*firstHeapAllocation=*/1000); +- fSlots = fProgram.allocateSlotData(&alloc); ++ fSlots = fProgram.allocateSlotData(&alloc).value(); + float* uniformPtr = alloc.makeArray(fProgram.fNumUniformSlots); + fUniforms = SkSpan(uniformPtr, fProgram.fNumUniformSlots); + +diff --git a/src/sksl/codegen/SkSLRasterPipelineBuilder.h b/src/sksl/codegen/SkSLRasterPipelineBuilder.h +index 4523fc7b9af8514dbbae026680558760424f7591..b0e32ca4f885aa069bc116fa4519e248aa3368f4 100644 +--- a/src/sksl/codegen/SkSLRasterPipelineBuilder.h ++++ b/src/sksl/codegen/SkSLRasterPipelineBuilder.h +@@ -19,6 +19,7 @@ + #include + #include + #include ++#include + + class SkArenaAlloc; + class SkRasterPipeline; +@@ -176,7 +177,7 @@ private: + SkSpan stack; + SkSpan immutable; + }; +- SlotData allocateSlotData(SkArenaAlloc* alloc) const; ++ std::optional allocateSlotData(SkArenaAlloc* alloc) const; + + struct Stage { + ProgramOp op; +diff --git a/tests/RasterPipelineCodeGeneratorTest.cpp b/tests/RasterPipelineCodeGeneratorTest.cpp +index 9da6e61a36fa59fc6ecf067d5643cc839d0e254f..24903c787414558864b4b9e1ef2c90c24f95f436 100644 +--- a/tests/RasterPipelineCodeGeneratorTest.cpp ++++ b/tests/RasterPipelineCodeGeneratorTest.cpp +@@ -22,6 +22,7 @@ + + #include + #include ++#include + #include + + //#define DUMP_PROGRAMS 1 +@@ -250,3 +251,80 @@ DEF_TEST(SkSLRasterPipelineCodeGeneratorComparisonIntrinsicTest, r) { + /*startingColor=*/SkColor4f{0.0, 0.0, 0.0, 0.0}, + /*expectedResult=*/SkColor4f{0.0, 1.0, 0.0, 1.0}); + } ++ ++DEF_TEST(SkSLRasterPipelineSlotOverflow_355465305, r) { ++ constexpr int kStructMembers1 = 6200; ++ constexpr int kStructMembers2 = 433; ++ std::stringstream str; ++ str << "struct M { float4x4 m"; ++ for (int i = 1; i < kStructMembers1; ++i) { ++ str << ",m" << i; ++ } ++ str << ";};"; ++ str << "struct M2 { float4x4 m"; ++ for (int i = 1; i < kStructMembers2; ++i) { ++ str << ",m" << i; ++ } ++ str << ";};"; ++ str << "M f() { M m; return m; }"; ++ constexpr int kConstMembers = 40; ++ str << "struct T { float4x4 m0"; ++ for (int i = 1; i < kConstMembers; ++i) { ++ str << ",m" << i; ++ } ++ str << ";};"; ++ str << "const T K = T("; ++ for (int i = 0; i < kConstMembers; ++i) { ++ if (i > 0) { ++ str << ","; ++ } ++ str << "mat4x4(1337)"; ++ } ++ str << ");"; ++ str << "half4 main(half4 color) {"; ++ str << "float4x4 a = M2("; ++ for (int j = 0; j < kStructMembers2; ++j) { ++ if (j > 0) { ++ str << ","; ++ } ++ const int numAddOps = (j == kStructMembers1 - 1) ? 23 : 25; ++ for (int i = 0; i < numAddOps; ++i) { ++ if (i > 0) { ++ str << "+"; ++ } ++ str << "f().m"; ++ } ++ } ++ str << ").m;"; ++ str << "return half4(a[0]+(K.m0+K.m1+K.m2+K.m3)[0]);"; ++ str << "}"; ++ std::string src = str.str(); ++ ++ SkSL::Compiler compiler; ++ std::unique_ptr program = ++ compiler.convertProgram(SkSL::ProgramKind::kRuntimeColorFilter, src, {}); ++ if (!program) { ++ ERRORF(r, "Unexpected error compiling %s\n%s", src.c_str(), compiler.errorText().c_str()); ++ return; ++ } ++ const SkSL::FunctionDeclaration* main = program->getFunction("main"); ++ if (!main) { ++ ERRORF(r, "Program must have a 'main' function"); ++ return; ++ } ++ SkArenaAlloc alloc(1000); ++ SkRasterPipeline pipeline(&alloc); ++ pipeline.appendConstantColor(&alloc, SkColors::kWhite); ++ std::unique_ptr rasterProg = ++ SkSL::MakeRasterPipelineProgram(*program, *main->definition()); ++ // Ideally, this program would fail in the front-end, because of the number of slots needed ++ // for expression evaluation. For now, it succeeds (but then fails in appendStages). ++ if (!rasterProg) { ++ ERRORF(r, "MakeRasterPipelineProgram failed"); ++ return; ++ } ++ ++ // Append the SkSL program to the raster pipeline. ++ bool success = rasterProg->appendStages(&pipeline, &alloc, /*callbacks=*/nullptr, {}); ++ REPORTER_ASSERT(r, !success, "appendStages should fail for very large program"); ++} diff --git a/patches/v8/.patches b/patches/v8/.patches index 20004f0cd48c..ea335cd67379 100644 --- a/patches/v8/.patches +++ b/patches/v8/.patches @@ -1,3 +1,4 @@ chore_allow_customizing_microtask_policy_per_context.patch deps_add_v8_object_setinternalfieldfornodecore.patch cherry-pick-81155a8f3b20.patch +merged_maglev_fix_non-materialized_receiver_closure.patch diff --git a/patches/v8/merged_maglev_fix_non-materialized_receiver_closure.patch b/patches/v8/merged_maglev_fix_non-materialized_receiver_closure.patch new file mode 100644 index 000000000000..7459338eadb7 --- /dev/null +++ b/patches/v8/merged_maglev_fix_non-materialized_receiver_closure.patch @@ -0,0 +1,41 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Leszek Swirski +Date: Mon, 23 Sep 2024 13:23:59 +0200 +Subject: Merged: [maglev] Fix non-materialized receiver & closure + +Stack walks expect the receiver and closure to be materialized. + +Bug: 368311899 + +(cherry picked from commit 6b455eb2c448348b940728241c799c5d7b508c51) + +Change-Id: Ib5657712dd49fca6c92d881967228e74a5705a9f +Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5893176 +Reviewed-by: Leszek Swirski +Commit-Queue: Victor Gomes +Commit-Queue: Leszek Swirski +Auto-Submit: Victor Gomes +Cr-Commit-Position: refs/branch-heads/12.9@{#45} +Cr-Branched-From: 64a21d7ad7fca1ddc73a9264132f703f35000b69-refs/heads/12.9.202@{#1} +Cr-Branched-From: da4200b2cfe6eb1ad73c457ed27cf5b7ff32614f-refs/heads/main@{#95679} + +diff --git a/src/maglev/maglev-graph-builder.cc b/src/maglev/maglev-graph-builder.cc +index e281e328a8d983a18226ad46b7d17f26ecb964dc..64fc99d3ee8746bcb6403cacd5e86719d45eab07 100644 +--- a/src/maglev/maglev-graph-builder.cc ++++ b/src/maglev/maglev-graph-builder.cc +@@ -1347,7 +1347,14 @@ DeoptFrame MaglevGraphBuilder::GetDeoptFrameForLazyDeoptHelper( + if (result_size == 0 || + !base::IsInRange(reg.index(), result_location.index(), + result_location.index() + result_size - 1)) { +- AddDeoptUse(node); ++ // Receiver and closure values have to be materialized, even if ++ // they don't otherwise escape. ++ if (reg == interpreter::Register::receiver() || ++ reg == interpreter::Register::function_closure()) { ++ node->add_use(); ++ } else { ++ AddDeoptUse(node); ++ } + } + }); + AddDeoptUse(ret.closure());