From a4ddb3bc4d240ef9b892ee3721c110bde400fec1 Mon Sep 17 00:00:00 2001 From: i509VCB Date: Fri, 9 Dec 2022 17:35:08 -0600 Subject: [PATCH] Harden ObjectId -> wasm conversions --- wgpu/Cargo.toml | 2 +- wgpu/src/backend/web.rs | 57 ++++++++++++++++------------------------- 2 files changed, 23 insertions(+), 36 deletions(-) diff --git a/wgpu/Cargo.toml b/wgpu/Cargo.toml index 9eebf1ec8c..7da719dd3d 100644 --- a/wgpu/Cargo.toml +++ b/wgpu/Cargo.toml @@ -76,7 +76,7 @@ name = "water" test = true [features] -default = ["wgsl", "expose-ids"] +default = ["wgsl", "expose-ids", "strict_asserts"] # Apply run-time checks, even in release builds. These are in addition # to the validation carried out at public APIs in all builds. strict_asserts = ["wgc/strict_asserts"] diff --git a/wgpu/src/backend/web.rs b/wgpu/src/backend/web.rs index 4ac768ad22..9f58a68684 100644 --- a/wgpu/src/backend/web.rs +++ b/wgpu/src/backend/web.rs @@ -42,33 +42,25 @@ fn create_identified(value: T) -> Identified { // type is (for now) harmless. Eventually wasm32 will support threading, and depending on how this // is integrated (or not integrated) with values like those in webgpu, this may become unsound. -unsafe fn wasm_from_id(id: ObjectId) -> T -where - T::Abi: From, -{ - let raw = NonZeroU128::from(id); - let abi = ::from(raw.get() as u32); - unsafe { T::from_abi(abi) } -} - -impl From for Identified -where - T::Abi: From, -{ +impl + JsCast> From for Identified { fn from(id: ObjectId) -> Self { let raw = std::num::NonZeroU128::from(id); - let wasm = unsafe { wasm_from_id(id) }; let global_id = (raw.get() >> 64) as u64; - Self(wasm, global_id) + + // SAFETY: wasm_bindgen says an ABI representation may only be cast to a wrapper type if it was created + // using into_abi. + // + // This assumption we sadly have to assume to prevent littering the code with unsafe blocks. + let wasm = unsafe { JsValue::from_abi(raw.get() as u32) }; + strict_assert!(wasm.is_instance_of::()); + // SAFETY: The ABI of the type must be a u32, and strict asserts ensure the right type is used. + Self(wasm.unchecked_into(), global_id) } } -impl From> for ObjectId -where - T::Abi: Into, -{ +impl> From> for ObjectId { fn from(id: Identified) -> Self { - let abi = id.0.into_abi().into(); + let abi = id.0.into_abi(); let id = abi as u128 | ((id.1 as u128) << 64); Self::from(NonZeroU128::new(id).unwrap()) } @@ -1342,8 +1334,7 @@ impl crate::context::Context for Context { size, }) => { let buffer = &<::BufferId>::from(buffer.id).0; - let mut mapped_buffer_binding = - web_sys::GpuBufferBinding::new(buffer); + let mut mapped_buffer_binding = web_sys::GpuBufferBinding::new(buffer); mapped_buffer_binding.offset(offset as f64); if let Some(s) = size { mapped_buffer_binding.size(s.get() as f64); @@ -1372,8 +1363,7 @@ impl crate::context::Context for Context { .collect::(); let bgl = &<::BindGroupLayoutId>::from(desc.layout.id).0; - let mut mapped_desc = - web_sys::GpuBindGroupDescriptor::new(&mapped_entries, bgl); + let mut mapped_desc = web_sys::GpuBindGroupDescriptor::new(&mapped_entries, bgl); if let Some(label) = desc.label { mapped_desc.label(label); } @@ -1411,10 +1401,7 @@ impl crate::context::Context for Context { desc: &crate::RenderPipelineDescriptor, ) -> (Self::RenderPipelineId, Self::RenderPipelineData) { let module = &<::ShaderModuleId>::from(desc.vertex.module.id).0; - let mut mapped_vertex_state = web_sys::GpuVertexState::new( - desc.vertex.entry_point, - module, - ); + let mut mapped_vertex_state = web_sys::GpuVertexState::new(desc.vertex.entry_point, module); let buffers = desc .vertex @@ -1483,11 +1470,8 @@ impl crate::context::Context for Context { }) .collect::(); let module = &<::ShaderModuleId>::from(frag.module.id).0; - let mapped_fragment_desc = web_sys::GpuFragmentState::new( - frag.entry_point, - module, - &targets, - ); + let mapped_fragment_desc = + web_sys::GpuFragmentState::new(frag.entry_point, module, &targets); mapped_desc.fragment(&mapped_fragment_desc); } @@ -2050,7 +2034,8 @@ impl crate::context::Context for Context { mapped_color_attachment.clear_value(&cv); } if let Some(rt) = ca.resolve_target { - let texture_view = &<::TextureViewId>::from(rt.id).0; + let texture_view = + &<::TextureViewId>::from(rt.id).0; mapped_color_attachment.resolve_target(texture_view); } mapped_color_attachment.store_op(map_store_op(ca.ops.store)); @@ -2097,7 +2082,9 @@ impl crate::context::Context for Context { None => (web_sys::GpuLoadOp::Load, web_sys::GpuStoreOp::Store), }; let mut mapped_depth_stencil_attachment = - web_sys::GpuRenderPassDepthStencilAttachment::new(&<::TextureViewId>::from(dsa.view.id).0); + web_sys::GpuRenderPassDepthStencilAttachment::new( + &<::TextureViewId>::from(dsa.view.id).0, + ); mapped_depth_stencil_attachment.depth_clear_value(depth_clear_value); mapped_depth_stencil_attachment.depth_load_op(depth_load_op); mapped_depth_stencil_attachment.depth_store_op(depth_store_op);