d3d12: null-check the ComPtr produced by some creation functions

wgpu-hal/src/dx12/descriptor.rs

@@ -1,3 +1,4 @@
+use super::null_comptr_check;
 use crate::auxil::dxgi::result::HResult as _;
 use bit_set::BitSet;
 use parking_lot::Mutex;
@@ -53,6 +54,8 @@ impl GeneralHeap {
                 .into_device_result("Descriptor heap creation")?
         };
 
+        null_comptr_check(&raw)?;
+
         Ok(Self {
             raw: raw.clone(),
             ty,
@@ -130,6 +133,8 @@ impl FixedSizeHeap {
             )
             .into_device_result("Descriptor heap creation")?;
 
+        null_comptr_check(&heap)?;
+
         Ok(Self {
             handle_size: device.get_descriptor_increment_size(ty) as _,
             availability: !0, // all free!
@@ -254,6 +259,8 @@ impl CpuHeap {
             .create_descriptor_heap(total, ty, d3d12::DescriptorHeapFlags::empty(), 0)
             .into_device_result("CPU descriptor heap creation")?;
 
+        null_comptr_check(&raw)?;
+
         Ok(Self {
             inner: Mutex::new(CpuHeapInner {
                 _raw: raw.clone(),

wgpu-hal/src/dx12/device.rs

@@ -1,9 +1,11 @@
 use crate::{
     auxil::{self, dxgi::result::HResult as _},
     dx12::shader_compilation,
+    DeviceError,
 };
+use d3d12::ComPtr;
 
-use super::{conv, descriptor, view};
+use super::{conv, descriptor, null_comptr_check, view};
 use parking_lot::Mutex;
 use std::{
     ffi, mem,
@@ -29,7 +31,7 @@ impl super::Device {
         private_caps: super::PrivateCapabilities,
         library: &Arc<d3d12::D3D12Lib>,
         dxc_container: Option<Arc<shader_compilation::DxcContainer>>,
-    ) -> Result<Self, crate::DeviceError> {
+    ) -> Result<Self, DeviceError> {
         let mem_allocator = if private_caps.suballocation_supported {
             super::suballocation::create_allocator_wrapper(&raw)?
         } else {
@@ -48,6 +50,8 @@ impl super::Device {
         };
         hr.into_device_result("Idle fence creation")?;
 
+        null_comptr_check(&idle_fence)?;
+
         let mut zero_buffer = d3d12::Resource::null();
         unsafe {
             let raw_desc = d3d12_ty::D3D12_RESOURCE_DESC {
@@ -89,6 +93,8 @@ impl super::Device {
             )
             .into_device_result("Zero buffer creation")?;
 
+            null_comptr_check(&zero_buffer)?;
+
             // Note: without `D3D12_HEAP_FLAG_CREATE_NOT_ZEROED`
             // this resource is zeroed by default.
         };
@@ -142,7 +148,7 @@ impl super::Device {
         // A null pResource is used to initialize a null descriptor,
         // which guarantees D3D11-like null binding behavior (reading 0s, writes are discarded)
         raw.create_render_target_view(
-            d3d12::ComPtr::null(),
+            ComPtr::null(),
             &d3d12::RenderTargetViewDesc::texture_2d(
                 winapi::shared::dxgiformat::DXGI_FORMAT_R8G8B8A8_UNORM,
                 0,
@@ -185,10 +191,10 @@ impl super::Device {
     // Blocks until the dedicated present queue is finished with all of its work.
     //
     // Once this method completes, the surface is able to be resized or deleted.
-    pub(super) unsafe fn wait_for_present_queue_idle(&self) -> Result<(), crate::DeviceError> {
+    pub(super) unsafe fn wait_for_present_queue_idle(&self) -> Result<(), DeviceError> {
         let cur_value = self.idler.fence.get_value();
         if cur_value == !0 {
-            return Err(crate::DeviceError::Lost);
+            return Err(DeviceError::Lost);
         }
 
         let value = cur_value + 1;
@@ -326,7 +332,7 @@ impl crate::Device<super::Api> for super::Device {
     unsafe fn create_buffer(
         &self,
         desc: &crate::BufferDescriptor,
-    ) -> Result<super::Buffer, crate::DeviceError> {
+    ) -> Result<super::Buffer, DeviceError> {
         let mut resource = d3d12::Resource::null();
         let mut size = desc.size;
         if desc.usage.contains(crate::BufferUses::UNIFORM) {
@@ -381,11 +387,12 @@ impl crate::Device<super::Api> for super::Device {
         &self,
         buffer: &super::Buffer,
         range: crate::MemoryRange,
-    ) -> Result<crate::BufferMapping, crate::DeviceError> {
+    ) -> Result<crate::BufferMapping, DeviceError> {
         let mut ptr = ptr::null_mut();
         // TODO: 0 for subresource should be fine here until map and unmap buffer is subresource aware?
         let hr = unsafe { (*buffer.resource).Map(0, ptr::null(), &mut ptr) };
         hr.into_device_result("Map buffer")?;
+
         Ok(crate::BufferMapping {
             ptr: ptr::NonNull::new(unsafe { ptr.offset(range.start as isize).cast::<u8>() })
                 .unwrap(),
@@ -395,7 +402,7 @@ impl crate::Device<super::Api> for super::Device {
         })
     }
 
-    unsafe fn unmap_buffer(&self, buffer: &super::Buffer) -> Result<(), crate::DeviceError> {
+    unsafe fn unmap_buffer(&self, buffer: &super::Buffer) -> Result<(), DeviceError> {
         unsafe { (*buffer.resource).Unmap(0, ptr::null()) };
         Ok(())
     }
@@ -406,7 +413,7 @@ impl crate::Device<super::Api> for super::Device {
     unsafe fn create_texture(
         &self,
         desc: &crate::TextureDescriptor,
-    ) -> Result<super::Texture, crate::DeviceError> {
+    ) -> Result<super::Texture, DeviceError> {
         use super::suballocation::create_texture_resource;
 
         let mut resource = d3d12::Resource::null();
@@ -465,7 +472,7 @@ impl crate::Device<super::Api> for super::Device {
         &self,
         texture: &super::Texture,
         desc: &crate::TextureViewDescriptor,
-    ) -> Result<super::TextureView, crate::DeviceError> {
+    ) -> Result<super::TextureView, DeviceError> {
         let view_desc = desc.to_internal(texture);
 
         Ok(super::TextureView {
@@ -591,7 +598,7 @@ impl crate::Device<super::Api> for super::Device {
     unsafe fn create_sampler(
         &self,
         desc: &crate::SamplerDescriptor,
-    ) -> Result<super::Sampler, crate::DeviceError> {
+    ) -> Result<super::Sampler, DeviceError> {
         let handle = self.sampler_pool.lock().alloc_handle()?;
 
         let reduction = match desc.compare {
@@ -633,7 +640,7 @@ impl crate::Device<super::Api> for super::Device {
     unsafe fn create_command_encoder(
         &self,
         desc: &crate::CommandEncoderDescriptor<super::Api>,
-    ) -> Result<super::CommandEncoder, crate::DeviceError> {
+    ) -> Result<super::CommandEncoder, DeviceError> {
         let allocator = self
             .raw
             .create_command_allocator(d3d12::CmdListType::Direct)
@@ -665,7 +672,7 @@ impl crate::Device<super::Api> for super::Device {
     unsafe fn create_bind_group_layout(
         &self,
         desc: &crate::BindGroupLayoutDescriptor,
-    ) -> Result<super::BindGroupLayout, crate::DeviceError> {
+    ) -> Result<super::BindGroupLayout, DeviceError> {
         let (mut num_buffer_views, mut num_samplers, mut num_texture_views) = (0, 0, 0);
         for entry in desc.entries.iter() {
             let count = entry.count.map_or(1, NonZeroU32::get);
@@ -714,7 +721,7 @@ impl crate::Device<super::Api> for super::Device {
     unsafe fn create_pipeline_layout(
         &self,
         desc: &crate::PipelineLayoutDescriptor<super::Api>,
-    ) -> Result<super::PipelineLayout, crate::DeviceError> {
+    ) -> Result<super::PipelineLayout, DeviceError> {
         use naga::back::hlsl;
         // Pipeline layouts are implemented as RootSignature for D3D12.
         //
@@ -1024,7 +1031,7 @@ impl crate::Device<super::Api> for super::Device {
             )
             .map_err(|e| {
                 log::error!("Unable to find serialization function: {:?}", e);
-                crate::DeviceError::Lost
+                DeviceError::Lost
             })?
             .into_device_result("Root signature serialization")?;
 
@@ -1033,7 +1040,7 @@ impl crate::Device<super::Api> for super::Device {
                 "Root signature serialization error: {:?}",
                 unsafe { error.as_c_str() }.to_str().unwrap()
             );
-            return Err(crate::DeviceError::Lost);
+            return Err(DeviceError::Lost);
         }
 
         let raw = self
@@ -1076,7 +1083,7 @@ impl crate::Device<super::Api> for super::Device {
     unsafe fn create_bind_group(
         &self,
         desc: &crate::BindGroupDescriptor<super::Api>,
-    ) -> Result<super::BindGroup, crate::DeviceError> {
+    ) -> Result<super::BindGroup, DeviceError> {
         let mut cpu_views = desc
             .layout
             .cpu_heap_views
@@ -1437,6 +1444,8 @@ impl crate::Device<super::Api> for super::Device {
         hr.into_result()
             .map_err(|err| crate::PipelineError::Linkage(shader_stages, err.into_owned()))?;
 
+        null_comptr_check(&raw)?;
+
         if let Some(name) = desc.label {
             let cwstr = conv::map_label(name);
             unsafe { raw.SetName(cwstr.as_ptr()) };
@@ -1474,6 +1483,8 @@ impl crate::Device<super::Api> for super::Device {
             crate::PipelineError::Linkage(wgt::ShaderStages::COMPUTE, err.into_owned())
         })?;
 
+        null_comptr_check(&raw)?;
+
         if let Some(name) = desc.label {
             let cwstr = conv::map_label(name);
             unsafe { raw.SetName(cwstr.as_ptr()) };
@@ -1489,7 +1500,7 @@ impl crate::Device<super::Api> for super::Device {
     unsafe fn create_query_set(
         &self,
         desc: &wgt::QuerySetDescriptor<crate::Label>,
-    ) -> Result<super::QuerySet, crate::DeviceError> {
+    ) -> Result<super::QuerySet, DeviceError> {
         let (heap_ty, raw_ty) = match desc.ty {
             wgt::QueryType::Occlusion => (
                 d3d12::QueryHeapType::Occlusion,
@@ -1510,6 +1521,8 @@ impl crate::Device<super::Api> for super::Device {
             .create_query_heap(heap_ty, desc.count, 0)
             .into_device_result("Query heap creation")?;
 
+        null_comptr_check(&raw)?;
+
         if let Some(label) = desc.label {
             let cwstr = conv::map_label(label);
             unsafe { raw.SetName(cwstr.as_ptr()) };
@@ -1519,7 +1532,7 @@ impl crate::Device<super::Api> for super::Device {
     }
     unsafe fn destroy_query_set(&self, _set: super::QuerySet) {}
 
-    unsafe fn create_fence(&self) -> Result<super::Fence, crate::DeviceError> {
+    unsafe fn create_fence(&self) -> Result<super::Fence, DeviceError> {
         let mut raw = d3d12::Fence::null();
         let hr = unsafe {
             self.raw.CreateFence(
@@ -1530,21 +1543,23 @@ impl crate::Device<super::Api> for super::Device {
             )
         };
         hr.into_device_result("Fence creation")?;
+        null_comptr_check(&raw)?;
+
         Ok(super::Fence { raw })
     }
     unsafe fn destroy_fence(&self, _fence: super::Fence) {}
     unsafe fn get_fence_value(
         &self,
         fence: &super::Fence,
-    ) -> Result<crate::FenceValue, crate::DeviceError> {
+    ) -> Result<crate::FenceValue, DeviceError> {
         Ok(unsafe { fence.raw.GetCompletedValue() })
     }
     unsafe fn wait(
         &self,
         fence: &super::Fence,
         value: crate::FenceValue,
         timeout_ms: u32,
-    ) -> Result<bool, crate::DeviceError> {
+    ) -> Result<bool, DeviceError> {
         let timeout_duration = Duration::from_millis(timeout_ms as u64);
 
         // We first check if the fence has already reached the value we're waiting for.
@@ -1602,15 +1617,15 @@ impl crate::Device<super::Api> for super::Device {
                 winbase::WAIT_OBJECT_0 => {}
                 winbase::WAIT_ABANDONED | winbase::WAIT_FAILED => {
                     log::error!("Wait failed!");
-                    break Err(crate::DeviceError::Lost);
+                    break Err(DeviceError::Lost);
                 }
                 winerror::WAIT_TIMEOUT => {
                     log::trace!("Wait timed out!");
                     break Ok(false);
                 }
                 other => {
                     log::error!("Unexpected wait status: 0x{:x}", other);
-                    break Err(crate::DeviceError::Lost);
+                    break Err(DeviceError::Lost);
                 }
             };
 
@@ -1664,7 +1679,7 @@ impl crate::Device<super::Api> for super::Device {
     unsafe fn create_acceleration_structure(
         &self,
         _desc: &crate::AccelerationStructureDescriptor,
-    ) -> Result<super::AccelerationStructure, crate::DeviceError> {
+    ) -> Result<super::AccelerationStructure, DeviceError> {
         // Create a D3D12 resource as per-usual.
         todo!()
     }

wgpu-hal/src/dx12/mod.rs

@@ -938,3 +938,15 @@ impl crate::Queue<Api> for Queue {
         (1_000_000_000.0 / frequency as f64) as f32
     }
 }
+
+/// A shorthand for producing a `ResourceCreationFailed` error if a ComPtr is null.
+#[inline]
+pub fn null_comptr_check<T: winapi::Interface>(
+    ptr: &d3d12::ComPtr<T>,
+) -> Result<(), crate::DeviceError> {
+    if d3d12::ComPtr::is_null(ptr) {
+        return Err(crate::DeviceError::ResourceCreationFailed);
+    }
+
+    Ok(())
+}

wgpu-hal/src/dx12/suballocation.rs

@@ -16,6 +16,7 @@ use placed as allocation;
 // This is the fast path using gpu_allocator to suballocate buffers and textures.
 #[cfg(feature = "windows_rs")]
 mod placed {
+    use crate::dx12::null_comptr_check;
     use d3d12::ComPtr;
     use parking_lot::Mutex;
     use std::ptr;
@@ -115,6 +116,8 @@ mod placed {
             )
         };
 
+        null_comptr_check(resource)?;
+
         Ok((hr, Some(AllocationWrapper { allocation })))
     }
 
@@ -162,6 +165,8 @@ mod placed {
             )
         };
 
+        null_comptr_check(resource)?;
+
         Ok((hr, Some(AllocationWrapper { allocation })))
     }
 
@@ -223,6 +228,7 @@ mod placed {
 // This is the older, slower path where it doesn't suballocate buffers.
 // Tracking issue for when it can be removed: https://github.com/gfx-rs/wgpu/issues/3207
 mod committed {
+    use crate::dx12::null_comptr_check;
     use d3d12::ComPtr;
     use parking_lot::Mutex;
     use std::ptr;
@@ -296,6 +302,8 @@ mod committed {
             )
         };
 
+        null_comptr_check(resource)?;
+
         Ok((hr, None))
     }
 
@@ -332,6 +340,8 @@ mod committed {
             )
         };
 
+        null_comptr_check(resource)?;
+
         Ok((hr, None))
     }