Fix crash with invalid out parameters.
Many calls to `setRefKind` failed to check the return value; if it's
false, an error has occurred and the program is in a bad state.
Specifically, there is an assignment to a variable that's not marked as
"written-to." If we continue processing the program, we're likely to
assert.
Change-Id: I2dd5d1f41aa5ca0d30f8d638f05fe2e838216d78
Bug: skia:10753
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319116
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 21b9c12..f9d2683 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -2099,11 +2099,13 @@
if (!arguments[i]) {
return nullptr;
}
- if (arguments[i] && (function.fParameters[i]->fModifiers.fFlags & Modifiers::kOut_Flag)) {
- this->setRefKind(*arguments[i],
- function.fParameters[i]->fModifiers.fFlags & Modifiers::kIn_Flag ?
- VariableReference::kReadWrite_RefKind :
- VariableReference::kPointer_RefKind);
+ const Modifiers& paramModifiers = function.fParameters[i]->fModifiers;
+ if (paramModifiers.fFlags & Modifiers::kOut_Flag) {
+ if (!this->setRefKind(*arguments[i], paramModifiers.fFlags & Modifiers::kIn_Flag
+ ? VariableReference::kReadWrite_RefKind
+ : VariableReference::kPointer_RefKind)) {
+ return nullptr;
+ }
}
}
@@ -2359,23 +2361,23 @@
return nullptr;
}
return base;
+
case Token::Kind::TK_MINUS:
- if (base->kind() == Expression::Kind::kIntLiteral) {
- return std::unique_ptr<Expression>(new IntLiteral(fContext, base->fOffset,
- -base->as<IntLiteral>().fValue));
+ if (base->is<IntLiteral>()) {
+ return std::make_unique<IntLiteral>(fContext, base->fOffset,
+ -base->as<IntLiteral>().fValue);
}
- if (base->kind() == Expression::Kind::kFloatLiteral) {
- double value = -base->as<FloatLiteral>().fValue;
- return std::unique_ptr<Expression>(new FloatLiteral(fContext, base->fOffset,
- value));
+ if (base->is<FloatLiteral>()) {
+ return std::make_unique<FloatLiteral>(fContext, base->fOffset,
+ -base->as<FloatLiteral>().fValue);
}
if (!baseType.isNumber() && baseType.typeKind() != Type::TypeKind::kVector) {
fErrors.error(expression.fOffset,
"'-' cannot operate on '" + baseType.displayName() + "'");
return nullptr;
}
- return std::unique_ptr<Expression>(new PrefixExpression(Token::Kind::TK_MINUS,
- std::move(base)));
+ return std::make_unique<PrefixExpression>(Token::Kind::TK_MINUS, std::move(base));
+
case Token::Kind::TK_PLUSPLUS:
if (!baseType.isNumber()) {
fErrors.error(expression.fOffset,
@@ -2383,7 +2385,9 @@
"' cannot operate on '" + baseType.displayName() + "'");
return nullptr;
}
- this->setRefKind(*base, VariableReference::kReadWrite_RefKind);
+ if (!this->setRefKind(*base, VariableReference::kReadWrite_RefKind)) {
+ return nullptr;
+ }
break;
case Token::Kind::TK_MINUSMINUS:
if (!baseType.isNumber()) {
@@ -2392,7 +2396,9 @@
"' cannot operate on '" + baseType.displayName() + "'");
return nullptr;
}
- this->setRefKind(*base, VariableReference::kReadWrite_RefKind);
+ if (!this->setRefKind(*base, VariableReference::kReadWrite_RefKind)) {
+ return nullptr;
+ }
break;
case Token::Kind::TK_LOGICALNOT:
if (baseType != *fContext.fBool_Type) {
@@ -2402,8 +2408,8 @@
return nullptr;
}
if (base->kind() == Expression::Kind::kBoolLiteral) {
- return std::unique_ptr<Expression>(
- new BoolLiteral(fContext, base->fOffset, !base->as<BoolLiteral>().fValue));
+ return std::make_unique<BoolLiteral>(fContext, base->fOffset,
+ !base->as<BoolLiteral>().fValue);
}
break;
case Token::Kind::TK_BITWISENOT:
@@ -2417,8 +2423,7 @@
default:
ABORT("unsupported prefix operator\n");
}
- return std::unique_ptr<Expression>(new PrefixExpression(expression.getToken().fKind,
- std::move(base)));
+ return std::make_unique<PrefixExpression>(expression.getToken().fKind, std::move(base));
}
std::unique_ptr<Expression> IRGenerator::convertIndex(std::unique_ptr<Expression> base,
@@ -2784,9 +2789,10 @@
"' cannot operate on '" + baseType.displayName() + "'");
return nullptr;
}
- this->setRefKind(*base, VariableReference::kReadWrite_RefKind);
- return std::unique_ptr<Expression>(new PostfixExpression(std::move(base),
- expression.getToken().fKind));
+ if (!this->setRefKind(*base, VariableReference::kReadWrite_RefKind)) {
+ return nullptr;
+ }
+ return std::make_unique<PostfixExpression>(std::move(base), expression.getToken().fKind);
}
void IRGenerator::checkValid(const Expression& expr) {
diff --git a/src/sksl/SkSLInliner.cpp b/src/sksl/SkSLInliner.cpp
index 2eb7758..f17b9fe 100644
--- a/src/sksl/SkSLInliner.cpp
+++ b/src/sksl/SkSLInliner.cpp
@@ -668,7 +668,7 @@
const Variable* p = function.fDeclaration.fParameters[i];
if (p->fModifiers.fFlags & Modifiers::kOut_Flag) {
SkASSERT(varMap.find(p) != varMap.end());
- if (arguments[i]->kind() == Expression::Kind::kVariableReference &&
+ if (arguments[i]->is<VariableReference>() &&
&arguments[i]->as<VariableReference>().fVariable == varMap[p]) {
// We didn't create a temporary for this parameter, so there's nothing to copy back
// out.
diff --git a/tests/SkSLGLSLTestbed.cpp b/tests/SkSLGLSLTestbed.cpp
index baffd8b..cbf40c1 100644
--- a/tests/SkSLGLSLTestbed.cpp
+++ b/tests/SkSLGLSLTestbed.cpp
@@ -30,6 +30,7 @@
*inputs = program->fInputs;
REPORTER_ASSERT(r, compiler.toGLSL(*program, &output));
REPORTER_ASSERT(r, output != "");
+ //SkDebugf("GLSL output:\n\n%s", output.c_str());
}
}
diff --git a/tests/sksl/errors/golden/InvalidOutParams.glsl b/tests/sksl/errors/golden/InvalidOutParams.glsl
index 9d598bb..c3621d4 100644
--- a/tests/sksl/errors/golden/InvalidOutParams.glsl
+++ b/tests/sksl/errors/golden/InvalidOutParams.glsl
@@ -1,3 +1,6 @@
### Compilation failed:
-
+error: 1: cannot assign to this expression
+error: 5: cannot assign to this expression
+error: 6: cannot assign to this expression
+3 errors
