vulkan: Don't use pointer to dropped PhysicalDeviceDriverProperties.

CHANGELOG.md

@@ -57,6 +57,10 @@ Bottom level categories:
 
 - Update the `minimum supported rust version` to 1.62
 
+#### Vulkan
+
+- Don't use a pointer to a local copy of a `PhysicalDeviceDriverProperties` struct after it has gone out of scope. In fact, don't make a local copy at all. Introduce a helper function for building `CStr`s from C character arrays, and remove some `unsafe` blocks. By @jimblandy in [#3076](https://github.com/gfx-rs/wgpu/pull/3076).
+
 ## wgpu-0.14.0 (2022-10-05)
 
 ### Major Changes

wgpu-hal/src/auxil/mod.rs

@@ -115,3 +115,24 @@ impl crate::TextureCopy {
         self.size = self.size.min(&max_src_size).min(&max_dst_size);
     }
 }
+
+/// Construct a `CStr` from a byte slice, up to the first zero byte.
+///
+/// Return a `CStr` extending from the start of `bytes` up to and
+/// including the first zero byte. If there is no zero byte in
+/// `bytes`, return `None`.
+///
+/// This can be removed when `CStr::from_bytes_until_nul` is stabilized.
+/// ([#95027](https://github.com/rust-lang/rust/issues/95027))
+#[allow(dead_code)]
+pub(crate) fn cstr_from_bytes_until_nul(bytes: &[std::os::raw::c_char]) -> Option<&std::ffi::CStr> {
+    if bytes.contains(&0) {
+        // Safety for `CStr::from_ptr`:
+        // - We've ensured that the slice does contain a null terminator.
+        // - The range is valid to read, because the slice covers it.
+        // - The memory won't be changed, because the slice borrows it.
+        unsafe { Some(std::ffi::CStr::from_ptr(bytes.as_ptr())) }
+    } else {
+        None
+    }
+}

wgpu-hal/src/vulkan/adapter.rs

@@ -538,9 +538,10 @@ impl PhysicalDeviceCapabilities {
     }
 
     pub fn supports_extension(&self, extension: &CStr) -> bool {
+        use crate::auxil::cstr_from_bytes_until_nul;
         self.supported_extensions
             .iter()
-            .any(|ep| unsafe { CStr::from_ptr(ep.extension_name.as_ptr()) } == extension)
+            .any(|ep| cstr_from_bytes_until_nul(&ep.extension_name) == Some(extension))
     }
 
     /// Map `requested_features` to the list of Vulkan extension strings required to create the logical device.
@@ -885,14 +886,15 @@ impl super::Instance {
         &self,
         phd: vk::PhysicalDevice,
     ) -> Option<crate::ExposedAdapter<super::Api>> {
+        use crate::auxil::cstr_from_bytes_until_nul;
         use crate::auxil::db;
 
         let (phd_capabilities, phd_features) = self.shared.inspect(phd);
 
         let info = wgt::AdapterInfo {
-            name: unsafe {
-                CStr::from_ptr(phd_capabilities.properties.device_name.as_ptr())
-                    .to_str()
+            name: {
+                cstr_from_bytes_until_nul(&phd_capabilities.properties.device_name)
+                    .and_then(|info| info.to_str().ok())
                     .unwrap_or("?")
                     .to_owned()
             },
@@ -906,23 +908,23 @@ impl super::Instance {
                 ash::vk::PhysicalDeviceType::CPU => wgt::DeviceType::Cpu,
                 _ => wgt::DeviceType::Other,
             },
-            driver: unsafe {
-                let driver_name = if let Some(driver) = phd_capabilities.driver {
-                    CStr::from_ptr(driver.driver_name.as_ptr()).to_str().ok()
-                } else {
-                    None
-                };
-
-                driver_name.unwrap_or("?").to_owned()
+            driver: {
+                phd_capabilities
+                    .driver
+                    .as_ref()
+                    .and_then(|driver| cstr_from_bytes_until_nul(&driver.driver_name))
+                    .and_then(|name| name.to_str().ok())
+                    .unwrap_or("?")
+                    .to_owned()
             },
-            driver_info: unsafe {
-                let driver_info = if let Some(driver) = phd_capabilities.driver {
-                    CStr::from_ptr(driver.driver_info.as_ptr()).to_str().ok()
-                } else {
-                    None
-                };
-
-                driver_info.unwrap_or("?").to_owned()
+            driver_info: {
+                phd_capabilities
+                    .driver
+                    .as_ref()
+                    .and_then(|driver| cstr_from_bytes_until_nul(&driver.driver_info))
+                    .and_then(|name| name.to_str().ok())
+                    .unwrap_or("?")
+                    .to_owned()
             },
             backend: wgt::Backend::Vulkan,
         };

wgpu-hal/src/vulkan/instance.rs

@@ -201,10 +201,9 @@ impl super::Instance {
 
         // Only keep available extensions.
         extensions.retain(|&ext| {
-            if instance_extensions
-                .iter()
-                .any(|inst_ext| unsafe { CStr::from_ptr(inst_ext.extension_name.as_ptr()) == ext })
-            {
+            if instance_extensions.iter().any(|inst_ext| {
+                crate::auxil::cstr_from_bytes_until_nul(&inst_ext.extension_name) == Some(ext)
+            }) {
                 true
             } else {
                 log::info!("Unable to find extension: {}", ext.to_string_lossy());
@@ -483,6 +482,8 @@ impl Drop for super::InstanceShared {
 
 impl crate::Instance<super::Api> for super::Instance {
     unsafe fn init(desc: &crate::InstanceDescriptor) -> Result<Self, crate::InstanceError> {
+        use crate::auxil::cstr_from_bytes_until_nul;
+
         let entry = match ash::Entry::load() {
             Ok(entry) => entry,
             Err(err) => {
@@ -531,9 +532,9 @@ impl crate::Instance<super::Api> for super::Instance {
         })?;
 
         let nv_optimus_layer = CStr::from_bytes_with_nul(b"VK_LAYER_NV_optimus\0").unwrap();
-        let has_nv_optimus = instance_layers
-            .iter()
-            .any(|inst_layer| CStr::from_ptr(inst_layer.layer_name.as_ptr()) == nv_optimus_layer);
+        let has_nv_optimus = instance_layers.iter().any(|inst_layer| {
+            cstr_from_bytes_until_nul(&inst_layer.layer_name) == Some(nv_optimus_layer)
+        });
 
         // Check requested layers against the available layers
         let layers = {
@@ -544,10 +545,9 @@ impl crate::Instance<super::Api> for super::Instance {
 
             // Only keep available layers.
             layers.retain(|&layer| {
-                if instance_layers
-                    .iter()
-                    .any(|inst_layer| CStr::from_ptr(inst_layer.layer_name.as_ptr()) == layer)
-                {
+                if instance_layers.iter().any(|inst_layer| {
+                    cstr_from_bytes_until_nul(&inst_layer.layer_name) == Some(layer)
+                }) {
                     true
                 } else {
                     log::warn!("Unable to find layer: {}", layer.to_string_lossy());