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>

src/sksl/SkSLIRGenerator.cpp

@@ -2099,11 +2099,13 @@ std::unique_ptr<Expression> IRGenerator::call(int offset,
         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 @@ std::unique_ptr<Expression> IRGenerator::convertPrefixExpression(const ASTNode&
                 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 @@ std::unique_ptr<Expression> IRGenerator::convertPrefixExpression(const ASTNode&
                               "' 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 @@ std::unique_ptr<Expression> IRGenerator::convertPrefixExpression(const ASTNode&
                               "' 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 @@ std::unique_ptr<Expression> IRGenerator::convertPrefixExpression(const ASTNode&
                 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 @@ std::unique_ptr<Expression> IRGenerator::convertPrefixExpression(const ASTNode&
         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 @@ std::unique_ptr<Expression> IRGenerator::convertPostfixExpression(const ASTNode&
                       "' 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) {

src/sksl/SkSLInliner.cpp

@@ -668,7 +668,7 @@ Inliner::InlinedCall Inliner::inlineCall(FunctionCall* call,
         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.

tests/SkSLGLSLTestbed.cpp

@@ -30,6 +30,7 @@ static void test(skiatest::Reporter* r, const SkSL::Program::Settings& settings,
         *inputs = program->fInputs;
         REPORTER_ASSERT(r, compiler.toGLSL(*program, &output));
         REPORTER_ASSERT(r, output != "");
+        //SkDebugf("GLSL output:\n\n%s", output.c_str());
     }
 }
 

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