Reland "[M120-LTS][SkSL:RP] Prevent overflow when computing slot allocation size"
This reverts commit 7ba220b34a1b39728bcba5f7fd5a8960557f6042.
Reason for revert: removing unit test that fails to compile in m120
since all that's needed is the actual SkSL::RP updates for the LTS
chrome release.
Original change's description:
> Revert "[M120-LTS][SkSL:RP] Prevent overflow when computing slot allocation size"
>
> This reverts commit a59291ec277803d5b231b1a82ab2275e4823bd3f.
>
> Reason for revert: unit test uses code not in m120.
>
> Original change's description:
> > [M120-LTS][SkSL:RP] Prevent overflow when computing slot allocation size
> >
> > Bug: 355465305
> > Change-Id: Ife25289f7b3489701c67b7dc5d30e473019a1193
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/888376
> > Commit-Queue: Brian Osman <brianosman@google.com>
> > (cherry picked from commit d1b243ba90f0698ced6fadc460adb9d66c248946)
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/892676
> > Reviewed-by: Michael Ludwig <michaelludwig@google.com>
>
> Bug: 355465305
> Change-Id: I27fe9fa6d769c84955bbbc2ca01c10305d4349b2
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/899717
> Auto-Submit: Michael Ludwig <michaelludwig@google.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bug: 355465305
Change-Id: Iea2884487313206150f98becbbf22a5a286ff512
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/901140
Reviewed-by: Robert Phillips <robertphillips@google.com>
diff --git a/src/sksl/codegen/SkSLRasterPipelineBuilder.cpp b/src/sksl/codegen/SkSLRasterPipelineBuilder.cpp
index 176533d..8e83402 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"
@@ -1615,13 +1619,17 @@
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);
@@ -1642,8 +1650,11 @@
#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.
@@ -1657,7 +1668,7 @@
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();
@@ -2806,7 +2817,7 @@
// 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 7b103f8..8e879e9 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 @@
SkSpan<float> stack;
SkSpan<float> immutable;
};
- SlotData allocateSlotData(SkArenaAlloc* alloc) const;
+ std::optional<SlotData> allocateSlotData(SkArenaAlloc* alloc) const;
struct Stage {
ProgramOp op;
