commit dc652564a7639f0c4269bd2d94e29e15fac4c810
author John Stiles <johnstiles@google.com> Mon Sep 13 08:55:40 2021 -0400
committer SkCQ <skcq-be@skia-corp.google.com.iam.gserviceaccount.com> Mon Sep 13 13:34:48 2021 +0000
tree 81165656f7b4364fb72c57584f077d5b85efc313
parent fe678d9b1c1355b53ee1964022654e11d6eac0d4

Fix for-statement assertion detected by the fuzzer.

The fuzzer found that the `DetectVarDeclarationWithoutScope` check was
placed too late in the function, and could be skipped over by for-loops
containing multiple variables. This was caught in ForStatement::Make,
which mirrors the Convert postconditions with matching assertions.

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

gn/sksl_tests.gni

diff --git a/gn/sksl_tests.gni b/gn/sksl_tests.gni
index 7754717..7a1194f 100644
--- a/gn/sksl_tests.gni
+++ b/gn/sksl_tests.gni
@@ -102,6 +102,7 @@
   "/sksl/errors/Ossfuzz37620.sksl",
   "/sksl/errors/Ossfuzz38106.sksl",
   "/sksl/errors/Ossfuzz38140.sksl",
+  "/sksl/errors/Ossfuzz38560.sksl",
   "/sksl/errors/OverflowFloatLiteral.sksl",
   "/sksl/errors/OverflowIntLiteral.sksl",
   "/sksl/errors/OverflowInt64Literal.sksl",

resources/sksl/errors/Ossfuzz38560.sksl

diff --git a/resources/sksl/errors/Ossfuzz38560.sksl b/resources/sksl/errors/Ossfuzz38560.sksl
new file mode 100644
index 0000000..1b2eab7
--- /dev/null
+++ b/resources/sksl/errors/Ossfuzz38560.sksl
@@ -0,0 +1 @@
+void n() { for(int r,r;;) int s; }

src/sksl/ir/SkSLForStatement.cpp

diff --git a/src/sksl/ir/SkSLForStatement.cpp b/src/sksl/ir/SkSLForStatement.cpp
index 9114322..9f8eb97 100644
--- a/src/sksl/ir/SkSLForStatement.cpp
+++ b/src/sksl/ir/SkSLForStatement.cpp
@@ -117,6 +117,10 @@
         }
     }
 
+    if (Analysis::DetectVarDeclarationWithoutScope(*statement, context.fErrors)) {
+        return nullptr;
+    }
+
     if (isVardeclBlockInitializer) {
         // If the initializer statement of a for loop contains multiple variables, this causes
         // difficulties for several of our backends; e.g. Metal doesn't have a way to express arrays
@@ -132,10 +136,6 @@
         return Block::Make(offset, std::move(scope));
     }
 
-    if (Analysis::DetectVarDeclarationWithoutScope(*statement, context.fErrors)) {
-        return nullptr;
-    }
-
     return ForStatement::Make(context, offset, std::move(initializer), std::move(test),
                               std::move(next), std::move(statement), std::move(unrollInfo),
                               std::move(symbolTable));

tests/sksl/errors/Ossfuzz38560.glsl

diff --git a/tests/sksl/errors/Ossfuzz38560.glsl b/tests/sksl/errors/Ossfuzz38560.glsl
new file mode 100644
index 0000000..42b75d7
--- /dev/null
+++ b/tests/sksl/errors/Ossfuzz38560.glsl
@@ -0,0 +1,5 @@
+### Compilation failed:
+
+error: 1: symbol 'r' was already defined
+error: 1: variable 's' must be created in a scope
+2 errors