From 2c581d1e5286b0d0530dcf5bd2127230ee7401b5 Mon Sep 17 00:00:00 2001
From: Brandon Jones <[email protected]>
Date: Tue, 09 Jun 2026 11:03:17 -0700
Subject: [PATCH] Fix OOB read when validating repeated bundles

Stops de-duplicating render bundles when added to a render pass so
that any indirect draws are properly counted and validated. Fixes
an issue where repeating the same bundle multiple times in a render
pass could cause an OOB read on the validated indirect draw
side-table.

Fixed: 520972775
Change-Id: I7ce9e64d19aef31bd1394fa9e18fca911e07f573
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/315036
Reviewed-by: Corentin Wallez <[email protected]>
Commit-Queue: Brandon Jones <[email protected]>
---

diff --git a/src/dawn/native/IndirectDrawMetadata.cpp b/src/dawn/native/IndirectDrawMetadata.cpp
index f748cef..db9eaac 100644
--- a/src/dawn/native/IndirectDrawMetadata.cpp
+++ b/src/dawn/native/IndirectDrawMetadata.cpp
@@ -214,11 +214,6 @@
 }
 
 void IndirectDrawMetadata::AddBundle(RenderBundleBase* bundle) {
-    auto [_, inserted] = mAddedBundles.insert(bundle);
-    if (!inserted) {
-        return;
-    }
-
     IndirectDrawIndex bundleIndirectDrawCount{0u};
     for (const auto& [config, validationInfo] :
          bundle->GetIndirectDrawMetadata().mIndexedIndirectBufferValidationInfo) {
diff --git a/src/dawn/native/IndirectDrawMetadata.h b/src/dawn/native/IndirectDrawMetadata.h
index 71d090b..63aa76b 100644
--- a/src/dawn/native/IndirectDrawMetadata.h
+++ b/src/dawn/native/IndirectDrawMetadata.h
@@ -215,7 +215,6 @@
 
   private:
     IndexedIndirectBufferValidationInfoMap mIndexedIndirectBufferValidationInfo;
-    absl::flat_hash_set<RenderBundleBase*> mAddedBundles;
 
     std::vector<IndirectMultiDraw> mMultiDraws;
 
diff --git a/src/dawn/tests/end2end/RenderBundleTests.cpp b/src/dawn/tests/end2end/RenderBundleTests.cpp
index 45cd3fa..cc963a2 100644
--- a/src/dawn/tests/end2end/RenderBundleTests.cpp
+++ b/src/dawn/tests/end2end/RenderBundleTests.cpp
@@ -470,6 +470,111 @@
     EXPECT_BUFFER_U32_EQ(3, counterRead, 0);
 }
 
+// Tests a bug outlined in crbug.com/520972775 where repeated execution of bundles with an indirect
+// draw could de-duplicate the draw calls when validating, causing the ValidatedIndirectDraw side
+// table to have too few entries.
+TEST_P(RenderBundleIndirectValidationTest, SinglePassRepeatedIndirectDraw) {
+    // Render Pass
+    utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, kRTSize, kRTSize);
+
+    // Index Buffers
+    wgpu::Buffer smallIdx = CreateIndexBuffer({0, 1, 2});
+
+    // Indirect buffer
+    wgpu::Buffer indirect = CreateIndirectBuffer({3, 1, 0, 0, 0});
+
+    // Buffers to use for simple fragment counter
+    uint32_t data[] = {0};
+    wgpu::Buffer counterBuffer = utils::CreateBufferFromData(
+        device, data, sizeof(uint32_t),
+        wgpu::BufferUsage::Storage | wgpu::BufferUsage::CopyDst | wgpu::BufferUsage::CopySrc);
+    wgpu::Buffer counterRead = utils::CreateBufferFromData(
+        device, data, sizeof(uint32_t), wgpu::BufferUsage::CopyDst | wgpu::BufferUsage::CopySrc);
+
+    // Pipeline
+    wgpu::ShaderModule module = utils::CreateShaderModule(device, R"(
+        struct Ctr { n: atomic<u32>, };
+        @group(0) @binding(0) var<storage, read_write> ctr: Ctr;
+
+        @vertex fn vs() -> @builtin(position) vec4f {
+            return vec4f(0.0, 0.0, 0.0, 1.0);
+        }
+        // Simply adds one to the simple fragment counter with each draw
+        @fragment fn fs() -> @location(0) vec4f {
+            atomicAdd(&ctr.n, 1u);
+            return vec4f(1.0, 0.0, 0.0, 1.0);
+        })");
+
+    utils::ComboRenderPipelineDescriptor descriptor;
+    descriptor.vertex.module = module;
+    descriptor.cFragment.module = module;
+    descriptor.primitive.topology = wgpu::PrimitiveTopology::PointList;
+    descriptor.cTargets[0].format = renderPass.colorFormat;
+    wgpu::RenderPipeline pipeline = device.CreateRenderPipeline(&descriptor);
+
+    // Bind group
+    wgpu::BindGroup bindGroup = utils::MakeBindGroup(device, pipeline.GetBindGroupLayout(0),
+                                                     {{0, counterBuffer, 0, sizeof(float)}});
+
+    // Render bundles
+    utils::ComboRenderBundleEncoderDescriptor desc = {};
+    desc.colorFormatCount = 1;
+    desc.cColorFormats[0] = renderPass.colorFormat;
+
+    wgpu::RenderBundle directRenderBundle;
+    {
+        wgpu::RenderBundleEncoder renderBundleEncoder = device.CreateRenderBundleEncoder(&desc);
+        renderBundleEncoder.SetPipeline(pipeline);
+        renderBundleEncoder.SetBindGroup(0, bindGroup);
+        renderBundleEncoder.DrawIndirect(indirect, 0);
+        directRenderBundle = renderBundleEncoder.Finish();
+    }
+
+    wgpu::RenderBundle indexedRenderBundle;
+    {
+        wgpu::RenderBundleEncoder renderBundleEncoder = device.CreateRenderBundleEncoder(&desc);
+        renderBundleEncoder.SetPipeline(pipeline);
+        renderBundleEncoder.SetBindGroup(0, bindGroup);
+        renderBundleEncoder.SetIndexBuffer(smallIdx, wgpu::IndexFormat::Uint32);
+        renderBundleEncoder.DrawIndexedIndirect(indirect, 0);
+        indexedRenderBundle = renderBundleEncoder.Finish();
+    }
+
+    //
+    // Bug - Same bundle executed twice in a single render pass.
+    //
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+
+        // Test bundles with DrawIndirect calls
+        wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass.renderPassInfo);
+        pass.ExecuteBundles(1, &directRenderBundle);
+        pass.ExecuteBundles(1, &directRenderBundle);
+        pass.End();
+
+        wgpu::CommandBuffer commands = encoder.Finish();
+        queue.Submit(1, &commands);
+    }
+
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+
+        // Test bundles with DrawIndexedIndirect calls
+        wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass.renderPassInfo);
+        pass.ExecuteBundles(1, &indexedRenderBundle);
+        pass.ExecuteBundles(1, &indexedRenderBundle);
+        pass.End();
+
+        // Copy the fragment counter results of both passes to the readback buffer.
+        encoder.CopyBufferToBuffer(counterBuffer, 0, counterRead, 0, 4);
+        wgpu::CommandBuffer commands = encoder.Finish();
+        queue.Submit(1, &commands);
+    }
+
+    // Each bundle should produce 3 fragments, for a total of 12.
+    EXPECT_BUFFER_U32_EQ(12, counterRead, 0);
+}
+
 DAWN_INSTANTIATE_TEST(RenderBundleIndirectValidationTest,
                       D3D11Backend(),
                       D3D12Backend(),

diff --git a/src/dawn/tests/end2end/RenderBundleTests.cpp b/src/dawn/tests/end2end/RenderBundleTests.cpp
index 45cd3fa..cc963a2 100644
--- a/src/dawn/tests/end2end/RenderBundleTests.cpp
+++ b/src/dawn/tests/end2end/RenderBundleTests.cpp
@@ -470,6 +470,111 @@
     EXPECT_BUFFER_U32_EQ(3, counterRead, 0);
 }
 
