Reject spirv arrays that have incorrect stride in rust (#1050)

SPIR-V allows the array stride and size of a type to differ, but rust defines them to be the same. Thus
certain types when represented in rust will have the wrong layout. E.g. an array of vec3 can have an array
stride of 16 in SPIR-V, but an array of [f32;3] in rust would have a stride of 12. Thus using one for the
other would cause corruption.

This suggests a workaround by using a wrapping struct or upgrading the size of the type to one where the size
is the array stride.

I considered generating the wrapping struct for the user, but that seems very confusing for the user. We could
generate wrapping structs for all vec and mat types in arrays, but that would be a large API change.

See #298.

CHANGELOG.md

@@ -5,8 +5,8 @@
 - Fix instance_count when using draw_index with instance buffers
 - Added a `reinterpret` function to `BufferSlice`
 - Made `AttributeInfo` derive `Copy`, `Clone` and `Debug`
-
 - Use [google/shaderc](https://github.com/google/shaderc-rs) for shader compilation
+- Reject generation of rust types for SPIR-V arrays that would have incorrect array stride.
 
 # Version 0.10.0 (2018-08-10)
 

vulkano-shaders/src/lib.rs

@@ -536,3 +536,66 @@ fn capability_name(cap: &enums::Capability) -> Option<&'static str> {
         enums::Capability::CapabilityMultiViewport => Some("multi_viewport"),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_bad_alignment() {
+        // vec3/mat3/mat3x* are problematic in arrays since their rust
+        // representations don't have the same array stride as the SPIR-V
+        // ones. E.g. in a vec3[2], the second element starts on the 16th
+        // byte, but in a rust [[f32;3];2], the second element starts on the
+        // 12th byte. Since we can't generate code for these types, we should
+        // create an error instead of generating incorrect code.
+        let comp = compile("
+        #version 450
+        struct MyStruct {
+            vec3 vs[2];
+        };
+        layout(binding=0) uniform UBO {
+            MyStruct s;
+        };
+        void main() {}
+        ", ShaderKind::Vertex).unwrap();
+        let doc = parse::parse_spirv(comp.as_binary()).unwrap();
+        let res = std::panic::catch_unwind(|| structs::write_structs(&doc));
+        assert!(res.is_err());
+    }
+    #[test]
+    fn test_trivial_alignment() {
+        let comp = compile("
+        #version 450
+        struct MyStruct {
+            vec4 vs[2];
+        };
+        layout(binding=0) uniform UBO {
+            MyStruct s;
+        };
+        void main() {}
+        ", ShaderKind::Vertex).unwrap();
+        let doc = parse::parse_spirv(comp.as_binary()).unwrap();
+        structs::write_structs(&doc);
+    }
+    #[test]
+    fn test_wrap_alignment() {
+        // This is a workaround suggested in the case of test_bad_alignment,
+        // so we should make sure it works.
+        let comp = compile("
+        #version 450
+        struct Vec3Wrap {
+            vec3 v;
+        };
+        struct MyStruct {
+            Vec3Wrap vs[2];
+        };
+        layout(binding=0) uniform UBO {
+            MyStruct s;
+        };
+        void main() {}
+        ", ShaderKind::Vertex).unwrap();
+        let doc = parse::parse_spirv(comp.as_binary()).unwrap();
+        structs::write_structs(&doc);
+    }
+}

vulkano-shaders/src/structs.rs

@@ -386,6 +386,7 @@ pub fn type_from_id(doc: &parse::Spirv, searched: u32) -> (String, Option<usize>
             } if result_id == searched => {
                 debug_assert_eq!(mem::align_of::<[u32; 3]>(), mem::align_of::<u32>());
                 let (t, t_size, t_align) = type_from_id(doc, type_id);
+                let t_size = t_size.expect("array components must be sized");
                 let len = doc.instructions
                     .iter()
                     .filter_map(|e| match e {
@@ -399,7 +400,24 @@ pub fn type_from_id(doc: &parse::Spirv, searched: u32) -> (String, Option<usize>
                     .next()
                     .expect("failed to find array length");
                 let len = len.iter().rev().fold(0u64, |a, &b| (a << 32) | b as u64);
-                return (format!("[{}; {}]", t, len), t_size.map(|s| s * len as usize), t_align); // FIXME:
+                let stride = doc.instructions.iter().filter_map(|e| match e {
+                    parse::Instruction::Decorate{
+                        target_id,
+                        decoration: enums::Decoration::DecorationArrayStride,
+                        ref params,
+                    } if *target_id == searched => {
+                        Some(params[0])
+                    },
+                    _ => None,
+                })
+                .next().expect("failed to find ArrayStride decoration");
+                if stride as usize > t_size {
+                    panic!("Not possible to generate a rust array with the correct alignment since the SPIR-V \
+                            ArrayStride is larger than the size of the array element in rust. Try wrapping \
+                            the array element in a struct or rounding up the size of a vector or matrix \
+                            (e.g. increase a vec3 to a vec4)")
+                }
+                return (format!("[{}; {}]", t, len), Some(t_size * len as usize), t_align);
             },
             &parse::Instruction::TypeRuntimeArray { result_id, type_id }
                 if result_id == searched => {