BACKPORT to 0.19: fix: don't depend on BG{,L} layout entry order in HAL #5421 (#5455)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This commit is contained in:
Erich Gubler 2024-04-17 15:37:47 -04:00 committed by GitHub
parent 2516bc2d42
commit 95dc7e5991
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 171 additions and 7 deletions

View File

@ -60,6 +60,19 @@ Bottom level categories:
- `step`
- `tan`
- `tanh`
### Bug Fixes
#### GLES
- Don't depend on bind group and bind group layout entry order in HAL. This caused incorrect severely incorrect command execution and, in some cases, crashes. By @ErichDonGubler in [#5421](https://github.com/gfx-rs/wgpu/pull/5421).
#### Metal
- Don't depend on bind group and bind group layout entry order in HAL. This caused incorrect severely incorrect command execution and, in some cases, crashes. By @ErichDonGubler in [#5421](https://github.com/gfx-rs/wgpu/pull/5421).
#### DX12
- Don't depend on bind group and bind group layout entry order in HAL. This caused incorrect severely incorrect command execution and, in some cases, crashes. By @ErichDonGubler in [#5421](https://github.com/gfx-rs/wgpu/pull/5421).
## v0.19.3 (2024-03-01)

4
Cargo.lock generated
View File

@ -2040,9 +2040,9 @@ dependencies = [
[[package]]
name = "mio"
version = "0.8.10"
version = "0.8.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8f3d0b296e374a4e6f3c7b0a1f5a51d748a0d34c85e7dc48fc3fa9a87657fe09"
checksum = "a4a650543ca06a924e8b371db273b2756685faae30f8487da1b56505a8f78b0c"
dependencies = [
"libc",
"log",

View File

@ -615,3 +615,125 @@ static DROPPED_GLOBAL_THEN_DEVICE_LOST: GpuTestConfiguration = GpuTestConfigurat
"Device lost callback should have been called."
);
});
#[gpu_test]
static DIFFERENT_BGL_ORDER_BW_SHADER_AND_API: GpuTestConfiguration = GpuTestConfiguration::new()
.parameters(TestParameters::default())
.run_sync(|ctx| {
// This test addresses a bug found in multiple backends where `wgpu_core` and `wgpu_hal`
// backends made different assumptions about the element order of vectors of bind group
// layout entries and bind group resource bindings.
//
// Said bug was exposed originally by:
//
// 1. Shader-declared bindings having a different order than resource bindings provided to
// `Device::create_bind_group`.
// 2. Having more of one type of resource in the bind group than another.
//
// …such that internals would accidentally attempt to use an out-of-bounds index (of one
// resource type) in the wrong list of a different resource type. Let's reproduce that
// here.
let trivial_shaders_with_some_reversed_bindings = "\
@group(0) @binding(3) var myTexture2: texture_2d<f32>;
@group(0) @binding(2) var myTexture1: texture_2d<f32>;
@group(0) @binding(1) var mySampler: sampler;
@fragment
fn fs_main(@builtin(position) pos: vec4<f32>) -> @location(0) vec4f {
return textureSample(myTexture1, mySampler, pos.xy) + textureSample(myTexture2, mySampler, pos.xy);
}
@vertex
fn vs_main() -> @builtin(position) vec4<f32> {
return vec4<f32>(0.0, 0.0, 0.0, 1.0);
}
";
let trivial_shaders_with_some_reversed_bindings =
ctx.device
.create_shader_module(wgpu::ShaderModuleDescriptor {
label: None,
source: wgpu::ShaderSource::Wgsl(
trivial_shaders_with_some_reversed_bindings.into(),
),
});
let my_texture = ctx.device.create_texture(&wgt::TextureDescriptor {
label: None,
size: wgt::Extent3d {
width: 1024,
height: 512,
depth_or_array_layers: 1,
},
mip_level_count: 1,
sample_count: 1,
dimension: wgt::TextureDimension::D2,
format: wgt::TextureFormat::Rgba8Unorm,
usage: wgt::TextureUsages::RENDER_ATTACHMENT | wgt::TextureUsages::TEXTURE_BINDING,
view_formats: &[],
});
let my_texture_view = my_texture.create_view(&wgpu::TextureViewDescriptor {
label: None,
format: None,
dimension: None,
aspect: wgt::TextureAspect::All,
base_mip_level: 0,
mip_level_count: None,
base_array_layer: 0,
array_layer_count: None,
});
let my_sampler = ctx
.device
.create_sampler(&wgpu::SamplerDescriptor::default());
let render_pipeline = ctx
.device
.create_render_pipeline(&wgpu::RenderPipelineDescriptor {
fragment: Some(wgpu::FragmentState {
module: &trivial_shaders_with_some_reversed_bindings,
entry_point: "fs_main",
targets: &[Some(wgt::ColorTargetState {
format: wgt::TextureFormat::Bgra8Unorm,
blend: None,
write_mask: wgt::ColorWrites::ALL,
})],
}),
layout: None,
// Other fields below aren't interesting for this text.
label: None,
vertex: wgpu::VertexState {
module: &trivial_shaders_with_some_reversed_bindings,
entry_point: "vs_main",
buffers: &[],
},
primitive: wgt::PrimitiveState::default(),
depth_stencil: None,
multisample: wgt::MultisampleState::default(),
multiview: None,
});
// fail(&ctx.device, || {
// }, "");
ctx.device.create_bind_group(&wgpu::BindGroupDescriptor {
label: None,
layout: &render_pipeline.get_bind_group_layout(0),
entries: &[
wgpu::BindGroupEntry {
binding: 1,
resource: wgpu::BindingResource::Sampler(&my_sampler),
},
wgpu::BindGroupEntry {
binding: 2,
resource: wgpu::BindingResource::TextureView(&my_texture_view),
},
wgpu::BindGroupEntry {
binding: 3,
resource: wgpu::BindingResource::TextureView(&my_texture_view),
},
],
});
});

