Implement "tracker_asserts" feature in wgpu-core.

Since `wgpu-core`'s public functions are supposed to validate their parameters, the internal `track` module skips many of Rust's usual run-time checks in release builds. However, some `wgpu-core` users are happy to pay the performance cost in exchange for more safety. The `"tracker_asserts"` feature causes the `track` module to perform the same checks in release builds as it does in debug builds.
gfx-rs · Jul 12, 2022 · fde61d6 · fde61d6
1 parent a108efe
commit fde61d6
Show file tree

Hide file tree

Showing 9 changed files with 145 additions and 82 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -70,6 +70,12 @@ Bottom level categories:
 
 - Document safety requirements for `Adapter::from_external` in gles hal by @i509VCB in [#2863](https://github.com/gfx-rs/wgpu/pull/2863)
 
+### Build Configuration
+
+- Add the `"tracker_asserts"` feature, to enable additional internal
+  run-time validation in `wgpu-core`'s resource-tracking code. By
+  @jimblandy in [#2872](https://github.com/gfx-rs/wgpu/pull/2872).
+
 ## wgpu-0.13.1 (2022-07-02)
 
 ### Bug Fixes

diff --git a/deno_webgpu/Cargo.toml b/deno_webgpu/Cargo.toml
@@ -14,5 +14,5 @@ description = "WebGPU implementation for Deno"
 deno_core = "0.135.0"
 serde = { version = "1.0", features = ["derive"] }
 tokio = { version = "1.17", features = ["full"] }
-wgpu-core = { path = "../wgpu-core", features = ["trace", "replay", "serde"] }
+wgpu-core = { path = "../wgpu-core", features = ["trace", "replay", "serde", "tracker_asserts"] }
 wgpu-types = { path = "../wgpu-types", features = ["trace", "replay", "serde"] }
diff --git a/player/Cargo.toml b/player/Cargo.toml
@@ -29,7 +29,7 @@ features = ["replay"]
 [dependencies.wgc]
 path = "../wgpu-core"
 package = "wgpu-core"
-features = ["replay", "raw-window-handle"]
+features = ["replay", "raw-window-handle", "tracker_asserts"]
 
 [dev-dependencies]
 serde = "1"
diff --git a/wgpu-core/Cargo.toml b/wgpu-core/Cargo.toml
@@ -13,6 +13,10 @@ license = "MIT OR Apache-2.0"
 
 [features]
 default = []
+# Apply run-time checks in the resource tracker, even in release builds.
+# These are in addition to the validation carried out at public APIs
+# in all builds.
+tracker_asserts = []
 angle = ["hal/gles"]
 # Enable API tracing
 trace = ["ron", "serde", "wgt/trace", "arrayvec/serde", "naga/serialize"]

diff --git a/wgpu-core/src/track/assertions.rs b/wgpu-core/src/track/assertions.rs
@@ -0,0 +1,49 @@
+//! Macros for validation internal to the resource tracker.
+//!
+//! This module defines assertion macros that respect `wgpu-core`'s
+//! `"tracker_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 `"tracker_asserts"` feature enables that validation
+//! in both debug and release builds.
+
+#[cfg(feature = "tracker_asserts")]
+macro_rules! tracker_assert {
+    ( $( $arg:tt )* ) => {
+        assert!( $( $arg )* )
+    }
+}
+
+#[cfg(feature = "tracker_asserts")]
+macro_rules! tracker_assert_eq {
+    ( $( $arg:tt )* ) => {
+        assert_eq!( $( $arg )* )
+    }
+}
+
+#[cfg(feature = "tracker_asserts")]
+macro_rules! tracker_assert_ne {
+    ( $( $arg:tt )* ) => {
+        assert_ne!( $( $arg )* )
+    }
+}
+
+#[cfg(not(feature = "tracker_asserts"))]
+#[macro_export]
+macro_rules! tracker_assert {
+    ( $( $arg:tt )* ) => {};
+}
+
+#[cfg(not(feature = "tracker_asserts"))]
+macro_rules! tracker_assert_eq {
+    ( $( $arg:tt )* ) => {};
+}
+
+#[cfg(not(feature = "tracker_asserts"))]
+macro_rules! tracker_assert_ne {
+    ( $( $arg:tt )* ) => {};
+}
diff --git a/wgpu-core/src/track/buffer.rs b/wgpu-core/src/track/buffer.rs
@@ -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) {
+        tracker_assert!(index < self.state.len());
+        self.metadata.tracker_assert_in_bounds(index);
     }
 
     /// Sets the size of all the vectors inside the tracker.
@@ -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(
@@ -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(
@@ -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) {
+        tracker_assert!(index < self.start.len());
+        tracker_assert!(index < self.end.len());
+        self.metadata.tracker_assert_in_bounds(index);
     }
 
     /// Sets the size of all the vectors inside the tracker.
@@ -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();
@@ -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(
@@ -373,7 +373,7 @@ impl<A: hub::HalApi> BufferTracker<A> {
             )
         };
 
-        debug_assert!(self.temp.len() <= 1);
+        tracker_assert!(self.temp.len() <= 1);
 
         Some((value, self.temp.pop()))
     }
@@ -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,
@@ -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,
@@ -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;
@@ -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() {
@@ -569,7 +569,7 @@ impl BufferStateProvider<'_> {
         match *self {
             BufferStateProvider::Direct { state } => state,
             BufferStateProvider::Indirect { state } => {
-                debug_assert!(index < state.len());
+                tracker_assert!(index < state.len());
                 *state.get_unchecked(index)
             }
         }
@@ -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);
+    tracker_assert_eq!(invalid_resource_state(new_start_state), false);
+    tracker_assert_eq!(invalid_resource_state(new_end_state), false);
 
     log::trace!("\tbuf {index}: insert {new_start_state:?}..{new_end_state:?}");
 

diff --git a/wgpu-core/src/track/mod.rs b/wgpu-core/src/track/mod.rs
@@ -89,6 +89,9 @@
  * [`UsageScope`]: https://gpuweb.github.io/gpuweb/#programming-model-synchronization
 !*/
 
+#[macro_use]
+mod assertions;
+
 mod buffer;
 mod range;
 mod stateless;
@@ -143,13 +146,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);
+        tracker_assert_ne!(self.usage.start, hal::TextureUses::UNKNOWN);
+        tracker_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);
+        tracker_assert_ne!(mip_count, 0);
         let layer_count = self.selector.layers.end - self.selector.layers.start;
