From 9229dc72787ab40fcbd9e2b6d0b304e4b653e6ec Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Mon, 1 May 2023 13:40:35 +0200 Subject: [PATCH] Switch to `CStr::from_bytes_until_nul` on sized char arrays in structs Certain structs contain sized character arrays that are converted to `CStr` for debugging using unsafe `CStr::from_ptr(...as_ptr())`. There is no need to round-trip to a pointer and possibly read out of bounds if the NULL character index (string length) is instead scanned for manually on the slice or via the newly stabilized `CStr::from_bytes_until_nul()` since Rust 1.69 (which panics if no NULL char is found before the end of the slice). Unfortunately `unsafe` is still needed to cast the array from a `c_char` (`i8` on most platforms) to `u8`, which is what `from_bytes_until_nul()` accepts. --- .github/workflows/ci.yml | 8 ++-- Changelog.md | 2 +- README.md | 2 +- ash-rewrite/Cargo.toml | 2 +- ash-window/Cargo.toml | 2 +- ash-window/Changelog.md | 2 +- ash-window/README.md | 2 +- ash/Cargo.toml | 2 +- ash/src/vk/definitions.rs | 99 +++++++++++---------------------------- ash/src/vk/prelude.rs | 10 ++++ generator/src/lib.rs | 18 ++----- 11 files changed, 52 insertions(+), 97 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cd2915449..3264f69b0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,19 +11,19 @@ jobs: - run: cargo check --workspace --all-targets --all-features check_msrv: - name: Check ash MSRV (1.60.0) + name: Check ash MSRV (1.69.0) runs-on: ubuntu-latest steps: - uses: actions/checkout@v1 - - uses: dtolnay/rust-toolchain@1.60.0 + - uses: dtolnay/rust-toolchain@1.69.0 - run: cargo check -p ash -p ash-rewrite --all-features check_ash_window_msrv: - name: Check ash-window MSRV (1.64.0) + name: Check ash-window MSRV (1.69.0) runs-on: ubuntu-latest steps: - uses: actions/checkout@v1 - - uses: dtolnay/rust-toolchain@1.64.0 + - uses: dtolnay/rust-toolchain@1.69.0 - run: cargo check -p ash-window -p examples --all-features # TODO: add a similar job for the rewrite once that generates code diff --git a/Changelog.md b/Changelog.md index 55059a908..8886fffb4 100644 --- a/Changelog.md +++ b/Changelog.md @@ -30,7 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Replaced builders with lifetimes/setters directly on Vulkan structs (#602) - Inlined struct setters (#602) -- Bumped MSRV from 1.59 to 1.60 (#709) +- Bumped MSRV from 1.59 to 1.69 (#709, #746) - Replaced `const fn name()` with associated `NAME` constants (#715) - Generic builders now automatically set `objecttype` to `::ObjectType` (#724) - `get_calibrated_timestamps()` now returns a single value for `max_deviation` (#738) diff --git a/README.md b/README.md index b7c08f89e..151d58bfb 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ A very lightweight wrapper around Vulkan [![LICENSE](https://img.shields.io/badge/license-MIT-blue.svg)](LICENSE-MIT) [![LICENSE](https://img.shields.io/badge/license-Apache--2.0-blue.svg)](LICENSE-APACHE) [![Join the chat at https://gitter.im/MaikKlein/ash](https://badges.gitter.im/MaikKlein/ash.svg)](https://gitter.im/MaikKlein/ash?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) -[![MSRV](https://img.shields.io/badge/rustc-1.60.0+-ab6000.svg)](https://blog.rust-lang.org/2022/04/07/Rust-1.60.0.html) +[![MSRV](https://img.shields.io/badge/rustc-1.69.0+-ab6000.svg)](https://blog.rust-lang.org/2023/04/20/Rust-1.69.0.html) ## Overview diff --git a/ash-rewrite/Cargo.toml b/ash-rewrite/Cargo.toml index f84365ce1..f1dd6e1e4 100644 --- a/ash-rewrite/Cargo.toml +++ b/ash-rewrite/Cargo.toml @@ -16,7 +16,7 @@ categories = [ documentation = "https://docs.rs/ash" edition = "2021" # TODO: reevaluate, then update in ci.yml -rust-version = "1.60.0" +rust-version = "1.69.0" [dependencies] libloading = { version = "0.7", optional = true } diff --git a/ash-window/Cargo.toml b/ash-window/Cargo.toml index 76ad7c154..98dcda91e 100644 --- a/ash-window/Cargo.toml +++ b/ash-window/Cargo.toml @@ -12,7 +12,7 @@ categories = ["game-engines", "graphics"] exclude = [".github/*"] workspace = ".." edition = "2021" -rust-version = "1.64.0" +rust-version = "1.69.0" [dependencies] ash = { path = "../ash", version = "0.37", default-features = false } diff --git a/ash-window/Changelog.md b/ash-window/Changelog.md index ec2cbbf8f..df526e736 100644 --- a/ash-window/Changelog.md +++ b/ash-window/Changelog.md @@ -2,7 +2,7 @@ ## [Unreleased] - ReleaseDate -- Bumped MSRV from 1.59 to 1.64 for `winit 0.28` and `raw-window-handle 0.5.1`. (#709, #716) +- Bumped MSRV from 1.59 to 1.69 for `winit 0.28` and `raw-window-handle 0.5.1`, and `CStr::from_bytes_until_nul`. (#709, #716, #746) ## [0.12.0] - 2022-09-23 diff --git a/ash-window/README.md b/ash-window/README.md index 8cb3279f1..f4e97eb07 100644 --- a/ash-window/README.md +++ b/ash-window/README.md @@ -8,7 +8,7 @@ Interoperability between [`ash`](https://github.com/MaikKlein/ash) and [`raw-win [![LICENSE](https://img.shields.io/badge/license-MIT-blue.svg)](LICENSE-MIT) [![LICENSE](https://img.shields.io/badge/license-apache-blue.svg)](LICENSE-APACHE) [![Join the chat at https://gitter.im/MaikKlein/ash](https://badges.gitter.im/MaikKlein/ash.svg)](https://gitter.im/MaikKlein/ash?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) -[![MSRV](https://img.shields.io/badge/rustc-1.64.0+-ab6000.svg)](https://blog.rust-lang.org/2022/09/22/Rust-1.64.0.html) +[![MSRV](https://img.shields.io/badge/rustc-1.69.0+-ab6000.svg)](https://blog.rust-lang.org/2023/04/20/Rust-1.69.0.html) ## Usage diff --git a/ash/Cargo.toml b/ash/Cargo.toml index db8516987..46d7d7fda 100644 --- a/ash/Cargo.toml +++ b/ash/Cargo.toml @@ -13,7 +13,7 @@ readme = "../README.md" keywords = ["vulkan", "graphic"] documentation = "https://docs.rs/ash" edition = "2021" -rust-version = "1.60.0" +rust-version = "1.69.0" [dependencies] libloading = { version = "0.7", optional = true } diff --git a/ash/src/vk/definitions.rs b/ash/src/vk/definitions.rs index c5ab81ebd..e7714bc9e 100644 --- a/ash/src/vk/definitions.rs +++ b/ash/src/vk/definitions.rs @@ -817,9 +817,7 @@ impl fmt::Debug for PhysicalDeviceProperties { .field("vendor_id", &self.vendor_id) .field("device_id", &self.device_id) .field("device_type", &self.device_type) - .field("device_name", &unsafe { - ::std::ffi::CStr::from_ptr(self.device_name.as_ptr()) - }) + .field("device_name", &wrap_cstr_slice_until_nul(&self.device_name)) .field("pipeline_cache_uuid", &self.pipeline_cache_uuid) .field("limits", &self.limits) .field("sparse_properties", &self.sparse_properties) @@ -900,9 +898,10 @@ pub struct ExtensionProperties { impl fmt::Debug for ExtensionProperties { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { fmt.debug_struct("ExtensionProperties") - .field("extension_name", &unsafe { - ::std::ffi::CStr::from_ptr(self.extension_name.as_ptr()) - }) + .field( + "extension_name", + &wrap_cstr_slice_until_nul(&self.extension_name), + ) .field("spec_version", &self.spec_version) .finish() } @@ -941,14 +940,10 @@ pub struct LayerProperties { impl fmt::Debug for LayerProperties { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { fmt.debug_struct("LayerProperties") - .field("layer_name", &unsafe { - ::std::ffi::CStr::from_ptr(self.layer_name.as_ptr()) - }) + .field("layer_name", &wrap_cstr_slice_until_nul(&self.layer_name)) .field("spec_version", &self.spec_version) .field("implementation_version", &self.implementation_version) - .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr()) - }) + .field("description", &wrap_cstr_slice_until_nul(&self.description)) .finish() } } @@ -10900,12 +10895,8 @@ impl fmt::Debug for PhysicalDeviceDriverProperties<'_> { .field("s_type", &self.s_type) .field("p_next", &self.p_next) .field("driver_id", &self.driver_id) - .field("driver_name", &unsafe { - ::std::ffi::CStr::from_ptr(self.driver_name.as_ptr()) - }) - .field("driver_info", &unsafe { - ::std::ffi::CStr::from_ptr(self.driver_info.as_ptr()) - }) + .field("driver_name", &wrap_cstr_slice_until_nul(&self.driver_name)) + .field("driver_info", &wrap_cstr_slice_until_nul(&self.driver_info)) .field("conformance_version", &self.conformance_version) .finish() } @@ -27050,15 +27041,9 @@ impl fmt::Debug for PerformanceCounterDescriptionKHR<'_> { .field("s_type", &self.s_type) .field("p_next", &self.p_next) .field("flags", &self.flags) - .field("name", &unsafe { - ::std::ffi::CStr::from_ptr(self.name.as_ptr()) - }) - .field("category", &unsafe { - ::std::ffi::CStr::from_ptr(self.category.as_ptr()) - }) - .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr()) - }) + .field("name", &wrap_cstr_slice_until_nul(&self.name)) + .field("category", &wrap_cstr_slice_until_nul(&self.category)) + .field("description", &wrap_cstr_slice_until_nul(&self.description)) .finish() } } @@ -28137,12 +28122,8 @@ impl fmt::Debug for PipelineExecutablePropertiesKHR<'_> { .field("s_type", &self.s_type) .field("p_next", &self.p_next) .field("stages", &self.stages) - .field("name", &unsafe { - ::std::ffi::CStr::from_ptr(self.name.as_ptr()) - }) - .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr()) - }) + .field("name", &wrap_cstr_slice_until_nul(&self.name)) + .field("description", &wrap_cstr_slice_until_nul(&self.description)) .field("subgroup_size", &self.subgroup_size) .finish() } @@ -28257,12 +28238,8 @@ impl fmt::Debug for PipelineExecutableStatisticKHR<'_> { fmt.debug_struct("PipelineExecutableStatisticKHR") .field("s_type", &self.s_type) .field("p_next", &self.p_next) - .field("name", &unsafe { - ::std::ffi::CStr::from_ptr(self.name.as_ptr()) - }) - .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr()) - }) + .field("name", &wrap_cstr_slice_until_nul(&self.name)) + .field("description", &wrap_cstr_slice_until_nul(&self.description)) .field("format", &self.format) .field("value", &"union") .finish() @@ -28326,12 +28303,8 @@ impl fmt::Debug for PipelineExecutableInternalRepresentationKHR<'_> { fmt.debug_struct("PipelineExecutableInternalRepresentationKHR") .field("s_type", &self.s_type) .field("p_next", &self.p_next) - .field("name", &unsafe { - ::std::ffi::CStr::from_ptr(self.name.as_ptr()) - }) - .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr()) - }) + .field("name", &wrap_cstr_slice_until_nul(&self.name)) + .field("description", &wrap_cstr_slice_until_nul(&self.description)) .field("is_text", &self.is_text) .field("data_size", &self.data_size) .field("p_data", &self.p_data) @@ -29834,12 +29807,8 @@ impl fmt::Debug for PhysicalDeviceVulkan12Properties<'_> { .field("s_type", &self.s_type) .field("p_next", &self.p_next) .field("driver_id", &self.driver_id) - .field("driver_name", &unsafe { - ::std::ffi::CStr::from_ptr(self.driver_name.as_ptr()) - }) - .field("driver_info", &unsafe { - ::std::ffi::CStr::from_ptr(self.driver_info.as_ptr()) - }) + .field("driver_name", &wrap_cstr_slice_until_nul(&self.driver_name)) + .field("driver_info", &wrap_cstr_slice_until_nul(&self.driver_info)) .field("conformance_version", &self.conformance_version) .field( "denorm_behavior_independence", @@ -31190,19 +31159,11 @@ impl fmt::Debug for PhysicalDeviceToolProperties<'_> { fmt.debug_struct("PhysicalDeviceToolProperties") .field("s_type", &self.s_type) .field("p_next", &self.p_next) - .field("name", &unsafe { - ::std::ffi::CStr::from_ptr(self.name.as_ptr()) - }) - .field("version", &unsafe { - ::std::ffi::CStr::from_ptr(self.version.as_ptr()) - }) + .field("name", &wrap_cstr_slice_until_nul(&self.name)) + .field("version", &wrap_cstr_slice_until_nul(&self.version)) .field("purposes", &self.purposes) - .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr()) - }) - .field("layer", &unsafe { - ::std::ffi::CStr::from_ptr(self.layer.as_ptr()) - }) + .field("description", &wrap_cstr_slice_until_nul(&self.description)) + .field("layer", &wrap_cstr_slice_until_nul(&self.layer)) .finish() } } @@ -45383,9 +45344,7 @@ impl fmt::Debug for RenderPassSubpassFeedbackInfoEXT { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { fmt.debug_struct("RenderPassSubpassFeedbackInfoEXT") .field("subpass_merge_status", &self.subpass_merge_status) - .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr()) - }) + .field("description", &wrap_cstr_slice_until_nul(&self.description)) .field("post_merge_index", &self.post_merge_index) .finish() } @@ -48154,9 +48113,7 @@ pub struct DeviceFaultVendorInfoEXT { impl fmt::Debug for DeviceFaultVendorInfoEXT { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { fmt.debug_struct("DeviceFaultVendorInfoEXT") - .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr()) - }) + .field("description", &wrap_cstr_slice_until_nul(&self.description)) .field("vendor_fault_code", &self.vendor_fault_code) .field("vendor_fault_data", &self.vendor_fault_data) .finish() @@ -48252,9 +48209,7 @@ impl fmt::Debug for DeviceFaultInfoEXT<'_> { fmt.debug_struct("DeviceFaultInfoEXT") .field("s_type", &self.s_type) .field("p_next", &self.p_next) - .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr()) - }) + .field("description", &wrap_cstr_slice_until_nul(&self.description)) .field("p_address_infos", &self.p_address_infos) .field("p_vendor_infos", &self.p_vendor_infos) .field("p_vendor_binary_data", &self.p_vendor_binary_data) diff --git a/ash/src/vk/prelude.rs b/ash/src/vk/prelude.rs index cb2f17718..5de1f2c19 100644 --- a/ash/src/vk/prelude.rs +++ b/ash/src/vk/prelude.rs @@ -59,3 +59,13 @@ impl From for vk::Rect2D { pub unsafe trait TaggedStructure { const STRUCTURE_TYPE: vk::StructureType; } + +#[cfg(feature = "debug")] +pub(crate) fn wrap_cstr_slice_until_nul( + bytes: &[::core::ffi::c_char], +) -> Result<&::core::ffi::CStr, ::core::ffi::FromBytesUntilNulError> { + ::core::ffi::CStr::from_bytes_until_nul( + // SAFETY: The cast from c_char to u8 is ok because a c_char is always one byte. + unsafe { ::core::slice::from_raw_parts(bytes.as_ptr().cast(), bytes.len()) }, + ) +} diff --git a/generator/src/lib.rs b/generator/src/lib.rs index 2b897afac..373de57c2 100644 --- a/generator/src/lib.rs +++ b/generator/src/lib.rs @@ -1788,25 +1788,15 @@ fn derive_debug( let param_ident = field.param_ident(); let param_str = param_ident.to_string(); let debug_value = if is_static_array(field) && field.basetype == "char" { - quote! { - &unsafe { - ::std::ffi::CStr::from_ptr(self.#param_ident.as_ptr()) - } - } + quote!(&wrap_cstr_slice_until_nul(&self.#param_ident)) } else if param_str.contains("pfn") { - quote! { - &(self.#param_ident.map(|x| x as *const ())) - } + quote!(&(self.#param_ident.map(|x| x as *const ()))) } else if union_types.contains(field.basetype.as_str()) { quote!(&"union") } else { - quote! { - &self.#param_ident - } + quote!(&self.#param_ident) }; - quote! { - .field(#param_str, #debug_value) - } + quote!(.field(#param_str, #debug_value)) }); let name_str = name.to_string(); let lifetime = has_lifetime.then(|| quote!(<'_>));