Skip to content

Commit d655017

Browse files
jimblandyjimblandy
authored
Implement "strict_asserts" feature in wgpu-core. (#2872)
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 `"strict_asserts"` feature causes `wgpu_core` to perform the same checks in release builds as it does in debug builds.
1 parent f7526ae commit d655017

File tree

10 files changed

+144
-82
lines changed

10 files changed

+144
-82
lines changed

‎CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,12 @@ the same every time it is rendered, we now warn if it is missing.
9090

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

93+
### Build Configuration
94+
95+
- Add the `"strict_asserts"` feature, to enable additional internal
96+
run-time validation in `wgpu-core`. By @jimblandy in
97+
[#2872](https://github.com/gfx-rs/wgpu/pull/2872).
98+
9399
## wgpu-0.13.2 (2022-07-13)
94100

95101
### Bug Fixes

‎deno_webgpu/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,5 @@ description = "WebGPU implementation for Deno"
1414
deno_core = "0.144.0"
1515
serde = { version = "1.0", features = ["derive"] }
1616
tokio = { version = "1.19", features = ["full"] }
17-
wgpu-core = { path = "../wgpu-core", features = ["trace", "replay", "serde"] }
17+
wgpu-core = { path = "../wgpu-core", features = ["trace", "replay", "serde", "strict_asserts"] }
1818
wgpu-types = { path = "../wgpu-types", features = ["trace", "replay", "serde"] }

‎player/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ features = ["replay"]
2929
[dependencies.wgc]
3030
path = "../wgpu-core"
3131
package = "wgpu-core"
32-
features = ["replay", "raw-window-handle"]
32+
features = ["replay", "raw-window-handle", "strict_asserts"]
3333

3434
[dev-dependencies]
3535
serde = "1"

‎wgpu-core/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ license = "MIT OR Apache-2.0"
1313

1414
[features]
1515
default = []
16+
# Apply run-time checks, even in release builds. These are in addition
17+
# to the validation carried out at public APIs in all builds.
18+
strict_asserts = []
1619
angle = ["hal/gles"]
1720
# Enable API tracing
1821
trace = ["ron", "serde", "wgt/trace", "arrayvec/serde", "naga/serialize"]

‎wgpu-core/src/assertions.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
//! Macros for validation internal to the resource tracker.
2+
//!
3+
//! This module defines assertion macros that respect `wgpu-core`'s
4+
//! `"strict_asserts"` feature.
5+
//!
6+
//! Because `wgpu-core`'s public APIs validate their arguments in all
7+
//! types of builds, for performance, the `track` module skips some of
8+
//! Rust's usual run-time checks on its internal operations in release
9+
//! builds. However, some `wgpu-core` applications have a strong
10+
//! preference for robustness over performance. To accommodate them,
11+
//! `wgpu-core`'s `"strict_asserts"` feature enables that validation
12+
//! in both debug and release builds.
13+
14+
#[cfg(feature = "strict_asserts")]
15+
macro_rules! strict_assert {
16+
( $( $arg:tt )* ) => {
17+
assert!( $( $arg )* )
18+
}
19+
}
20+
21+
#[cfg(feature = "strict_asserts")]
22+
macro_rules! strict_assert_eq {
23+
( $( $arg:tt )* ) => {
24+
assert_eq!( $( $arg )* )
25+
}
26+
}
27+
28+
#[cfg(feature = "strict_asserts")]
29+
macro_rules! strict_assert_ne {
30+
( $( $arg:tt )* ) => {
31+
assert_ne!( $( $arg )* )
32+
}
33+
}
34+
35+
#[cfg(not(feature = "strict_asserts"))]
36+
#[macro_export]
37+
macro_rules! strict_assert {
38+
( $( $arg:tt )* ) => {};
39+
}
40+
41+
#[cfg(not(feature = "strict_asserts"))]
42+
macro_rules! strict_assert_eq {
43+
( $( $arg:tt )* ) => {};
44+
}
45+
46+
#[cfg(not(feature = "strict_asserts"))]
47+
macro_rules! strict_assert_ne {
48+
( $( $arg:tt )* ) => {};
49+
}

‎wgpu-core/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@
3333
clippy::pattern_type_mismatch,
3434
)]
3535

36+
#[macro_use]
37+
mod assertions;
38+
3639
pub mod binding_model;
3740
pub mod command;
3841
mod conv;

‎wgpu-core/src/track/buffer.rs

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,9 @@ impl<A: hub::HalApi> BufferUsageScope<A> {
101101
}
102102
}
103103

