Disallow global variables containing arrays of opaque types.
GLSL only supports arrays of samplers in very limited ways; they aren't
supported at all by SkSL. We now detect arrays of opaque objects and
reject the code.
We have several paths through the IR generator that create and process
array types; the unit test covers global and local variables, and array
on the type versus array on the variable.
Change-Id: I5b45e88e31cf4005723c3bf35561622d65321f7b
Bug: skia:11008
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339317
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 8a3fbd5..f71d9ca 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -375,35 +375,42 @@
ExpressionArray sizes;
sizes.reserve_back(varData.fSizeCount);
auto iter = varDecl.begin();
- for (size_t i = 0; i < varData.fSizeCount; ++i, ++iter) {
- const ASTNode& rawSize = *iter;
- if (rawSize) {
- auto size = this->coerce(this->convertExpression(rawSize), *fContext.fInt_Type);
- if (!size) {
+ if (iter != varDecl.end()) {
+ if (type->isOpaque()) {
+ fErrors.error(type->fOffset,
+ "opaque type '" + type->name() + "' may not be used in an array");
+ }
+ for (size_t i = 0; i < varData.fSizeCount; ++i, ++iter) {
+ const ASTNode& rawSize = *iter;
+ if (rawSize) {
+ auto size = this->coerce(this->convertExpression(rawSize), *fContext.fInt_Type);
+ if (!size) {
+ return {};
+ }
+ String name(type->name());
+ int64_t count;
+ if (!size->is<IntLiteral>()) {
+ fErrors.error(size->fOffset, "array size must be an integer");
+ return {};
+ }
+ count = size->as<IntLiteral>().value();
+ if (count <= 0) {
+ fErrors.error(size->fOffset, "array size must be positive");
+ return {};
+ }
+ name += "[" + to_string(count) + "]";
+ type = fSymbolTable->takeOwnershipOfSymbol(
+ std::make_unique<Type>(name, Type::TypeKind::kArray, *type, (int)count));
+ sizes.push_back(std::move(size));
+ } else if (i == 0) {
+ type = fSymbolTable->takeOwnershipOfSymbol(
+ std::make_unique<Type>(type->name() + "[]", Type::TypeKind::kArray,
+ *type, Type::kUnsizedArray));
+ sizes.push_back(nullptr);
+ } else {
+ fErrors.error(varDecl.fOffset, "array size must be specified");
return {};
}
- String name(type->name());
- int64_t count;
- if (!size->is<IntLiteral>()) {
- fErrors.error(size->fOffset, "array size must be an integer");
- return {};
- }
- count = size->as<IntLiteral>().value();
- if (count <= 0) {
- fErrors.error(size->fOffset, "array size must be positive");
- return {};
- }
- name += "[" + to_string(count) + "]";
- type = fSymbolTable->takeOwnershipOfSymbol(
- std::make_unique<Type>(name, Type::TypeKind::kArray, *type, (int)count));
- sizes.push_back(std::move(size));
- } else if (i == 0) {
- type = fSymbolTable->takeOwnershipOfSymbol(std::make_unique<Type>(
- type->name() + "[]", Type::TypeKind::kArray, *type, Type::kUnsizedArray));
- sizes.push_back(nullptr);
- } else {
- fErrors.error(varDecl.fOffset, "array size must be specified");
- return {};
}
}
auto var = std::make_unique<Variable>(varDecl.fOffset, fModifiers->addToPool(modifiers),
@@ -1218,6 +1225,14 @@
}
}
+void IRGenerator::convertGlobalVarDeclarations(const ASTNode& decl) {
+ StatementArray decls = this->convertVarDeclarations(decl, Variable::Storage::kGlobal);
+ for (std::unique_ptr<Statement>& stmt : decls) {
+ fProgramElements->push_back(std::make_unique<GlobalVarDeclaration>(decl.fOffset,
+ std::move(stmt)));
+ }
+}
+
void IRGenerator::convertEnum(const ASTNode& e) {
if (fKind == Program::kPipelineStage_Kind) {
fErrors.error(e.fOffset, "enum is not allowed here");
@@ -1283,49 +1298,55 @@
const Type* IRGenerator::convertType(const ASTNode& type, bool allowVoid) {
ASTNode::TypeData td = type.getTypeData();
const Symbol* result = (*fSymbolTable)[td.fName];
- if (result && result->is<Type>()) {
- if (td.fIsNullable) {
- if (result->as<Type>() == *fContext.fFragmentProcessor_Type) {
- if (type.begin() != type.end()) {
- fErrors.error(type.fOffset, "type '" + td.fName + "' may not be used in "
- "an array");
- }
- result = fSymbolTable->takeOwnershipOfSymbol(std::make_unique<Type>(
- String(result->name()) + "?", Type::TypeKind::kNullable,
- result->as<Type>()));
- } else {
- fErrors.error(type.fOffset, "type '" + td.fName + "' may not be nullable");
- }
- }
- if (result->as<Type>() == *fContext.fVoid_Type) {
- if (!allowVoid) {
- fErrors.error(type.fOffset, "type '" + td.fName + "' not allowed in this context");
- return nullptr;
- }
- if (type.begin() != type.end()) {
- fErrors.error(type.fOffset, "type '" + td.fName + "' may not be used in an array");
- return nullptr;
- }
- }
- if (!fIsBuiltinCode && this->typeContainsPrivateFields(result->as<Type>())) {
- fErrors.error(type.fOffset, "type '" + td.fName + "' is private");
- return {};
- }
- for (const auto& size : type) {
- String name(result->name());
- name += "[";
- if (size) {
- name += to_string(size.getInt());
- }
- name += "]";
- result = fSymbolTable->takeOwnershipOfSymbol(
- std::make_unique<Type>(name, Type::TypeKind::kArray, result->as<Type>(),
- size ? size.getInt() : Type::kUnsizedArray));
- }
- return &result->as<Type>();
+ if (!result || !result->is<Type>()) {
+ fErrors.error(type.fOffset, "unknown type '" + td.fName + "'");
+ return nullptr;
}
- fErrors.error(type.fOffset, "unknown type '" + td.fName + "'");
- return nullptr;
+ const bool isArray = (type.begin() != type.end());
+ if (td.fIsNullable) {
+ if (result->as<Type>() == *fContext.fFragmentProcessor_Type) {
+ if (isArray) {
+ fErrors.error(type.fOffset, "type '" + td.fName + "' may not be used in "
+ "an array");
+ }
+ result = fSymbolTable->takeOwnershipOfSymbol(std::make_unique<Type>(
+ String(result->name()) + "?", Type::TypeKind::kNullable,
+ result->as<Type>()));
+ } else {
+ fErrors.error(type.fOffset, "type '" + td.fName + "' may not be nullable");
+ }
+ }
+ if (result->as<Type>() == *fContext.fVoid_Type) {
+ if (!allowVoid) {
+ fErrors.error(type.fOffset, "type '" + td.fName + "' not allowed in this context");
+ return nullptr;
+ }
+ if (isArray) {
+ fErrors.error(type.fOffset, "type '" + td.fName + "' may not be used in an array");
+ return nullptr;
+ }
+ }
+ if (!fIsBuiltinCode && this->typeContainsPrivateFields(result->as<Type>())) {
+ fErrors.error(type.fOffset, "type '" + td.fName + "' is private");
+ return nullptr;
+ }
+ if (isArray && result->as<Type>().isOpaque()) {
+ fErrors.error(type.fOffset,
+ "opaque type '" + td.fName + "' may not be used in an array");
+ return nullptr;
+ }
+ for (const auto& size : type) {
+ String name(result->name());
+ name += "[";
+ if (size) {
+ name += to_string(size.getInt());
+ }
+ name += "]";
+ result = fSymbolTable->takeOwnershipOfSymbol(
+ std::make_unique<Type>(name, Type::TypeKind::kArray, result->as<Type>(),
+ size ? size.getInt() : Type::kUnsizedArray));
+ }
+ return &result->as<Type>();
}
std::unique_ptr<Expression> IRGenerator::convertExpression(const ASTNode& expr) {
@@ -2984,23 +3005,18 @@
SkASSERT(fFile);
for (const auto& decl : fFile->root()) {
switch (decl.fKind) {
- case ASTNode::Kind::kVarDeclarations: {
- StatementArray decls = this->convertVarDeclarations(decl,
- Variable::Storage::kGlobal);
- for (auto& varDecl : decls) {
- fProgramElements->push_back(std::make_unique<GlobalVarDeclaration>(
- decl.fOffset, std::move(varDecl)));
- }
+ case ASTNode::Kind::kVarDeclarations:
+ this->convertGlobalVarDeclarations(decl);
break;
- }
- case ASTNode::Kind::kEnum: {
+
+ case ASTNode::Kind::kEnum:
this->convertEnum(decl);
break;
- }
- case ASTNode::Kind::kFunction: {
+
+ case ASTNode::Kind::kFunction:
this->convertFunction(decl);
break;
- }
+
case ASTNode::Kind::kModifiers: {
std::unique_ptr<ModifiersDeclaration> f = this->convertModifiersDeclaration(decl);
if (f) {