Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement "strict_asserts" feature in wgpu-core. #2872

Merged
merged 1 commit into from
Aug 8, 2022
Merged

Implement "strict_asserts" feature in wgpu-core. #2872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,12 @@ the same every time it is rendered, we now warn if it is missing.

- Expanded `StagingBelt` documentation by @kpreid in [#2905](https://github.com/gfx-rs/wgpu/pull/2905)

### Build Configuration

- Add the `"strict_asserts"` feature, to enable additional internal
run-time validation in `wgpu-core`. By @jimblandy in
[#2872](https://github.com/gfx-rs/wgpu/pull/2872).

## wgpu-0.13.2 (2022-07-13)

### Bug Fixes
Expand Down
2 changes: 1 addition & 1 deletion deno_webgpu/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ description = "WebGPU implementation for Deno"
deno_core = "0.144.0"
serde = { version = "1.0", features = ["derive"] }
tokio = { version = "1.19", features = ["full"] }
wgpu-core = { path = "../wgpu-core", features = ["trace", "replay", "serde"] }
wgpu-core = { path = "../wgpu-core", features = ["trace", "replay", "serde", "strict_asserts"] }
wgpu-types = { path = "../wgpu-types", features = ["trace", "replay", "serde"] }
2 changes: 1 addition & 1 deletion player/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ features = ["replay"]
[dependencies.wgc]
path = "../wgpu-core"
package = "wgpu-core"
features = ["replay", "raw-window-handle"]
features = ["replay", "raw-window-handle", "strict_asserts"]

[dev-dependencies]
serde = "1"
3 changes: 3 additions & 0 deletions wgpu-core/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ license = "MIT OR Apache-2.0"

[features]
default = []
# 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 = []
angle = ["hal/gles"]
# Enable API tracing
trace = ["ron", "serde", "wgt/trace", "arrayvec/serde", "naga/serialize"]
Expand Down
49 changes: 49 additions & 0 deletions wgpu-core/src/assertions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
//! Macros for validation internal to the resource tracker.
//!
//! This module defines assertion macros that respect `wgpu-core`'s
//! `"strict_asserts"` feature.
//!
//! Because `wgpu-core`'s public APIs validate their arguments in all
//! types of builds, for performance, the `track` module skips some of
//! Rust's usual run-time checks on its internal operations in release
//! builds. However, some `wgpu-core` applications have a strong
//! preference for robustness over performance. To accommodate them,
//! `wgpu-core`'s `"strict_asserts"` feature enables that validation
//! in both debug and release builds.

#[cfg(feature = "strict_asserts")]
macro_rules! strict_assert {
( $( $arg:tt )* ) => {
assert!( $( $arg )* )
}
}

#[cfg(feature = "strict_asserts")]
macro_rules! strict_assert_eq {
( $( $arg:tt )* ) => {
assert_eq!( $( $arg )* )
}
}

#[cfg(feature = "strict_asserts")]
macro_rules! strict_assert_ne {
( $( $arg:tt )* ) => {
assert_ne!( $( $arg )* )
}
}

#[cfg(not(feature = "strict_asserts"))]
#[macro_export]
macro_rules! strict_assert {
( $( $arg:tt )* ) => {};
}

#[cfg(not(feature = "strict_asserts"))]
macro_rules! strict_assert_eq {
( $( $arg:tt )* ) => {};
}

#[cfg(not(feature = "strict_asserts"))]
macro_rules! strict_assert_ne {
( $( $arg:tt )* ) => {};
}
3 changes: 3 additions & 0 deletions wgpu-core/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
clippy::pattern_type_mismatch,
)]

#[macro_use]
mod assertions;

pub mod binding_model;
pub mod command;
mod conv;
Expand Down
44 changes: 22 additions & 22 deletions wgpu-core/src/track/buffer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ impl<A: hub::HalApi> BufferUsageScope<A> {
}
}