+// Tests a bug outlined in crbug.com/520972775 where repeated execution of bundles with an indirect
+// draw could de-duplicate the draw calls when validating, causing the ValidatedIndirectDraw side
+// table to have too few entries.
+TEST_P(RenderBundleIndirectValidationTest, SinglePassRepeatedIndirectDraw) {
+    // Render Pass
+    utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, kRTSize, kRTSize);
+
+    // Index Buffers
+    wgpu::Buffer smallIdx = CreateIndexBuffer({0, 1, 2});
+
+    // Indirect buffer
+    wgpu::Buffer indirect = CreateIndirectBuffer({3, 1, 0, 0, 0});
+
+    // Buffers to use for simple fragment counter
+    uint32_t data[] = {0};
+    wgpu::Buffer counterBuffer = utils::CreateBufferFromData(
+        device, data, sizeof(uint32_t),
+        wgpu::BufferUsage::Storage | wgpu::BufferUsage::CopyDst | wgpu::BufferUsage::CopySrc);
+    wgpu::Buffer counterRead = utils::CreateBufferFromData(
+        device, data, sizeof(uint32_t), wgpu::BufferUsage::CopyDst | wgpu::BufferUsage::CopySrc);
+
+    // Pipeline
+    wgpu::ShaderModule module = utils::CreateShaderModule(device, R"(
+        struct Ctr { n: atomic<u32>, };
+        @group(0) @binding(0) var<storage, read_write> ctr: Ctr;
+
+        @vertex fn vs() -> @builtin(position) vec4f {
+            return vec4f(0.0, 0.0, 0.0, 1.0);
+        }
+        // Simply adds one to the simple fragment counter with each draw
+        @fragment fn fs() -> @location(0) vec4f {
+            atomicAdd(&ctr.n, 1u);
+            return vec4f(1.0, 0.0, 0.0, 1.0);
+        })");
+
+    utils::ComboRenderPipelineDescriptor descriptor;
+    descriptor.vertex.module = module;
+    descriptor.cFragment.module = module;
+    descriptor.primitive.topology = wgpu::PrimitiveTopology::PointList;
+    descriptor.cTargets[0].format = renderPass.colorFormat;
+    wgpu::RenderPipeline pipeline = device.CreateRenderPipeline(&descriptor);
+
+    // Bind group
+    wgpu::BindGroup bindGroup = utils::MakeBindGroup(device, pipeline.GetBindGroupLayout(0),
+                                                     {{0, counterBuffer, 0, sizeof(float)}});
+
+    // Render bundles
+    utils::ComboRenderBundleEncoderDescriptor desc = {};
+    desc.colorFormatCount = 1;
+    desc.cColorFormats[0] = renderPass.colorFormat;
+
+    wgpu::RenderBundle directRenderBundle;
+    {
+        wgpu::RenderBundleEncoder renderBundleEncoder = device.CreateRenderBundleEncoder(&desc);
+        renderBundleEncoder.SetPipeline(pipeline);
+        renderBundleEncoder.SetBindGroup(0, bindGroup);
+        renderBundleEncoder.DrawIndirect(indirect, 0);
+        directRenderBundle = renderBundleEncoder.Finish();
+    }
+
+    wgpu::RenderBundle indexedRenderBundle;
+    {
+        wgpu::RenderBundleEncoder renderBundleEncoder = device.CreateRenderBundleEncoder(&desc);
+        renderBundleEncoder.SetPipeline(pipeline);
+        renderBundleEncoder.SetBindGroup(0, bindGroup);
+        renderBundleEncoder.SetIndexBuffer(smallIdx, wgpu::IndexFormat::Uint32);
+        renderBundleEncoder.DrawIndexedIndirect(indirect, 0);
+        indexedRenderBundle = renderBundleEncoder.Finish();
+    }
+
+    //
+    // Bug - Same bundle executed twice in a single render pass.
+    //
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+
+        // Test bundles with DrawIndirect calls
+        wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass.renderPassInfo);
+        pass.ExecuteBundles(1, &directRenderBundle);
+        pass.ExecuteBundles(1, &directRenderBundle);
+        pass.End();
+
+        wgpu::CommandBuffer commands = encoder.Finish();
+        queue.Submit(1, &commands);
+    }
+
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+
+        // Test bundles with DrawIndexedIndirect calls
+        wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass.renderPassInfo);
+        pass.ExecuteBundles(1, &indexedRenderBundle);
+        pass.ExecuteBundles(1, &indexedRenderBundle);
+        pass.End();
+
+        // Copy the fragment counter results of both passes to the readback buffer.
+        encoder.CopyBufferToBuffer(counterBuffer, 0, counterRead, 0, 4);
+        wgpu::CommandBuffer commands = encoder.Finish();
+        queue.Submit(1, &commands);
+    }
+
+    // Each bundle should produce 3 fragments, for a total of 12.
+    EXPECT_BUFFER_U32_EQ(12, counterRead, 0);
+}
+
 DAWN_INSTANTIATE_TEST(RenderBundleIndirectValidationTest,
                       D3D11Backend(),
                       D3D12Backend(),