Fixed fuzzer-discovered bug with interface blocks
While I was in this code, I realized that the setVariable method of
InterfaceBlock was unused and there was therefore no reason to be
storing a pointer instead of a reference.
Bug: oss-fuzz:39000
Change-Id: If7505ba87f4060370cfd32ca2e30c76648965101
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450446
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
diff --git a/gn/sksl_tests.gni b/gn/sksl_tests.gni
index 5a18df9..6258e18 100644
--- a/gn/sksl_tests.gni
+++ b/gn/sksl_tests.gni
@@ -109,6 +109,7 @@
"/sksl/errors/Ossfuzz38560.sksl",
"/sksl/errors/Ossfuzz38865.sksl",
"/sksl/errors/Ossfuzz38944.sksl",
+ "/sksl/errors/Ossfuzz39000.sksl",
"/sksl/errors/OverflowFloatLiteral.sksl",
"/sksl/errors/OverflowIntLiteral.sksl",
"/sksl/errors/OverflowInt64Literal.sksl",
diff --git a/resources/sksl/errors/Ossfuzz39000.sksl b/resources/sksl/errors/Ossfuzz39000.sksl
new file mode 100644
index 0000000..6a4e505
--- /dev/null
+++ b/resources/sksl/errors/Ossfuzz39000.sksl
@@ -0,0 +1,2 @@
+q { int y; };
+G { int q=_; };
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index f430bf3..41383e1 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -794,7 +794,7 @@
}
}
std::unique_ptr<SkSL::InterfaceBlock> result = std::make_unique<SkSL::InterfaceBlock>(
- intf.fOffset, var, id.fTypeName, id.fInstanceName, arraySize, symbols);
+ intf.fOffset, *var, id.fTypeName, id.fInstanceName, arraySize, symbols);
this->scanInterfaceBlock(*result);
return result;
}
diff --git a/src/sksl/SkSLRehydrator.cpp b/src/sksl/SkSLRehydrator.cpp
index 5d10c82..b9fab8b 100644
--- a/src/sksl/SkSLRehydrator.cpp
+++ b/src/sksl/SkSLRehydrator.cpp
@@ -285,7 +285,7 @@
skstd::string_view typeName = this->readString();
skstd::string_view instanceName = this->readString();
int arraySize = this->readS8();
- return std::make_unique<InterfaceBlock>(/*offset=*/-1, &var->as<Variable>(), typeName,
+ return std::make_unique<InterfaceBlock>(/*offset=*/-1, var->as<Variable>(), typeName,
instanceName, arraySize, nullptr);
}
case Rehydrator::kVarDeclarations_Command: {
diff --git a/src/sksl/codegen/SkSLSPIRVCodeGenerator.cpp b/src/sksl/codegen/SkSLSPIRVCodeGenerator.cpp
index 4cdd5fd..d42b6e2 100644
--- a/src/sksl/codegen/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/codegen/SkSLSPIRVCodeGenerator.cpp
@@ -3052,7 +3052,7 @@
intfVar.storage()));
fSPIRVBonusVariables.insert(modifiedVar);
InterfaceBlock modifiedCopy(intf.fOffset,
- modifiedVar,
+ *modifiedVar,
intf.typeName(),
intf.instanceName(),
intf.arraySize(),
@@ -3459,7 +3459,7 @@
// Create an interface block object for this global variable.
fUniformBuffer.fInterfaceBlock = std::make_unique<InterfaceBlock>(
- /*offset=*/-1, fUniformBuffer.fInnerVariable.get(), kUniformBufferName,
+ /*offset=*/-1, *fUniformBuffer.fInnerVariable, kUniformBufferName,
kUniformBufferName, /*arraySize=*/0, topLevelSymbolTable);
// Generate an interface block and hold onto its ID.
@@ -3528,7 +3528,7 @@
fProgram.fSymbols->add(std::make_unique<Field>(/*offset=*/-1, intfVar, /*field=*/0));
}
InterfaceBlock intf(/*offset=*/-1,
- intfVar,
+ *intfVar,
name,
/*instanceName=*/"",
/*arraySize=*/0,
diff --git a/src/sksl/dsl/DSLCore.cpp b/src/sksl/dsl/DSLCore.cpp
index 4c140b7..bc5b38f 100644
--- a/src/sksl/dsl/DSLCore.cpp
+++ b/src/sksl/dsl/DSLCore.cpp
@@ -230,20 +230,22 @@
if (!DSLWriter::Settings().fDSLMarkVarsDeclared) {
DSLWriter::MarkDeclared(var);
}
- auto intf = std::make_unique<SkSL::InterfaceBlock>(/*offset=*/-1,
- DSLWriter::Var(var), typeName, varName, arraySize, DSLWriter::SymbolTable());
- DSLWriter::IRGenerator().scanInterfaceBlock(*intf);
- DSLWriter::ProgramElements().push_back(std::move(intf));
- if (varName.empty()) {
- const std::vector<SkSL::Type::Field>& structFields = structType->fields();
- const SkSL::Variable* skslVar = DSLWriter::Var(var);
- for (size_t i = 0; i < structFields.size(); ++i) {
- DSLWriter::SymbolTable()->add(std::make_unique<SkSL::Field>(/*offset=*/-1,
- skslVar,
- i));
+ const SkSL::Variable* skslVar = DSLWriter::Var(var);
+ if (skslVar) {
+ auto intf = std::make_unique<SkSL::InterfaceBlock>(/*offset=*/-1,
+ *skslVar, typeName, varName, arraySize, DSLWriter::SymbolTable());
+ DSLWriter::IRGenerator().scanInterfaceBlock(*intf);
+ DSLWriter::ProgramElements().push_back(std::move(intf));
+ if (varName.empty()) {
+ const std::vector<SkSL::Type::Field>& structFields = structType->fields();
+ for (size_t i = 0; i < structFields.size(); ++i) {
+ DSLWriter::SymbolTable()->add(std::make_unique<SkSL::Field>(/*offset=*/-1,
+ skslVar,
+ i));
+ }
+ } else {
+ AddToSymbolTable(var);
}
- } else {
- AddToSymbolTable(var);
}
GetErrorReporter().reportPendingErrors(pos);
return var;
diff --git a/src/sksl/ir/SkSLInterfaceBlock.h b/src/sksl/ir/SkSLInterfaceBlock.h
index 0405e2e..1665446 100644
--- a/src/sksl/ir/SkSLInterfaceBlock.h
+++ b/src/sksl/ir/SkSLInterfaceBlock.h
@@ -31,7 +31,7 @@
public:
static constexpr Kind kProgramElementKind = Kind::kInterfaceBlock;
- InterfaceBlock(int offset, const Variable* var, skstd::string_view typeName,
+ InterfaceBlock(int offset, const Variable& var, skstd::string_view typeName,
skstd::string_view instanceName, int arraySize,
std::shared_ptr<SymbolTable> typeOwner)
: INHERITED(offset, kProgramElementKind)
@@ -42,11 +42,7 @@
, fTypeOwner(std::move(typeOwner)) {}
const Variable& variable() const {
- return *fVariable;
- }
-
- void setVariable(const Variable* var) {
- fVariable = var;
+ return fVariable;
}
skstd::string_view typeName() const {
@@ -66,7 +62,7 @@
}
std::unique_ptr<ProgramElement> clone() const override {
- return std::make_unique<InterfaceBlock>(fOffset, &this->variable(), this->typeName(),
+ return std::make_unique<InterfaceBlock>(fOffset, this->variable(), this->typeName(),
this->instanceName(), this->arraySize(),
SymbolTable::WrapIfBuiltin(this->typeOwner()));
}
@@ -91,7 +87,7 @@
}
private:
- const Variable* fVariable;
+ const Variable& fVariable;
skstd::string_view fTypeName;
skstd::string_view fInstanceName;
int fArraySize;
diff --git a/tests/sksl/errors/Ossfuzz39000.glsl b/tests/sksl/errors/Ossfuzz39000.glsl
new file mode 100644
index 0000000..163e3d4
--- /dev/null
+++ b/tests/sksl/errors/Ossfuzz39000.glsl
@@ -0,0 +1,4 @@
+### Compilation failed:
+
+error: 2: expected ';', but found '='
+1 error