From e69a70bfb7b3dff1391d5189d5c68fb798c02040 Mon Sep 17 00:00:00 2001
From: Jim Blandy <jimb@red-bean.com>
Date: Wed, 10 Nov 2021 13:58:57 -0800
Subject: [PATCH] [spv-in] Permit pointers to runtime arrays only in
 StorageBuffer. (#1522)

* [spv-in] Change shadow.spv test input to use StorageBuffer.

The ecosystem around Naga will generally not be able to use Vulkan adapters that
don't support the SPV_KHR_storage_buffer_storage_class (which was incorporated
into SPIR-V 1.3), so we can assume it is present.

Changing the test not to use runtime-sized arrays in the Uniform storage class
will allow Naga to tighten up some validation checks.

* [spv-in] Permit pointers to runtime arrays only in StorageBuffer.

Fixes #1519.
---
 src/front/spv/error.rs  |   2 ++
 src/front/spv/mod.rs    |  19 ++++++++++++++++++-
 tests/in/spv/shadow.spv | Bin 4764 -> 4812 bytes
 tests/out/ir/shadow.ron |  30 +++++++++++++++++++++++++-----
 4 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/src/front/spv/error.rs b/src/front/spv/error.rs
index 6817106906..54109dd852 100644
--- a/src/front/spv/error.rs
+++ b/src/front/spv/error.rs
@@ -41,6 +41,8 @@ pub enum Error {
     UnsupportedControlFlow(spirv::Word),
     #[error("unsupported binary operator %{0}")]
     UnsupportedBinaryOperator(spirv::Word),
+    #[error("Naga supports OpTypeRuntimeArray in the StorageBuffer storage class only")]
+    UnsupportedRuntimeArrayStorageClass,
     #[error("unknown binary operator {0:?}")]
     UnknownBinaryOperator(spirv::Op),
     #[error("unknown relational function {0:?}")]
diff --git a/src/front/spv/mod.rs b/src/front/spv/mod.rs
index 26f849bd2f..fc5d2096b7 100644
--- a/src/front/spv/mod.rs
+++ b/src/front/spv/mod.rs
@@ -3881,7 +3881,8 @@ impl<I: Iterator<Item = u32>> Parser<I> {
 
         let decor = self.future_decor.remove(&id);
         let base_lookup_ty = self.lookup_type.lookup(type_id)?;
-        let class = if let Some(class) = module.types[base_lookup_ty.handle].inner.pointer_class() {
+        let base_inner = &module.types[base_lookup_ty.handle].inner;
+        let class = if let Some(class) = base_inner.pointer_class() {
             class
         } else if self
             .lookup_storage_buffer_types
@@ -3897,6 +3898,22 @@ impl<I: Iterator<Item = u32>> Parser<I> {
             }
         };
 
+        // We don't support pointers to runtime-sized arrays in the `Uniform`
+        // storage class with the `BufferBlock` decoration. Runtime-sized arrays
+        // should be in the StorageBuffer class.
+        if let crate::TypeInner::Array {
+            size: crate::ArraySize::Dynamic,
+            ..
+        } = *base_inner
+        {
+            match class {
+                crate::StorageClass::Storage { .. } => {}
+                _ => {
+                    return Err(Error::UnsupportedRuntimeArrayStorageClass);
+                }
+            }
+        }
+
         // Don't bother with pointer stuff for `Handle` types.
         let lookup_ty = if class == crate::StorageClass::Handle {
             base_lookup_ty.clone()
diff --git a/tests/in/spv/shadow.spv b/tests/in/spv/shadow.spv
index 265cf65fc08335dfb5180801283e218430a0fd67..b4dff9df6da1fbcacb844a01d1f89b9e5dbc140f 100644
GIT binary patch
delta 358
zcmbQEdPdcinMs+Qfq{{Mfq|XjE|3HPK?WwU2p0o4LvTP?ythYCd~r#BQDS;(d{Svz
zT51uTo1BwaT+Fah@-!2t2LlU3DoFe0mrReD6nTIw5Z4+goDQVZfEXmE0OV(Y*puI~
z$_s)-5}~3wU{P7Nct)PdwQQ(z^<cTRZ1O<4vtaTqnu;EXZg!9gTQC{Rj%vnCu!>%=
z+*<Z}pgLU+R2AF7Dq=Z6X4Hboxo9emfK{9Y%e@7Yyqu_JTm-AI<pi3sIhNC&696+)
BJAMEF

delta 310
zcmX@3I!9HKnMs+Qfq{_$1nz=K1_nU}CI-fhuBVw;-5Hn}(l$#nKW0*70!pR=u{8q=
zLpqR717eVv0+62pVo#Q3lNSVuBtk`Vz@olv@r+E9*P_YQgXQkB$phv7f=O9+RNXxg
z-RvM0xnQyvO~p*GioIaDyX^Hqb+H_%W^4zm=;Z*Ju@+38MN@GEtl}?NPL>m-&XyC^
QjEi6uxgfdCy`1)(03z`#`~Uy|

diff --git a/tests/out/ir/shadow.ron b/tests/out/ir/shadow.ron
index 572ffe39f6..13220b7aea 100644
--- a/tests/out/ir/shadow.ron
+++ b/tests/out/ir/shadow.ron
@@ -200,21 +200,33 @@
             name: None,
             inner: Pointer(
                 base: 19,
-                class: Uniform,
+                class: Storage(
+                    access: (
+                        bits: 3,
+                    ),
+                ),
             ),
         ),
         (
             name: None,
             inner: Pointer(
                 base: 18,
-                class: Uniform,
+                class: Storage(
+                    access: (
+                        bits: 3,
+                    ),
+                ),
             ),
         ),
         (
             name: None,
             inner: Pointer(
                 base: 17,
-                class: Uniform,
+                class: Storage(
+                    access: (
+                        bits: 3,
+                    ),
+                ),
             ),
         ),
         (
@@ -235,14 +247,22 @@
             name: None,
             inner: Pointer(
                 base: 4,
-                class: Uniform,
+                class: Storage(
+                    access: (
+                        bits: 3,
+                    ),
+                ),
             ),
         ),
         (
             name: None,
             inner: Pointer(
                 base: 1,
-                class: Uniform,
+                class: Storage(
+                    access: (
+                        bits: 3,
+                    ),
+                ),
             ),
         ),
         (