From 45f321880f49167c15e384ce90d0e4ee30bce965 Mon Sep 17 00:00:00 2001 From: Lee Salzman Date: Thu, 20 Apr 2023 02:59:26 +0000 Subject: [PATCH] Bug 1828931 - Cherry-pick SkSL fixes. r=jrmuizel Differential Revision: https://phabricator.services.mozilla.com/D175977 --- gfx/skia/skia/src/sksl/dsl/DSLType.cpp | 4 +- .../src/sksl/ir/SkSLFunctionDefinition.cpp | 43 +++++++++++-------- gfx/skia/skia/src/sksl/ir/SkSLType.cpp | 20 +++++++-- 3 files changed, 44 insertions(+), 23 deletions(-) diff --git a/gfx/skia/skia/src/sksl/dsl/DSLType.cpp b/gfx/skia/skia/src/sksl/dsl/DSLType.cpp index 33c38b02ec3f3..82c000e5ab49d 100644 --- a/gfx/skia/skia/src/sksl/dsl/DSLType.cpp +++ b/gfx/skia/skia/src/sksl/dsl/DSLType.cpp @@ -274,7 +274,7 @@ DSLExpression DSLType::Construct(DSLType type, SkSpan argArray) { DSLType Array(const DSLType& base, int count, Position pos) { count = base.skslType().convertArraySize(ThreadContext::Context(), pos, - DSLExpression(count, pos).release()); + DSLExpression(count, pos).release()); if (!count) { return DSLType(kPoison_Type); } @@ -286,7 +286,7 @@ DSLType UnsizedArray(const DSLType& base, Position pos) { return DSLType(kPoison_Type); } return ThreadContext::SymbolTable()->addArrayDimension(&base.skslType(), - SkSL::Type::kUnsizedArray); + SkSL::Type::kUnsizedArray); } DSLType StructType(std::string_view name, diff --git a/gfx/skia/skia/src/sksl/ir/SkSLFunctionDefinition.cpp b/gfx/skia/skia/src/sksl/ir/SkSLFunctionDefinition.cpp index f5ff4f9411438..b33a4352a62a1 100644 --- a/gfx/skia/skia/src/sksl/ir/SkSLFunctionDefinition.cpp +++ b/gfx/skia/skia/src/sksl/ir/SkSLFunctionDefinition.cpp @@ -37,6 +37,7 @@ #include #include #include +#include namespace SkSL { @@ -88,9 +89,29 @@ std::unique_ptr FunctionDefinition::Convert(const Context& c bool builtin) { class Finalizer : public ProgramWriter { public: - Finalizer(const Context& context, const FunctionDeclaration& function) + Finalizer(const Context& context, const FunctionDeclaration& function, Position pos) : fContext(context) - , fFunction(function) {} + , fFunction(function) { + // Function parameters count as local variables. + for (const Variable* var : function.parameters()) { + this->addLocalVariable(var, pos); + } + } + + void addLocalVariable(const Variable* var, Position pos) { + // We count the number of slots used, but don't consider the precision of the base type. + // In practice, this reflects what GPUs actually do pretty well. (i.e., RelaxedPrecision + // math doesn't mean your variable takes less space.) We also don't attempt to reclaim + // slots at the end of a Block. + size_t prevSlotsUsed = fSlotsUsed; + fSlotsUsed = SkSafeMath::Add(fSlotsUsed, var->type().slotCount()); + // To avoid overzealous error reporting, only trigger the error at the first + // place where the stack limit is exceeded. + if (prevSlotsUsed < kVariableSlotLimit && fSlotsUsed >= kVariableSlotLimit) { + fContext.fErrors->error(pos, "variable '" + std::string(var->name()) + + "' exceeds the stack size limit"); + } + } ~Finalizer() override { SkASSERT(fBreakableLevel == 0); @@ -109,24 +130,12 @@ std::unique_ptr FunctionDefinition::Convert(const Context& c bool visitStatement(Statement& stmt) override { switch (stmt.kind()) { case Statement::Kind::kVarDeclaration: { - // We count the number of slots used, but don't consider the precision of the - // base type. In practice, this reflects what GPUs really do pretty well. - // (i.e., RelaxedPrecision math doesn't mean your variable takes less space.) - // We also don't attempt to reclaim slots at the end of a Block. - size_t prevSlotsUsed = fSlotsUsed; const Variable* var = stmt.as().var(); if (var->type().isOrContainsUnsizedArray()) { fContext.fErrors->error(stmt.fPosition, "unsized arrays are not permitted here"); - break; - } - fSlotsUsed = SkSafeMath::Add(fSlotsUsed, var->type().slotCount()); - // To avoid overzealous error reporting, only trigger the error at the first - // place where the stack limit is exceeded. - if (prevSlotsUsed < kVariableSlotLimit && fSlotsUsed >= kVariableSlotLimit) { - fContext.fErrors->error(stmt.fPosition, - "variable '" + std::string(var->name()) + - "' exceeds the stack size limit"); + } else { + this->addLocalVariable(var, stmt.fPosition); } break; } @@ -219,7 +228,7 @@ std::unique_ptr FunctionDefinition::Convert(const Context& c using INHERITED = ProgramWriter; }; - Finalizer(context, function).visitStatement(*body); + Finalizer(context, function, pos).visitStatement(*body); if (function.isMain() && ProgramConfig::IsVertex(context.fConfig->fKind)) { append_rtadjust_fixup_to_vertex_main(context, function, body->as()); } diff --git a/gfx/skia/skia/src/sksl/ir/SkSLType.cpp b/gfx/skia/skia/src/sksl/ir/SkSLType.cpp index a909b5118bd86..265d49dcc8abe 100644 --- a/gfx/skia/skia/src/sksl/ir/SkSLType.cpp +++ b/gfx/skia/skia/src/sksl/ir/SkSLType.cpp @@ -9,9 +9,9 @@ #include "include/private/SkSLLayout.h" #include "include/private/SkSLString.h" -#include "include/private/base/SkTFitsIn.h" #include "include/sksl/SkSLErrorReporter.h" #include "src/base/SkMathPriv.h" +#include "src/base/SkSafeMath.h" #include "src/sksl/SkSLBuiltinTypes.h" #include "src/sksl/SkSLConstantFolder.h" #include "src/sksl/SkSLContext.h" @@ -694,6 +694,17 @@ std::unique_ptr Type::MakeStructType(const Context& context, break; } } + size_t slots = 0; + for (const Field& field : fields) { + if (field.fType->isUnsizedArray()) { + continue; + } + slots = SkSafeMath::Add(slots, field.fType->slotCount()); + if (slots >= kVariableSlotLimit) { + context.fErrors->error(pos, "struct is too large"); + break; + } + } return std::make_unique(pos, name, std::move(fields), interfaceBlock); } @@ -1164,8 +1175,9 @@ bool Type::checkIfUsableInArray(const Context& context, Position arrayPos) const return true; } -SKSL_INT Type::convertArraySize(const Context& context, Position arrayPos, - std::unique_ptr size) const { +SKSL_INT Type::convertArraySize(const Context& context, + Position arrayPos, + std::unique_ptr size) const { size = context.fTypes.fInt->coerceExpression(std::move(size), context); if (!size) { return 0; @@ -1182,7 +1194,7 @@ SKSL_INT Type::convertArraySize(const Context& context, Position arrayPos, context.fErrors->error(size->fPosition, "array size must be positive"); return 0; } - if (!SkTFitsIn(count)) { + if (SkSafeMath::Mul(this->slotCount(), count) > kVariableSlotLimit) { context.fErrors->error(size->fPosition, "array size is too large"); return 0; }