104-
fn debug_assert_in_bounds(&self, index: usize) {
105-
debug_assert!(index < self.state.len());
106-
self.metadata.debug_assert_in_bounds(index);
104+
fn tracker_assert_in_bounds(&self, index: usize) {
105+
strict_assert!(index < self.state.len());
106+
self.metadata.tracker_assert_in_bounds(index);
107107
}
108108

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

181181
for index in iterate_bitvec_indices(&scope.metadata.owned) {
182-
self.debug_assert_in_bounds(index);
183-
scope.debug_assert_in_bounds(index);
182+
self.tracker_assert_in_bounds(index);
183+
scope.tracker_assert_in_bounds(index);
184184

185185
unsafe {
186186
insert_or_merge(
@@ -225,7 +225,7 @@ impl<A: hub::HalApi> BufferUsageScope<A> {
225225

226226
self.allow_index(index);
227227

228-
self.debug_assert_in_bounds(index);
228+
self.tracker_assert_in_bounds(index);
229229

230230
unsafe {
231231
insert_or_merge(
@@ -265,10 +265,10 @@ impl<A: hub::HalApi> BufferTracker<A> {
265265
}
266266
}
267267

268-
fn debug_assert_in_bounds(&self, index: usize) {
269-
debug_assert!(index < self.start.len());
270-
debug_assert!(index < self.end.len());
271-
self.metadata.debug_assert_in_bounds(index);
268+
fn tracker_assert_in_bounds(&self, index: usize) {
269+
strict_assert!(index < self.start.len());
270+
strict_assert!(index < self.end.len());
271+
self.metadata.tracker_assert_in_bounds(index);
272272
}
273273

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

312312
self.allow_index(index);
313313

314-
self.debug_assert_in_bounds(index);
314+
self.tracker_assert_in_bounds(index);
315315

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

357357
self.allow_index(index);
358358

359-
self.debug_assert_in_bounds(index);
359+
self.tracker_assert_in_bounds(index);
360360

361361
unsafe {
362362
insert_or_barrier_update(
@@ -373,7 +373,7 @@ impl<A: hub::HalApi> BufferTracker<A> {
373373
)
374374
};
375375

376-
debug_assert!(self.temp.len() <= 1);
376+
strict_assert!(self.temp.len() <= 1);
377377

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

395395
for index in iterate_bitvec_indices(&tracker.metadata.owned) {
396-
self.debug_assert_in_bounds(index);
397-
tracker.debug_assert_in_bounds(index);
396+
self.tracker_assert_in_bounds(index);
397+
tracker.tracker_assert_in_bounds(index);
398398
unsafe {
399399
insert_or_barrier_update(
400400
None,
@@ -433,8 +433,8 @@ impl<A: hub::HalApi> BufferTracker<A> {
433433
}
434434

435435
for index in iterate_bitvec_indices(&scope.metadata.owned) {
436-
self.debug_assert_in_bounds(index);
437-
scope.debug_assert_in_bounds(index);
436+
self.tracker_assert_in_bounds(index);
437+
scope.tracker_assert_in_bounds(index);
438438
unsafe {
439439
insert_or_barrier_update(
440440
None,
@@ -488,7 +488,7 @@ impl<A: hub::HalApi> BufferTracker<A> {
488488
let (index32, _, _) = id.0.unzip();
489489
let index = index32 as usize;
490490

491-
scope.debug_assert_in_bounds(index);
491+
scope.tracker_assert_in_bounds(index);
492492

493493
if !scope.metadata.owned.get(index).unwrap_unchecked() {
494494
continue;
@@ -529,7 +529,7 @@ impl<A: hub::HalApi> BufferTracker<A> {
529529
return false;
530530
}
531531

532-
self.debug_assert_in_bounds(index);
532+
self.tracker_assert_in_bounds(index);
533533

534534
unsafe {
535535
if self.metadata.owned.get(index).unwrap_unchecked() {
@@ -569,7 +569,7 @@ impl BufferStateProvider<'_> {
569569
match *self {
570570
BufferStateProvider::Direct { state } => state,
571571
BufferStateProvider::Indirect { state } => {
572-
debug_assert!(index < state.len());
572+
strict_assert!(index < state.len());
573573
*state.get_unchecked(index)
574574
}
575575
}
@@ -695,8 +695,8 @@ unsafe fn insert<A: hub::HalApi>(
695695

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

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

‎wgpu-core/src/track/mod.rs

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -143,13 +143,13 @@ impl PendingTransition<hal::TextureUses> {
143143
let texture = tex.inner.as_raw().expect("Texture is destroyed");
144144

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

149149
let mip_count = self.selector.mips.end - self.selector.mips.start;
150-
debug_assert_ne!(mip_count, 0);
150+
strict_assert_ne!(mip_count, 0);
151151
let layer_count = self.selector.layers.end - self.selector.layers.start;
152-
debug_assert_ne!(layer_count, 0);
152+
strict_assert_ne!(layer_count, 0);
153153

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

359-
debug_assert!(if self.owned.get(index).unwrap() {
360+
strict_assert!(if self.owned.get(index).unwrap() {
360361
self.ref_counts[index].is_some()
361362
} else {
362363
true
@@ -373,7 +374,7 @@ impl<A: hub::HalApi> ResourceMetadata<A> {
373374
/// Returns ids for all resources we own.
374375
fn used<Id: TypedId>(&self) -> impl Iterator<Item = id::Valid<Id>> + '_ {
375376
if !self.owned.is_empty() {
376-
self.debug_assert_in_bounds(self.owned.len() - 1)
377+
self.tracker_assert_in_bounds(self.owned.len() - 1)
377378
};
378379
iterate_bitvec_indices(&self.owned).map(move |index| {
379380
let epoch = unsafe { *self.epochs.get_unchecked(index) };
@@ -418,7 +419,7 @@ impl<A: hub::HalApi> ResourceMetadataProvider<'_, A> {
418419
(epoch, ref_count.into_owned())
419420
}
420421
ResourceMetadataProvider::Indirect { metadata } => {
421-
metadata.debug_assert_in_bounds(index);
422+
metadata.tracker_assert_in_bounds(index);
422423
(
423424
*metadata.epochs.get_unchecked(index),
424425
metadata
@@ -429,7 +430,7 @@ impl<A: hub::HalApi> ResourceMetadataProvider<'_, A> {
429430
)
430431
}
431432
ResourceMetadataProvider::Resource { epoch } => {
432-
debug_assert!(life_guard.is_some());
433+
strict_assert!(life_guard.is_some());
433434
(epoch, life_guard.unwrap_unchecked().add_ref())
434435
}
435436
}
@@ -445,7 +446,7 @@ impl<A: hub::HalApi> ResourceMetadataProvider<'_, A> {
445446
ResourceMetadataProvider::Direct { epoch, .. }
446447
| ResourceMetadataProvider::Resource { epoch, .. } => epoch,
447448
ResourceMetadataProvider::Indirect { metadata } => {
448-
metadata.debug_assert_in_bounds(index);
449+
metadata.tracker_assert_in_bounds(index);
449450
*metadata.epochs.get_unchecked(index)
450451
}
451452
}

‎wgpu-core/src/track/stateless.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ impl<A: hub::HalApi, T: hub::Resource, Id: TypedId> StatelessTracker<A, T, Id> {
7070
}
7171
}
7272

73-
fn debug_assert_in_bounds(&self, index: usize) {
74-
self.metadata.debug_assert_in_bounds(index);
73+
fn tracker_assert_in_bounds(&self, index: usize) {
74+
self.metadata.tracker_assert_in_bounds(index);
7575
}
7676

7777
/// 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> {
106106

107107
self.allow_index(index);
108108

109-
self.debug_assert_in_bounds(index);
109+
self.tracker_assert_in_bounds(index);
110110

111111
unsafe {
112112
*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> {
127127

128128
self.allow_index(index);
129129

130-
self.debug_assert_in_bounds(index);
130+
self.tracker_assert_in_bounds(index);
131131

132132
unsafe {
133133
*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> {
149149
}
150150

151151
for index in iterate_bitvec_indices(&other.metadata.owned) {
152-
self.debug_assert_in_bounds(index);
153-
other.debug_assert_in_bounds(index);
152+
self.tracker_assert_in_bounds(index);
153+
other.tracker_assert_in_bounds(index);
154154
unsafe {
155155
let previously_owned = self.metadata.owned.get(index).unwrap_unchecked();
156156

@@ -187,7 +187,7 @@ impl<A: hub::HalApi, T: hub::Resource, Id: TypedId> StatelessTracker<A, T, Id> {
187187
return false;
188188
}
189189

190-
self.debug_assert_in_bounds(index);
190+
self.tracker_assert_in_bounds(index);
191191

192192
unsafe {
193193
if self.metadata.owned.get(index).unwrap_unchecked() {

0 commit comments

Comments
 (0)