fn debug_assert_in_bounds(&self, index: usize) {
debug_assert!(index < self.state.len());
self.metadata.debug_assert_in_bounds(index);
fn tracker_assert_in_bounds(&self, index: usize) {
strict_assert!(index < self.state.len());
self.metadata.tracker_assert_in_bounds(index);
}

/// Sets the size of all the vectors inside the tracker.
Expand Down Expand Up @@ -179,8 +179,8 @@ impl<A: hub::HalApi> BufferUsageScope<A> {
}

for index in iterate_bitvec_indices(&scope.metadata.owned) {
self.debug_assert_in_bounds(index);
scope.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);
scope.tracker_assert_in_bounds(index);

unsafe {
insert_or_merge(
Expand Down Expand Up @@ -225,7 +225,7 @@ impl<A: hub::HalApi> BufferUsageScope<A> {

self.allow_index(index);

self.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);

unsafe {
insert_or_merge(
Expand Down Expand Up @@ -265,10 +265,10 @@ impl<A: hub::HalApi> BufferTracker<A> {
}
}

fn debug_assert_in_bounds(&self, index: usize) {
debug_assert!(index < self.start.len());
debug_assert!(index < self.end.len());
self.metadata.debug_assert_in_bounds(index);
fn tracker_assert_in_bounds(&self, index: usize) {
strict_assert!(index < self.start.len());
strict_assert!(index < self.end.len());
self.metadata.tracker_assert_in_bounds(index);
}

/// Sets the size of all the vectors inside the tracker.
Expand Down Expand Up @@ -311,7 +311,7 @@ impl<A: hub::HalApi> BufferTracker<A> {

self.allow_index(index);

self.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);

unsafe {
let currently_owned = self.metadata.owned.get(index).unwrap_unchecked();
Expand Down Expand Up @@ -356,7 +356,7 @@ impl<A: hub::HalApi> BufferTracker<A> {

self.allow_index(index);

self.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);

unsafe {
insert_or_barrier_update(
Expand All @@ -373,7 +373,7 @@ impl<A: hub::HalApi> BufferTracker<A> {
)
};

debug_assert!(self.temp.len() <= 1);
strict_assert!(self.temp.len() <= 1);

Some((value, self.temp.pop()))
}
Expand All @@ -393,8 +393,8 @@ impl<A: hub::HalApi> BufferTracker<A> {
}

for index in iterate_bitvec_indices(&tracker.metadata.owned) {
self.debug_assert_in_bounds(index);
tracker.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);
tracker.tracker_assert_in_bounds(index);
unsafe {
insert_or_barrier_update(
None,
Expand Down Expand Up @@ -433,8 +433,8 @@ impl<A: hub::HalApi> BufferTracker<A> {
}

for index in iterate_bitvec_indices(&scope.metadata.owned) {
self.debug_assert_in_bounds(index);
scope.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);
scope.tracker_assert_in_bounds(index);
unsafe {
insert_or_barrier_update(
None,
Expand Down Expand Up @@ -488,7 +488,7 @@ impl<A: hub::HalApi> BufferTracker<A> {
let (index32, _, _) = id.0.unzip();
let index = index32 as usize;

scope.debug_assert_in_bounds(index);
scope.tracker_assert_in_bounds(index);

if !scope.metadata.owned.get(index).unwrap_unchecked() {
continue;
Expand Down Expand Up @@ -529,7 +529,7 @@ impl<A: hub::HalApi> BufferTracker<A> {
return false;
}

self.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);

unsafe {
if self.metadata.owned.get(index).unwrap_unchecked() {
Expand Down Expand Up @@ -569,7 +569,7 @@ impl BufferStateProvider<'_> {
match *self {
BufferStateProvider::Direct { state } => state,
BufferStateProvider::Indirect { state } => {
debug_assert!(index < state.len());
strict_assert!(index < state.len());
*state.get_unchecked(index)
}
}
Expand Down Expand Up @@ -695,8 +695,8 @@ unsafe fn insert<A: hub::HalApi>(

// This should only ever happen with a wgpu bug, but let's just double
// check that resource states don't have any conflicts.
debug_assert_eq!(invalid_resource_state(new_start_state), false);
debug_assert_eq!(invalid_resource_state(new_end_state), false);
strict_assert_eq!(invalid_resource_state(new_start_state), false);
strict_assert_eq!(invalid_resource_state(new_end_state), false);

log::trace!("\tbuf {index}: insert {new_start_state:?}..{new_end_state:?}");

Expand Down
27 changes: 14 additions & 13 deletions wgpu-core/src/track/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,13 @@ impl PendingTransition<hal::TextureUses> {
let texture = tex.inner.as_raw().expect("Texture is destroyed");

// These showing up in a barrier is always a bug
debug_assert_ne!(self.usage.start, hal::TextureUses::UNKNOWN);
debug_assert_ne!(self.usage.end, hal::TextureUses::UNKNOWN);
strict_assert_ne!(self.usage.start, hal::TextureUses::UNKNOWN);
strict_assert_ne!(self.usage.end, hal::TextureUses::UNKNOWN);

let mip_count = self.selector.mips.end - self.selector.mips.start;
debug_assert_ne!(mip_count, 0);
strict_assert_ne!(mip_count, 0);
let layer_count = self.selector.layers.end - self.selector.layers.start;
debug_assert_ne!(layer_count, 0);
strict_assert_ne!(layer_count, 0);

hal::TextureBarrier {
texture,
Expand Down Expand Up @@ -351,12 +351,13 @@ impl<A: hub::HalApi> ResourceMetadata<A> {
/// sanity checks of the presence of a refcount.
///
/// In release mode this function is completely empty and is removed.
fn debug_assert_in_bounds(&self, index: usize) {
debug_assert!(index < self.owned.len());
debug_assert!(index < self.ref_counts.len());
debug_assert!(index < self.epochs.len());
#[cfg_attr(not(feature = "strict_asserts"), allow(unused_variables))]
fn tracker_assert_in_bounds(&self, index: usize) {
strict_assert!(index < self.owned.len());
strict_assert!(index < self.ref_counts.len());
strict_assert!(index < self.epochs.len());

debug_assert!(if self.owned.get(index).unwrap() {
strict_assert!(if self.owned.get(index).unwrap() {
self.ref_counts[index].is_some()
} else {
true
Expand All @@ -373,7 +374,7 @@ impl<A: hub::HalApi> ResourceMetadata<A> {
/// Returns ids for all resources we own.
fn used<Id: TypedId>(&self) -> impl Iterator<Item = id::Valid<Id>> + '_ {
if !self.owned.is_empty() {
self.debug_assert_in_bounds(self.owned.len() - 1)
self.tracker_assert_in_bounds(self.owned.len() - 1)
};
iterate_bitvec_indices(&self.owned).map(move |index| {
let epoch = unsafe { *self.epochs.get_unchecked(index) };
Expand Down Expand Up @@ -418,7 +419,7 @@ impl<A: hub::HalApi> ResourceMetadataProvider<'_, A> {
(epoch, ref_count.into_owned())
}
ResourceMetadataProvider::Indirect { metadata } => {
metadata.debug_assert_in_bounds(index);
metadata.tracker_assert_in_bounds(index);
(
*metadata.epochs.get_unchecked(index),
metadata
Expand All @@ -429,7 +430,7 @@ impl<A: hub::HalApi> ResourceMetadataProvider<'_, A> {
)
}
ResourceMetadataProvider::Resource { epoch } => {
debug_assert!(life_guard.is_some());
strict_assert!(life_guard.is_some());
(epoch, life_guard.unwrap_unchecked().add_ref())
}
}
Expand All @@ -445,7 +446,7 @@ impl<A: hub::HalApi> ResourceMetadataProvider<'_, A> {
ResourceMetadataProvider::Direct { epoch, .. }
| ResourceMetadataProvider::Resource { epoch, .. } => epoch,
ResourceMetadataProvider::Indirect { metadata } => {
metadata.debug_assert_in_bounds(index);
metadata.tracker_assert_in_bounds(index);
*metadata.epochs.get_unchecked(index)
}
}
Expand Down
14 changes: 7 additions & 7 deletions wgpu-core/src/track/stateless.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ impl<A: hub::HalApi, T: hub::Resource, Id: TypedId> StatelessTracker<A, T, Id> {
}
}

fn debug_assert_in_bounds(&self, index: usize) {
self.metadata.debug_assert_in_bounds(index);
fn tracker_assert_in_bounds(&self, index: usize) {
self.metadata.tracker_assert_in_bounds(index);
}

/// Sets the size of all the vectors inside the tracker.
Expand Down Expand Up @@ -106,7 +106,7 @@ impl<A: hub::HalApi, T: hub::Resource, Id: TypedId> StatelessTracker<A, T, Id> {

self.allow_index(index);

self.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);

unsafe {
*self.metadata.epochs.get_unchecked_mut(index) = epoch;
Expand All @@ -127,7 +127,7 @@ impl<A: hub::HalApi, T: hub::Resource, Id: TypedId> StatelessTracker<A, T, Id> {

self.allow_index(index);

self.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);

unsafe {
*self.metadata.epochs.get_unchecked_mut(index) = epoch;
Expand All @@ -149,8 +149,8 @@ impl<A: hub::HalApi, T: hub::Resource, Id: TypedId> StatelessTracker<A, T, Id> {
}

for index in iterate_bitvec_indices(&other.metadata.owned) {
self.debug_assert_in_bounds(index);
other.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);
other.tracker_assert_in_bounds(index);
unsafe {
let previously_owned = self.metadata.owned.get(index).unwrap_unchecked();

Expand Down Expand Up @@ -187,7 +187,7 @@ impl<A: hub::HalApi, T: hub::Resource, Id: TypedId> StatelessTracker<A, T, Id> {
return false;
}

self.debug_assert_in_bounds(index);
self.tracker_assert_in_bounds(index);

unsafe {
if self.metadata.owned.get(index).unwrap_unchecked() {
Expand Down
Loading