commit b05bbd03f98e5b5857df21dcad04dd8bbe3b2b72
author John Stiles <johnstiles@google.com> Fri Sep 24 15:11:16 2021 -0400
committer John Stiles <johnstiles@google.com> Fri Sep 24 20:15:31 2021 +0000
tree 36d478a3a58a446850c30af1c3ea896b0d2ad985
parent fcc0ef1ee4841101576dc7f9dfca0e1398ee923c

Fix for fuzzer-discovered error with invalid var-initializers.

If a VarDeclaration line contained multiple variables, and the first
variable had an illegal initializer-expression, the Declare() would
return a Nop. AddVarDeclaration did not expect to see a Nop and would
assert once we tried to process the second var-declaration. Now, we
allow adding var declarations to a Nop.

Bulked up some tests to cover local and global variables (since those
are parsed in separate functions) and to check both the first
initializer as well as follow-on initializers (since those are parsed in
separate parts of the var-decl handler).

Change-Id: I66341191698175b490a659715cb8edaafe2f75ae
Bug: oss-fuzz:39032
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452696
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>

src/sksl/SkSLDSLParser.cpp

diff --git a/src/sksl/SkSLDSLParser.cpp b/src/sksl/SkSLDSLParser.cpp
index 4cf8d44..a704855 100644
--- a/src/sksl/SkSLDSLParser.cpp
+++ b/src/sksl/SkSLDSLParser.cpp
@@ -430,7 +430,9 @@
     if (!this->parseArrayDimensions(offset, &type)) {
         return;
     }
-    this->parseInitializer(offset, &initializer);
+    if (!this->parseInitializer(offset, &initializer)) {
+        return;
+    }
     DSLGlobalVar first(mods, type, name, std::move(initializer), pos);
     Declare(first);
     AddToSymbolTable(first);
@@ -448,7 +450,8 @@
         if (!this->parseInitializer(offset, &anotherInitializer)) {
             return;
         }
-        DSLGlobalVar next(mods, type, this->text(identifierName), std::move(anotherInitializer));
+        DSLGlobalVar next(mods, type, this->text(identifierName), std::move(anotherInitializer),
+                          this->position(offset));
         Declare(next);
         AddToSymbolTable(next, this->position(identifierName));
     }
@@ -466,7 +469,9 @@
     if (!this->parseArrayDimensions(offset, &type)) {
         return {};
     }
-    this->parseInitializer(offset, &initializer);
+    if (!this->parseInitializer(offset, &initializer)) {
+        return {};
+    }
     DSLVar first(mods, type, name, std::move(initializer), pos);
     DSLStatement result = Declare(first);
     AddToSymbolTable(first);
@@ -484,7 +489,8 @@
         if (!this->parseInitializer(offset, &anotherInitializer)) {
             return result;
         }
-        DSLVar next(mods, type, this->text(identifierName), std::move(anotherInitializer));
+        DSLVar next(mods, type, this->text(identifierName), std::move(anotherInitializer),
+                    this->position(offset));
         DSLWriter::AddVarDeclaration(result, next);
         AddToSymbolTable(next, this->position(identifierName));
     }

src/sksl/dsl/priv/DSLWriter.cpp

diff --git a/src/sksl/dsl/priv/DSLWriter.cpp b/src/sksl/dsl/priv/DSLWriter.cpp
index 11ad426..5fc10ab 100644
--- a/src/sksl/dsl/priv/DSLWriter.cpp
+++ b/src/sksl/dsl/priv/DSLWriter.cpp
@@ -138,13 +138,15 @@
         SkSL::Block& block = existing.fStatement->as<Block>();
         SkASSERT(!block.isScope());
         block.children().push_back(Declare(additional).release());
-    } else {
-        SkASSERT(existing.fStatement->is<VarDeclaration>());
+    } else if (existing.fStatement->is<VarDeclaration>()) {
         StatementArray stmts;
         stmts.reserve_back(2);
         stmts.push_back(std::move(existing.fStatement));
         stmts.push_back(Declare(additional).release());
         existing.fStatement = SkSL::Block::MakeUnscoped(/*offset=*/-1, std::move(stmts));
+    } else if (existing.fStatement->isEmpty()) {
+        // If the variable declaration generated an error, we can end up with a Nop statement here.
+        existing.fStatement = Declare(additional).release();
     }
 }