Skip to content

Commit

Permalink
Change push_next() to unsafe fn extend() and add push() alterna…
Browse files Browse the repository at this point in the history
…tive (#909)

`push_next()` is totally `unsafe` because of dereferencing a chain of
`p_next` pointers to find the end of the chain to insert, which was
obfuscated by a large `unsafe` block for the `BaseOutStructure` pointer
cast in commit c8c8f69 ("`next` can contain a pointer chain and we need
to correct insert it.").

While this function should definitely be marked unsafe, wrapping
builders in `unsafe {}` en masse in user code isn't all too desirable,
especially when this soundness issue only exists to optionally walk
a `p_next` chain while most users are likely inserting bare structs
without pointer chains most of the time.  `push()` is introduced for
this reason, remaining safe to call without any unintended raw pointer
dereferences.  `push_next()` has been renamed to `extend()` to more
closely align with standard naming, and force existing users to notice
this change and hopefully analyze their uses to decide which are
safe/correct to switch over to safe `push()`.
  • Loading branch information
MarijnS95 authored Dec 6, 2024
1 parent 7dcf57b commit 986d7bb
Show file tree
Hide file tree
Showing 5 changed files with 4,743 additions and 925 deletions.
2 changes: 2 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- Added `push()` method to all root structs to insert a single extension-struct in the pointer chain. (#909)
- Update Vulkan-Headers to 1.3.296 (#910)
- Added `VK_KHR_get_display_properties2` instance extension (#932)
- Added `VK_EXT_metal_objects` device extension (#942)
Expand All @@ -24,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `get_pipeline_executable_properties()`;
- `get_pipeline_executable_statistics()`.
The expected length of this array can be queried with the respective `*_len()` variant of these functions.
- `push_next()` has been renamed to `extend()` and marked as `unsafe`. Users are encouraged to call `push()` for singular structs instead. (#909)

## [0.38.0] - 2024-04-01

Expand Down
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,19 +91,19 @@ let device: Device = instance

### Pointer chains

Use `base.push_next(ext)` to insert `ext` at the front of the pointer chain attached to `base`.
Use `base.push(ext)` to insert `ext` at the front of the pointer chain attached to `base`. If `ext` already contains a valid pointer chain of its own, `unsafe`ly call `extend()` instead.

```rust
let mut variable_pointers = vk::PhysicalDeviceVariablePointerFeatures::default();
let mut corner = vk::PhysicalDeviceCornerSampledImageFeaturesNV::default();

let mut device_create_info = vk::DeviceCreateInfo::default()
.push_next(&mut corner)
.push_next(&mut variable_pointers);
.push(&mut corner)
.push(&mut variable_pointers);
```

The generic argument of `.push_next()` only allows valid structs to extend a given struct (known as [`structextends` in the Vulkan registry](https://registry.khronos.org/vulkan/specs/1.3/styleguide.html#extensions-interactions), mapped to `Extends*` traits).
Only structs that are listed one or more times in any `structextends` will implement a `.push_next()`.
The generic argument of `.push()` only allows valid structs to extend a given struct (known as [`structextends` in the Vulkan registry](https://registry.khronos.org/vulkan/specs/1.3/styleguide.html#extensions-interactions), mapped to `Extends*` traits).
Only structs that are listed one or more times in any `structextends` will implement a `.push()`.

### Flags and constants as associated constants

Expand Down
4 changes: 2 additions & 2 deletions ash/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ mod tests {
<*mut _>::cast(&mut corner),
];
let mut device_create_info = vk::DeviceCreateInfo::default()
.push_next(&mut corner)
.push_next(&mut variable_pointers);
.push(&mut corner)
.push(&mut variable_pointers);
let chain2: Vec<*mut vk::BaseOutStructure<'_>> = unsafe {
vk::ptr_chain_iter(&mut device_create_info)
.skip(1)
Expand Down
Loading

0 comments on commit 986d7bb

Please sign in to comment.