Detect invalid layout(push_constant) modifier.

Previously, the frontend would allow this flag combination through. However, the SPIR-V backend asserts when layout(push_constant) is applied to an in-variable or out-variable, or when a binding/set is used. Bug: oss-fuzz:58375 Change-Id: Ibad8879b50818a9ba6953918b85edaa64654e2cc Reviewed-on: https://skia-review.googlesource.com/c/skia/+/683200 Commit-Queue: Arman Uguray <[email protected]> Auto-Submit: John Stiles <[email protected]> Reviewed-by: Arman Uguray <[email protected]>
armansito · Apr 27, 2023 · 487475e · 487475e
1 parent 1efb703
commit 487475e
Show file tree

Hide file tree

Showing 7 changed files with 25 additions and 1 deletion.
diff --git a/gn/sksl_tests.gni b/gn/sksl_tests.gni
@@ -189,6 +189,7 @@ sksl_error_tests = [
   "errors/Ossfuzz50922.sksl",
   "errors/Ossfuzz56373.sksl",
   "errors/Ossfuzz58037.sksl",
+  "errors/Ossfuzz58375.sksl",
   "errors/OverflowFloatIntrinsic.sksl",
   "errors/OverflowFloatLiteral.rts",
   "errors/OverflowInlinedLiteral.sksl",

diff --git a/resources/sksl/BUILD.bazel b/resources/sksl/BUILD.bazel
@@ -361,6 +361,7 @@ skia_filegroup(
         "errors/Ossfuzz50922.sksl",
         "errors/Ossfuzz56373.sksl",
         "errors/Ossfuzz58037.sksl",
+        "errors/Ossfuzz58375.sksl",
         "errors/OverflowFloatIntrinsic.sksl",
         "errors/OverflowFloatLiteral.rts",
         "errors/OverflowInlinedLiteral.sksl",

diff --git a/resources/sksl/errors/LayoutRepeatedQualifiers.sksl b/resources/sksl/errors/LayoutRepeatedQualifiers.sksl
@@ -49,5 +49,6 @@ layout qualifier 'gl' appears more than once
 'layout(color)' is only permitted on 'uniform' variables
 'layout(color)' is not permitted on variables of type 'float'
 only one backend qualifier can be used
+layout qualifier 'push_constant' is not permitted here
 layout qualifier 'set' is not permitted here
 *%%*/
diff --git a/resources/sksl/errors/Ossfuzz58375.sksl b/resources/sksl/errors/Ossfuzz58375.sksl
@@ -0,0 +1,5 @@
+layout (push_constant) in s { int x; };
+
+half4 main(float2) {
+    return half4(0);
+}
diff --git a/src/sksl/ir/SkSLVarDeclarations.cpp b/src/sksl/ir/SkSLVarDeclarations.cpp
@@ -323,6 +323,13 @@ void VarDeclaration::ErrorCheck(const Context& context,
         // Disallow all layout flags except 'color' in runtime effects
         permittedLayoutFlags &= Layout::kColor_Flag;
     }
+
+    // The `push_constant` flag isn't allowed on in-variables, out-variables, bindings or sets.
+    if ((modifiers.fLayout.fFlags & (Layout::kSet_Flag | Layout::kBinding_Flag)) ||
+        (modifiers.fFlags & (Modifiers::kIn_Flag | Modifiers::kOut_Flag))) {
+        permittedLayoutFlags &= ~Layout::kPushConstant_Flag;
+    }
+
     modifiers.checkPermitted(context, modifiersPosition, permitted, permittedLayoutFlags);
 }
 

diff --git a/tests/sksl/errors/LayoutRepeatedQualifiers.glsl b/tests/sksl/errors/LayoutRepeatedQualifiers.glsl
@@ -54,7 +54,10 @@ layout (
 error: 1: only one backend qualifier can be used
 layout (
 ^^^^^^^^...
+error: 1: layout qualifier 'push_constant' is not permitted here
+layout (
+^^^^^^^^...
 error: 1: layout qualifier 'set' is not permitted here
 layout (
 ^^^^^^^^...
-19 errors
+20 errors
diff --git a/tests/sksl/errors/Ossfuzz58375.glsl b/tests/sksl/errors/Ossfuzz58375.glsl
@@ -0,0 +1,6 @@
+### Compilation failed:
+
+error: 1: layout qualifier 'push_constant' is not permitted here
+layout (push_constant) in s { int x; };
+^^^^^^^^^^^^^^^^^^^^^^^^^
+1 error