reflect: guard against invalid spirv enum instantiation

spirv-cross-sys/native/spirv_cross_c_ext_rs.cpp

@@ -72,4 +72,8 @@ spvc_bool spvc_rs_type_is_forward_pointer(spvc_type type) {
     return type->forward_pointer;
 }
 
+void spvc_rs_compiler_get_execution_model_indirect(spvc_compiler compiler, SpvExecutionModel* out) {
+    *out = spvc_compiler_get_execution_model(compiler);
+}
+
 } // extern "C"

spirv-cross-sys/native/wrapper.h

@@ -13,4 +13,6 @@ spvc_result spvc_rs_compiler_variable_get_type(spvc_compiler compiler, spvc_vari
 
 spvc_bool spvc_rs_type_is_pointer(spvc_type type);
 
-spvc_bool spvc_rs_type_is_forward_pointer(spvc_type type);
+spvc_bool spvc_rs_type_is_forward_pointer(spvc_type type);
+
+void spvc_rs_compiler_get_execution_model_indirect(spvc_compiler compiler, SpvExecutionModel* out);

spirv-cross-sys/src/bindings.rs

@@ -4226,3 +4226,9 @@ extern "C" {
 extern "C" {
     pub fn spvc_rs_type_is_forward_pointer(type_: spvc_type) -> crate::ctypes::spvc_bool;
 }
+extern "C" {
+    pub fn spvc_rs_compiler_get_execution_model_indirect(
+        compiler: spvc_compiler,
+        out: *mut SpvExecutionModel,
+    );
+}

spirv-cross-sys/src/lib.rs

@@ -8,6 +8,9 @@
 //!
 //! Always go through `MaybeUninit` for anything that sets an enum,
 //! then check for `u32::MAX`.
+//!
+//! `spvc_rs` functions are unstable and are meant for consumption by [spirv-cross2](https://crates.io/crates/spirv-cross2)
+//! only.
 
 mod bindings;
 

spirv-cross2/Cargo.toml

@@ -23,6 +23,7 @@ bitflags = "2.6.0"
 
 half = { version = "2.4.1", optional = true }
 gfx-maths = { version = "0.2.9", optional = true }
+memchr = "2.7.4"
 
 [features]
 default = ["gfx-math-types", "f16"]

spirv-cross2/src/error.rs

@@ -54,6 +54,9 @@ pub enum SpirvCrossError {
         /// The column accessed.
         column: u32,
     },
+    #[error("An unexpected enum value was found.")]
+    /// An unexpected enum value was found.
+    InvalidEnum,
 }
 
 pub(crate) trait ToContextError {

spirv-cross2/src/reflect/entry_points.rs

@@ -1,5 +1,6 @@
 use crate::error::{SpirvCrossError, ToContextError};
 use crate::handle::Handle;
+use crate::reflect::try_valid_slice;
 use crate::string::ContextStr;
 use crate::{error, spirv};
 use crate::{Compiler, ContextRoot};
@@ -24,22 +25,23 @@ impl<'a> Iterator for ExtensionsIter<'a> {
 }
 
 /// Querying declared properties of the SPIR-V module.
-impl<'a, T> Compiler<'a, T> {
+impl<'ctx, T> Compiler<'ctx, T> {
     /// Gets the list of all SPIR-V Capabilities which were declared in the SPIR-V module.
-    pub fn declared_capabilities(&self) -> error::Result<&'a [spirv::Capability]> {
+    pub fn declared_capabilities(&self) -> error::Result<&'ctx [spirv::Capability]> {
         unsafe {
             let mut caps = std::ptr::null();
             let mut size = 0;
 
             sys::spvc_compiler_get_declared_capabilities(self.ptr.as_ptr(), &mut caps, &mut size)
                 .ok(self)?;
 
-            Ok(slice::from_raw_parts(caps, size))
+            const _: () = assert!(size_of::<spirv::Capability>() == size_of::<u32>());
+            try_valid_slice(caps, size)
         }
     }
 
     /// Gets the list of all SPIR-V extensions which were declared in the SPIR-V module.
-    pub fn declared_extensions(&self) -> error::Result<ExtensionsIter<'a>> {
+    pub fn declared_extensions(&self) -> error::Result<ExtensionsIter<'ctx>> {
         // SAFETY: 'a is OK to return here
         // https://github.com/KhronosGroup/SPIRV-Cross/blob/6a1fb66eef1bdca14acf7d0a51a3f883499d79f0/spirv_cross_c.cpp#L2756
         unsafe {
@@ -56,8 +58,20 @@ impl<'a, T> Compiler<'a, T> {
     }
 
     /// Get the execution model of the module.
-    pub fn execution_model(&self) -> spirv::ExecutionModel {
-        unsafe { sys::spvc_compiler_get_execution_model(self.ptr.as_ptr()) }
+    pub fn execution_model(&self) -> error::Result<spirv::ExecutionModel> {
+        unsafe {
+            let mut exec_model = MaybeUninit::zeroed();
+            sys::spvc_rs_compiler_get_execution_model_indirect(
+                self.ptr.as_ptr(),
+                exec_model.as_mut_ptr(),
+            );
+
+            if exec_model.as_ptr().cast::<u32>().read() == u32::MAX {
+                Err(SpirvCrossError::InvalidEnum)
+            } else {
+                Ok(exec_model.assume_init())
+            }
+        }
     }
 }
 
@@ -324,4 +338,19 @@ mod test {
 
         Ok(())
     }
+
+    #[test]
+    pub fn capabilities() -> Result<(), SpirvCrossError> {
+        let spv = SpirvCrossContext::new()?;
+        let vec = Vec::from(BASIC_SPV);
+        let words = Module::from_words(bytemuck::cast_slice(&vec));
+
+        let compiler: Compiler<targets::None> = spv.create_compiler(words)?;
+        let resources = compiler.shader_resources()?.all_resources()?;
+
+        let ty = compiler.declared_capabilities()?;
+        assert_eq!([spirv::Capability::Shader], ty);
+
+        Ok(())
+    }
 }

spirv-cross2/src/reflect/execution_modes.rs

@@ -1,10 +1,10 @@
 use crate::error::ToContextError;
 use crate::handle::Handle;
+use crate::reflect::try_valid_slice;
 use crate::Compiler;
 use crate::{error, spirv};
 use spirv_cross_sys as sys;
 use spirv_cross_sys::ConstantId;
-use std::slice;
 
 /// Arguments to an `OpExecutionMode`.
 #[derive(Debug)]
@@ -78,7 +78,9 @@ impl<'ctx, T> Compiler<'ctx, T> {
 
             // SAFETY: 'ctx is sound here.
             // https://github.com/KhronosGroup/SPIRV-Cross/blob/main/spirv_cross_c.cpp#L2250
-            Ok(slice::from_raw_parts(modes, size))
+
+            const _: () = assert!(size_of::<spirv::ExecutionMode>() == size_of::<u32>());
+            try_valid_slice(modes, size)
         }
     }
 
@@ -186,21 +188,9 @@ mod test {
         let compiler: Compiler<targets::None> = spv.create_compiler(words)?;
         let resources = compiler.shader_resources()?.all_resources()?;
 
-        // println!("{:#?}", resources);
-
         let ty = compiler.execution_modes()?;
         assert_eq!([spirv::ExecutionMode::OriginUpperLeft], ty);
 
-        let args = compiler.work_group_size_specialization_constants();
-        eprintln!("{:?}", args);
-
-        // match ty.inner {
-        //     TypeInner::Struct(ty) => {
-        //         compiler.get_type(ty.members[0].id)?;
-        //     }
-        //     TypeInner::Vector { .. } => {}
-        //     _ => {}
-        // }
         Ok(())
     }
 }

spirv-cross2/src/reflect/mod.rs

@@ -8,6 +8,7 @@ mod names;
 mod resources;
 mod types;
 
+use crate::{error, SpirvCrossError};
 pub use buffers::*;
 pub use combined_image_samplers::*;
 pub use constants::*;
@@ -16,3 +17,39 @@ pub use entry_points::*;
 pub use execution_modes::*;
 pub use resources::*;
 pub use types::*;
+
+/// Check if an enum slice contains u32 max.
+#[inline(always)]
+fn enum_slice_is_not_max<T>(enum_slice: &[u32]) -> bool {
+    #[cfg(target_endian = "big")]
+    let memchr = memchr::memmem::find(
+        bytemuck::must_cast_slice(enum_slice),
+        &*u32::MAX.to_be_bytes(),
+    );
+
+    #[cfg(target_endian = "little")]
+    let memchr = memchr::memmem::find(
+        bytemuck::must_cast_slice(enum_slice),
+        u32::MAX.to_le_bytes().as_slice(),
+    );
+
+    // memchr is only a heuristic, but if it doesn't find the bit pattern we're good.
+    if memchr.is_none() {
+        true
+    } else {
+        !enum_slice.contains(&u32::MAX)
+    }
+}
+
+// SAFETY: ensure size is u32 first!
+// won't need it after 1.79.
+unsafe fn try_valid_slice<'a, T>(ptr: *const T, size: usize) -> error::Result<&'a [T]> {
+    unsafe {
+        let int_slice = std::slice::from_raw_parts(ptr.cast::<u32>(), size);
+        if !enum_slice_is_not_max::<T>(int_slice) {
+            Err(SpirvCrossError::InvalidEnum)
+        } else {
+            Ok(std::slice::from_raw_parts(ptr, size))
+        }
+    }
+}

spirv-cross2/src/reflect/resources.rs

@@ -9,8 +9,9 @@ use spirv_cross_sys::{
     spvc_set,
 };
 use std::marker::PhantomData;
+use std::mem::MaybeUninit;
 use std::ptr::NonNull;
-use std::slice;
+use std::{ptr, slice};
 
 /// The type of built-in resources to query.
 pub use spirv_cross_sys::BuiltinResourceType;
@@ -173,7 +174,7 @@ impl<'a> Iterator for ResourceIter<'a> {
 /// Iterator over reflected builtin resources, created by [`ShaderResources::builtin_resources_for_type`].
 pub struct BuiltinResourceIter<'a>(
     PhantomCompiler<'a>,
-    slice::Iter<'a, spvc_reflected_builtin_resource>,
+    slice::Iter<'a, MaybeUninit<spvc_reflected_builtin_resource>>,
 );
 
 impl<'a> Iterator for BuiltinResourceIter<'a> {
@@ -182,7 +183,7 @@ impl<'a> Iterator for BuiltinResourceIter<'a> {
     fn next(&mut self) -> Option<Self::Item> {
         self.1
             .next()
-            .map(|o| BuiltinResource::from_raw(self.0.clone(), o))
+            .and_then(|o| BuiltinResource::from_raw(self.0.clone(), o))
     }
 }
 
@@ -276,12 +277,29 @@ impl From<BuiltinResource<'_>> for Handle<VariableId> {
 }
 
 impl<'a> BuiltinResource<'a> {
-    fn from_raw(comp: PhantomCompiler<'a>, value: &'a spvc_reflected_builtin_resource) -> Self {
-        Self {
+    fn from_raw(
+        comp: PhantomCompiler<'a>,
+        value: &'a MaybeUninit<spvc_reflected_builtin_resource>,
+    ) -> Option<Self> {
+        // builtin is potentially uninit, we need to check.
+        let value = unsafe {
+            let builtin = ptr::addr_of!((*value.as_ptr()).builtin);
+            if builtin.cast::<u32>().read() == u32::MAX {
+                if cfg!(debug_assertions) {
+                    panic!("Unexpected SpvBuiltIn in spvc_reflected_builtin_resource!")
+                } else {
+                    return None;
+                }
+            }
+
+            value.assume_init_ref()
+        };
+
+        Some(Self {
             builtin: value.builtin,
             value_type_id: comp.create_handle(value.value_type_id),
             resource: Resource::from_raw(comp, &value.resource),
-        }
+        })
     }
 }
 
@@ -468,7 +486,7 @@ impl<'ctx> ShaderResources<'ctx> {
             .ok(self)?;
         }
 
-        let slice = unsafe { std::slice::from_raw_parts(out, count) };
+        let slice = unsafe { slice::from_raw_parts(out, count) };
 
         Ok(ResourceIter(self.1.clone(), slice.iter()))
     }
@@ -495,12 +513,14 @@ impl<'ctx> ShaderResources<'ctx> {
             .ok(self)?;
         }
 
-        let slice = unsafe { std::slice::from_raw_parts(out, count) };
+        let slice = unsafe { slice::from_raw_parts(out.cast(), count) };
 
         Ok(BuiltinResourceIter(self.1.clone(), slice.iter()))
     }
 
     /// Get all resources declared in the shader.
+    ///
+    /// This will allocate a `Vec` for every resource type.
     #[rustfmt::skip]
     pub fn all_resources(&self) -> error::Result<AllResources<'ctx>> {
           // SAFETY: 'ctx is sound by transitive property of resources_for_type

spirv-cross2/tests/sandbox.rs

@@ -216,6 +216,7 @@ void main() {
     let compiler = cross.into_compiler::<spirv_cross2::targets::None>(Module::from_words(&spv))?;
     let res = compiler.shader_resources()?.all_resources()?;
 
+    eprintln!("{:?}", res.builtin_inputs);
     let counter = &res.storage_buffers[0];
 
     let TypeInner::Struct(struct_ty) = compiler.type_description(counter.base_type_id)?.inner