Skip to content

Commit 4a415f4

Browse files
committed
Fix naga panic and validate pipeline auto layout when immediate size overflows 256 bytes
1 parent b02ef5a commit 4a415f4

12 files changed

Lines changed: 89 additions & 35 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ By @beholdnec in [#8505](https://github.com/gfx-rs/wgpu/pull/8505).
203203
- Add clip distances validation for `maxInterStageShaderVariables`. By @ErichDonGubler in [#8762](https://github.com/gfx-rs/wgpu/pull/8762). This may break some existing programs, but it compiles with the WebGPU spec.
204204
- Bring immediates in line with webgpu spec. By @atlv24 in [#9280](https://github.com/gfx-rs/wgpu/pull/9280).
205205
- Validate `LoadOp` and `StoreOp` are `None` for attachments without corresponding depth or stencil aspect. By @beicause in [#9567](https://github.com/gfx-rs/wgpu/pull/9567).
206+
- Validate immediate size when using pipeline auto layout is within device limit. By @beicause in [#9725](https://github.com/gfx-rs/wgpu/pull/9725).
206207

207208
#### DX12
208209

@@ -239,6 +240,7 @@ By @beholdnec in [#8505](https://github.com/gfx-rs/wgpu/pull/8505).
239240
- Fix `packSnorm2x16` and `packUnorm2x16` swap in the GLSL frontend. By @treylutton in [#9675](https://github.com/gfx-rs/wgpu/pull/9675).
240241
- Fixed WGSL loop-local `var` declarations without explicit initializers so they are zero-initialized each iteration. By @ruihe774 in [#9592](https://github.com/gfx-rs/wgpu/pull/9592).
241242
- Fix naga `immediate_slots_required` calculation, and it's moved from `FunctionInfo::immediate_slots_required` to `ModuleInfo::get_entry_point_immediate_slots_required`. By @beicause in [#9725](https://github.com/gfx-rs/wgpu/pull/9725).
243+
- Fix naga panic when immediate size overflow 256 bytes. By @beicause in [#9725](https://github.com/gfx-rs/wgpu/pull/9725).
242244

243245
#### Vulkan
244246

cts_runner/fail.lst

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ webgpu:api,validation,encoding,render_bundle:* // 81%, readonly flag normalizati
4545
webgpu:api,validation,image_copy,buffer_texture_copies:* // https://github.com/gfx-rs/wgpu/issues/7946
4646
webgpu:api,validation,layout_shader_compat:pipeline_layout_shader_exact_match:* // dx12, https://bugzilla.mozilla.org/show_bug.cgi?id=2017725
4747
webgpu:api,validation,non_filterable_texture:non_filterable_texture_with_filtering_sampler:* // 80%, depth textures with filtering samplers
48-
webgpu:api,validation,pipeline,immediates:* // 0%, https://github.com/gfx-rs/wgpu/issues/8556
4948
webgpu:api,validation,query_set,create:count:* // 0%, wgpu incorrectly rejects zero-count query sets
5049
webgpu:api,validation,queue,destroyed,* // 71%, writeBuffer/writeTexture return value, destroyed query set
5150
webgpu:api,validation,queue,writeBuffer:ranges:* // 0%, missing OperationError for invalid ranges

cts_runner/test.lst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ webgpu:api,validation,image_copy,layout_related:required_bytes_in_copy:*
272272
webgpu:api,validation,image_copy,layout_related:rows_per_image_alignment:*
273273
webgpu:api,validation,image_copy,texture_related:*
274274
fails-if(dx12) webgpu:api,validation,layout_shader_compat:pipeline_layout_shader_exact_match:*
275+
webgpu:api,validation,pipeline,immediates:*
275276
webgpu:api,validation,query_set,destroy:*
276277
webgpu:api,validation,queue,buffer_mapped:*
277278
webgpu:api,validation,queue,destroyed,query_set:*

naga/src/valid/analyzer.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1307,7 +1307,7 @@ impl ModuleInfo {
13071307
self.entry_point_immediate_sizes[index]
13081308
}
13091309

1310-
pub fn get_entry_point_immediate_slots_required(&self, index: usize) -> ImmediateSlots {
1310+
pub fn get_entry_point_immediate_slots_required(&self, index: usize) -> Option<ImmediateSlots> {
13111311
self.entry_point_immediate_slots_required[index]
13121312
}
13131313
}

naga/src/valid/immediates.rs

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,31 +13,39 @@ impl ImmediateSlots {
1313
}
1414

1515
/// Compute the bitmask for a byte range [offset .. offset + size_bytes).
16-
pub const fn from_range(offset: u32, size_bytes: u32) -> Self {
16+
/// Return `None` if the bitmask overflows.
17+
pub const fn from_range(offset: u32, size_bytes: u32) -> Option<Self> {
18+
let Some(end) = offset.checked_add(size_bytes) else {
19+
return None;
20+
};
21+
if end > u64::BITS * 4 {
22+
return None;
23+
}
1724
if size_bytes == 0 {
18-
return Self(0);
25+
return Some(Self(0));
1926
}
2027
let lo = offset / 4;
2128
let hi = (offset + size_bytes).div_ceil(4);
22-
Self(u64::MAX << lo & u64::MAX >> (64 - hi))
29+
Some(Self(u64::MAX << lo & u64::MAX >> (64 - hi)))
2330
}
2431

2532
/// Compute the slots occupied by a type at a given byte offset,
2633
/// excluding padding between struct members.
34+
/// Return `None` if the bitmask overflows.
2735
pub fn from_type(
2836
ty: &crate::TypeInner,
2937
offset: u32,
3038
types: &crate::UniqueArena<crate::Type>,
3139
gctx: crate::proc::GlobalCtx,
32-
) -> Self {
40+
) -> Option<Self> {
3341
match *ty {
3442
crate::TypeInner::Struct { ref members, .. } => {
3543
let mut slots = Self::default();
3644
for member in members {
3745
let member_ty = &types[member.ty].inner;
38-
slots |= Self::from_type(member_ty, offset + member.offset, types, gctx);
46+
slots |= Self::from_type(member_ty, offset + member.offset, types, gctx)?;
3947
}
40-
slots
48+
Some(slots)
4149
}
4250
_ => Self::from_range(offset, ty.size(gctx)),
4351
}
@@ -101,35 +109,35 @@ mod tests {
101109
#[test]
102110
fn range_single() {
103111
assert_eq!(
104-
ImmediateSlots::from_range(0, 4),
112+
ImmediateSlots::from_range(0, 4).unwrap(),
105113
ImmediateSlots::from_raw(0b1)
106114
);
107115
assert_eq!(
108-
ImmediateSlots::from_range(4, 4),
116+
ImmediateSlots::from_range(4, 4).unwrap(),
109117
ImmediateSlots::from_raw(0b10)
110118
);
111119
assert_eq!(
112-
ImmediateSlots::from_range(8, 4),
120+
ImmediateSlots::from_range(8, 4).unwrap(),
113121
ImmediateSlots::from_raw(0b100)
114122
);
115123
}
116124

117125
#[test]
118126
fn range_vec4() {
119127
assert_eq!(
120-
ImmediateSlots::from_range(0, 16),
128+
ImmediateSlots::from_range(0, 16).unwrap(),
121129
ImmediateSlots::from_raw(0b1111)
122130
);
123131
assert_eq!(
124-
ImmediateSlots::from_range(16, 16),
132+
ImmediateSlots::from_range(16, 16).unwrap(),
125133
ImmediateSlots::from_raw(0b1111_0000)
126134
);
127135
}
128136

129137
#[test]
130138
fn range_full_256() {
131139
assert_eq!(
132-
ImmediateSlots::from_range(0, 256),
140+
ImmediateSlots::from_range(0, 256).unwrap(),
133141
ImmediateSlots::from_raw(u64::MAX)
134142
);
135143
}
@@ -140,18 +148,19 @@ mod tests {
140148
let struct_ty = (module.types.iter().map(|ty| ty.1))
141149
.find(|ty| ty.name.as_deref() == Some("S"))
142150
.unwrap();
143-
let slots = ImmediateSlots::from_type(&struct_ty.inner, 0, &module.types, module.to_ctx());
151+
let slots =
152+
ImmediateSlots::from_type(&struct_ty.inner, 0, &module.types, module.to_ctx()).unwrap();
144153
assert_eq!(slots, ImmediateSlots::from_raw(0b1111_0001));
145154
}
146155

147156
#[test]
148157
fn range_unaligned() {
149158
assert_eq!(
150-
ImmediateSlots::from_range(0, 3),
159+
ImmediateSlots::from_range(0, 3).unwrap(),
151160
ImmediateSlots::from_raw(0b1)
152161
);
153162
assert_eq!(
154-
ImmediateSlots::from_range(0, 5),
163+
ImmediateSlots::from_range(0, 5).unwrap(),
155164
ImmediateSlots::from_raw(0b11)
156165
);
157166
}
@@ -161,16 +170,16 @@ mod tests {
161170
let required = ImmediateSlots::from_raw(0b1111_0001);
162171
let mut set = ImmediateSlots::default();
163172
assert!(!set.contains(required));
164-
set |= ImmediateSlots::from_range(0, 4);
173+
set |= ImmediateSlots::from_range(0, 4).unwrap();
165174
assert!(!set.contains(required));
166-
set |= ImmediateSlots::from_range(16, 16);
175+
set |= ImmediateSlots::from_range(16, 16).unwrap();
167176
assert!(set.contains(required));
168177
}
169178

170179
#[test]
171180
fn difference() {
172181
let required = ImmediateSlots::from_raw(0b1111_0001);
173-
let set = ImmediateSlots::from_range(0, 4);
182+
let set = ImmediateSlots::from_range(0, 4).unwrap();
174183
assert_eq!(
175184
required.difference(set),
176185
ImmediateSlots::from_raw(0b1111_0000)

naga/src/valid/interface.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -898,7 +898,7 @@ impl VaryingContext<'_> {
898898
pub(super) struct ValidatedEntryPoint {
899899
pub(super) func: FunctionInfo,
900900
pub(super) immediate_size: u32,
901-
pub(super) immediate_slots_required: ImmediateSlots,
901+
pub(super) immediate_slots_required: Option<ImmediateSlots>,
902902
}
903903

904904
impl super::Validator {
@@ -1435,7 +1435,7 @@ impl super::Validator {
14351435
}
14361436

14371437
let mut immediate_size: u32 = 0;
1438-
let mut immediate_slots_required = ImmediateSlots::default();
1438+
let mut immediate_slots_required = Some(ImmediateSlots::default());
14391439
{
14401440
let mut used_immediates = module
14411441
.global_variables
@@ -1454,7 +1454,7 @@ impl super::Validator {
14541454
let ty = &module.types[module.global_variables[immediate].ty].inner;
14551455
let ctx = module.to_ctx();
14561456
immediate_size = ty.size(ctx);
1457-
immediate_slots_required |= ImmediateSlots::from_type(ty, 0, &module.types, ctx);
1457+
immediate_slots_required = ImmediateSlots::from_type(ty, 0, &module.types, ctx);
14581458
}
14591459
}
14601460

naga/src/valid/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ pub struct ModuleInfo {
346346
functions: Vec<FunctionInfo>,
347347
entry_points: Vec<FunctionInfo>,
348348
entry_point_immediate_sizes: Vec<u32>,
349-
entry_point_immediate_slots_required: Vec<ImmediateSlots>,
349+
entry_point_immediate_slots_required: Vec<Option<ImmediateSlots>>,
350350
const_expression_types: Box<[TypeResolution]>,
351351
}
352352

wgpu-core/src/command/bundle.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -816,7 +816,8 @@ fn set_immediates(state: &mut State, offset: u32, data: &[u8]) -> Result<(), Imm
816816
state.immediates.resize(end_offset, 0);
817817
}
818818
state.immediates[(offset as usize)..][..data.len()].copy_from_slice(data);
819-
state.immediate_slots_set |= naga::valid::ImmediateSlots::from_range(offset, data.len() as u32);
819+
state.immediate_slots_set |=
820+
naga::valid::ImmediateSlots::from_range(offset, data.len() as u32).unwrap();
820821
state.immediate_dirty = true;
821822
Ok(())
822823
}

wgpu-core/src/command/pass.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,8 @@ where
246246
state.immediates.resize(end_offset, 0);
247247
}
248248
state.immediates[(offset as usize)..][..data.len()].copy_from_slice(data);
249-
state.immediate_slots_set |= naga::valid::ImmediateSlots::from_range(offset, data.len() as u32);
249+
state.immediate_slots_set |=
250+
naga::valid::ImmediateSlots::from_range(offset, data.len() as u32).unwrap();
250251
state.immediates_dirty = true;
251252

252253
Ok(())

wgpu-core/src/device/resource.rs

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3862,6 +3862,12 @@ impl Device {
38623862
mut derived_group_layouts: Box<ArrayVec<bgl::EntryMap, { hal::MAX_BIND_GROUPS }>>,
38633863
immediate_size: u32,
38643864
) -> Result<Arc<binding_model::PipelineLayout>, pipeline::ImplicitLayoutError> {
3865+
if self.limits.max_immediate_size < immediate_size {
3866+
return Err(pipeline::ImplicitLayoutError::ImmediateRangeTooLarge {
3867+
size: immediate_size,
3868+
max: self.limits.max_immediate_size,
3869+
});
3870+
}
38653871
while derived_group_layouts
38663872
.last()
38673873
.is_some_and(|map| map.is_empty())
@@ -4034,7 +4040,7 @@ impl Device {
40344040
device: self.clone(),
40354041
_shader_module: shader_module,
40364042
late_sized_buffer_groups,
4037-
immediate_slots_required,
4043+
immediate_slots_required: immediate_slots_required.unwrap(),
40384044
label: desc.label.to_string(),
40394045
tracking_data: TrackingData::new(self.tracker_indices.compute_pipelines.clone()),
40404046
};
@@ -4481,7 +4487,7 @@ impl Device {
44814487
let mut _vertex_entry_point_name = String::new();
44824488
let mut _task_entry_point_name = String::new();
44834489
let mut _mesh_entry_point_name = String::new();
4484-
let mut immediate_slots_required = naga::valid::ImmediateSlots::default();
4490+
let mut immediate_slots_required = Some(naga::valid::ImmediateSlots::default());
44854491
let mut immediate_size_required = 0u32;
44864492
match desc.vertex {
44874493
pipeline::RenderPipelineVertexProcessor::Vertex(ref vertex) => {
@@ -4514,7 +4520,15 @@ impl Device {
45144520
stage.to_naga(),
45154521
&_vertex_entry_point_name,
45164522
);
4517-
immediate_slots_required |= _immediate_slots_required;
4523+
if let (
4524+
Some(mut immediate_slots_required),
4525+
Some(_immediate_slots_required),
4526+
) = (immediate_slots_required, _immediate_slots_required)
4527+
{
4528+
immediate_slots_required |= _immediate_slots_required;
4529+
} else {
4530+
immediate_slots_required = None;
4531+
}
45184532
immediate_size_required = immediate_size_required.max(_immediate_size);
45194533
io = interface
45204534
.check_stage(
@@ -4565,7 +4579,15 @@ impl Device {
45654579
stage.to_naga(),
45664580
&_task_entry_point_name,
45674581
);
4568-
immediate_slots_required |= _immediate_slots_required;
4582+
if let (
4583+
Some(mut immediate_slots_required),
4584+
Some(_immediate_slots_required),
4585+
) = (immediate_slots_required, _immediate_slots_required)
4586+
{
4587+
immediate_slots_required |= _immediate_slots_required;
4588+
} else {
4589+
immediate_slots_required = None;
4590+
}
45694591
immediate_size_required = immediate_size_required.max(_immediate_size);
45704592
io = interface
45714593
.check_stage(
@@ -4614,7 +4636,15 @@ impl Device {
46144636
stage.to_naga(),
46154637
&_mesh_entry_point_name,
46164638
);
4617-
immediate_slots_required |= _immediate_slots_required;
4639+
if let (
4640+
Some(mut immediate_slots_required),
4641+
Some(_immediate_slots_required),
4642+
) = (immediate_slots_required, _immediate_slots_required)
4643+
{
4644+
immediate_slots_required |= _immediate_slots_required;
4645+
} else {
4646+
immediate_slots_required = None;
4647+
}
46184648
immediate_size_required = immediate_size_required.max(_immediate_size);
46194649
io = interface
46204650
.check_stage(
@@ -4673,7 +4703,13 @@ impl Device {
46734703
stage.to_naga(),
46744704
&fragment_entry_point_name,
46754705
);
4676-
immediate_slots_required |= _immediate_slots_required;
4706+
if let (Some(mut immediate_slots_required), Some(_immediate_slots_required)) =
4707+
(immediate_slots_required, _immediate_slots_required)
4708+
{
4709+
immediate_slots_required |= _immediate_slots_required;
4710+
} else {
4711+
immediate_slots_required = None;
4712+
}
46774713
immediate_size_required = immediate_size_required.max(_immediate_size);
46784714
io = interface
46794715
.check_stage(
@@ -4905,7 +4941,7 @@ impl Device {
49054941
strip_index_format: desc.primitive.strip_index_format,
49064942
vertex_steps,
49074943
late_sized_buffer_groups,
4908-
immediate_slots_required,
4944+
immediate_slots_required: immediate_slots_required.unwrap(),
49094945
label: desc.label.to_string(),
49104946
tracking_data: TrackingData::new(self.tracker_indices.render_pipelines.clone()),
49114947
is_mesh,

0 commit comments

Comments
 (0)