View File

@ -1098,7 +1098,16 @@ impl crate::Device<super::Api> for super::Device {
}
let mut dynamic_buffers = Vec::new();
for (layout, entry) in desc.layout.entries.iter().zip(desc.entries.iter()) {
let layout_and_entry_iter = desc.entries.iter().map(|entry| {
let layout = desc
.layout
.entries
.iter()
.find(|layout_entry| layout_entry.binding == entry.binding)
.expect("internal error: no layout entry found with binding slot");
(layout, entry)
});
for (layout, entry) in layout_and_entry_iter {
match layout.ty {
wgt::BindingType::Buffer {
has_dynamic_offset: true,

View File

@ -1123,8 +1123,10 @@ impl crate::Device<super::Api> for super::Device {
!0;
bg_layout
.entries
.last()
.map_or(0, |b| b.binding as usize + 1)
.iter()
.map(|b| b.binding)
.max()
.map_or(0, |idx| idx as usize + 1)
]
.into_boxed_slice();
@ -1177,7 +1179,16 @@ impl crate::Device<super::Api> for super::Device {
) -> Result<super::BindGroup, crate::DeviceError> {
let mut contents = Vec::new();
for (entry, layout) in desc.entries.iter().zip(desc.layout.entries.iter()) {
let layout_and_entry_iter = desc.entries.iter().map(|entry| {
let layout = desc
.layout
.entries
.iter()
.find(|layout_entry| layout_entry.binding == entry.binding)
.expect("internal error: no layout entry found with binding slot");
(entry, layout)
});
for (entry, layout) in layout_and_entry_iter {
let binding = match layout.ty {
wgt::BindingType::Buffer { .. } => {
let bb = &desc.buffers[entry.resource_index as usize];

View File

@ -706,7 +706,16 @@ impl crate::Device<super::Api> for super::Device {
for (&stage, counter) in super::NAGA_STAGES.iter().zip(bg.counters.iter_mut()) {
let stage_bit = map_naga_stage(stage);
let mut dynamic_offsets_count = 0u32;
for (entry, layout) in desc.entries.iter().zip(desc.layout.entries.iter()) {
let layout_and_entry_iter = desc.entries.iter().map(|entry| {
let layout = desc
.layout
.entries
.iter()
.find(|layout_entry| layout_entry.binding == entry.binding)
.expect("internal error: no layout entry found with binding slot");
(entry, layout)
});
for (entry, layout) in layout_and_entry_iter {
let size = layout.count.map_or(1, |c| c.get());
if let wgt::BindingType::Buffer {
has_dynamic_offset: true,