Skip to content

Commit

Permalink
Bug 1828931 - Cherry-pick SkSL fixes. r=jrmuizel
Browse files Browse the repository at this point in the history
  • Loading branch information
lsalzman committed Apr 20, 2023
1 parent f22e14f commit 45f3218
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 23 deletions.
4 changes: 2 additions & 2 deletions gfx/skia/skia/src/sksl/dsl/DSLType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ DSLExpression DSLType::Construct(DSLType type, SkSpan<DSLExpression> 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);
}
Expand All @@ -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,
Expand Down
43 changes: 26 additions & 17 deletions gfx/skia/skia/src/sksl/ir/SkSLFunctionDefinition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include <cstddef>
#include <forward_list>
#include <string_view>
#include <vector>

namespace SkSL {

Expand Down Expand Up @@ -88,9 +89,29 @@ std::unique_ptr<FunctionDefinition> 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);
Expand All @@ -109,24 +130,12 @@ std::unique_ptr<FunctionDefinition> 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<VarDeclaration>().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;
}
Expand Down Expand Up @@ -219,7 +228,7 @@ std::unique_ptr<FunctionDefinition> 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<Block>());
}
Expand Down
20 changes: 16 additions & 4 deletions gfx/skia/skia/src/sksl/ir/SkSLType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -694,6 +694,17 @@ std::unique_ptr<Type> 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<StructType>(pos, name, std::move(fields), interfaceBlock);
}

Expand Down Expand Up @@ -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<Expression> size) const {
SKSL_INT Type::convertArraySize(const Context& context,
Position arrayPos,
std::unique_ptr<Expression> size) const {
size = context.fTypes.fInt->coerceExpression(std::move(size), context);
if (!size) {
return 0;
Expand All @@ -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<int32_t>(count)) {
if (SkSafeMath::Mul(this->slotCount(), count) > kVariableSlotLimit) {
context.fErrors->error(size->fPosition, "array size is too large");
return 0;
}
Expand Down

0 comments on commit 45f3218

Please sign in to comment.