chore: backport 2 changes from 2-M127 to 0-M129 (#44312)

This commit is contained in:
Pedro Pontes 2024-10-18 08:21:50 -07:00 committed by GitHub
parent d5d936d078
commit aaeedc768c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 243 additions and 1 deletions

View file

@ -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" }
]

1
patches/skia/.patches Normal file
View file

@ -0,0 +1 @@
sksl_rp_prevent_overflow_when_computing_slot_allocation_size.patch

View file

@ -0,0 +1,198 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Brian Osman <brianosman@google.com>
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 <jlavrova@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
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 <cstdint>
+#include <optional>
#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<void*>(val);
}
-Program::SlotData Program::allocateSlotData(SkArenaAlloc* alloc) const {
+std::optional<Program::SlotData> 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<int>(allocSize)) {
+ return std::nullopt;
+ }
float* slotPtr = static_cast<float*>(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<Stage> stages;
- SlotData slotData = this->allocateSlotData(alloc);
- this->makeStages(&stages, alloc, uniforms, slotData);
+ std::optional<SlotData> 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<float>(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 <cstddef>
#include <cstdint>
#include <memory>
+#include <optional>
class SkArenaAlloc;
class SkRasterPipeline;
@@ -176,7 +177,7 @@ private:
SkSpan<float> stack;
SkSpan<float> immutable;
};
- SlotData allocateSlotData(SkArenaAlloc* alloc) const;
+ std::optional<SlotData> 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 <memory>
#include <optional>
+#include <sstream>
#include <string>
//#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<SkSL::Program> 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<SkSL::RP::Program> 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");
+}

View file

@ -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

View file

@ -0,0 +1,41 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Leszek Swirski <leszeks@chromium.org>
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 <leszeks@chromium.org>
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Victor Gomes <victorgomes@chromium.org>
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());