-        debug_assert_ne!(layer_count, 0);
+        tracker_assert_ne!(layer_count, 0);
 
         hal::TextureBarrier {
             texture,
@@ -351,12 +354,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 = "tracker_asserts"), allow(unused_variables))]
+    fn tracker_assert_in_bounds(&self, index: usize) {
+        tracker_assert!(index < self.owned.len());
+        tracker_assert!(index < self.ref_counts.len());
+        tracker_assert!(index < self.epochs.len());
 
-        debug_assert!(if self.owned.get(index).unwrap() {
+        tracker_assert!(if self.owned.get(index).unwrap() {
             self.ref_counts[index].is_some()
         } else {
             true
@@ -373,7 +377,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) };
@@ -418,7 +422,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
@@ -429,7 +433,7 @@ impl<A: hub::HalApi> ResourceMetadataProvider<'_, A> {
                 )
             }
             ResourceMetadataProvider::Resource { epoch } => {
-                debug_assert!(life_guard.is_some());
+                tracker_assert!(life_guard.is_some());
                 (epoch, life_guard.unwrap_unchecked().add_ref())
             }
         }
@@ -445,7 +449,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)
             }
         }

diff --git a/wgpu-core/src/track/stateless.rs b/wgpu-core/src/track/stateless.rs
@@ -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.
@@ -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;
@@ -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;
@@ -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();
 
@@ -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() {