No-ops for out of bounds drawIndexed

Store index buffer size in CommandBufferStateTracker and skip
issuing drawIndexed call if it's out of bounds.

Bug: dawn:622
Change-Id: I8f4bd8ba03dea931815dc0db87ffacb9936a123d
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/42000
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Shrek Shao <shrekshao@google.com>

src/dawn_native/CommandBufferStateTracker.cpp

@@ -210,9 +210,10 @@ namespace dawn_native {
         mAspects.reset(VALIDATION_ASPECT_BIND_GROUPS);
     }
 
-    void CommandBufferStateTracker::SetIndexBuffer(wgpu::IndexFormat format) {
+    void CommandBufferStateTracker::SetIndexBuffer(wgpu::IndexFormat format, uint64_t size) {
         mIndexBufferSet = true;
         mIndexFormat = format;
+        mIndexBufferSize = size;
     }
 
     void CommandBufferStateTracker::SetVertexBuffer(VertexBufferSlot slot) {

src/dawn_native/CommandBufferStateTracker.h

@@ -38,12 +38,19 @@ namespace dawn_native {
         void SetComputePipeline(ComputePipelineBase* pipeline);
         void SetRenderPipeline(RenderPipelineBase* pipeline);
         void SetBindGroup(BindGroupIndex index, BindGroupBase* bindgroup);
-        void SetIndexBuffer(wgpu::IndexFormat format);
+        void SetIndexBuffer(wgpu::IndexFormat format, uint64_t size);
         void SetVertexBuffer(VertexBufferSlot slot);
 
         static constexpr size_t kNumAspects = 4;
         using ValidationAspects = std::bitset<kNumAspects>;
 
+        uint64_t GetIndexBufferSize() {
+            return mIndexBufferSize;
+        }
+        wgpu::IndexFormat GetIndexFormat() {
+            return mIndexFormat;
+        }
+
       private:
         MaybeError ValidateOperation(ValidationAspects requiredAspects);
         void RecomputeLazyAspects(ValidationAspects aspects);
@@ -57,6 +64,7 @@ namespace dawn_native {
         ityp::bitset<VertexBufferSlot, kMaxVertexBuffers> mVertexBufferSlotsUsed;
         bool mIndexBufferSet = false;
         wgpu::IndexFormat mIndexFormat;
+        uint64_t mIndexBufferSize = 0;
 
         PipelineLayoutBase* mLastPipelineLayout = nullptr;
         RenderPipelineBase* mLastRenderPipeline = nullptr;

src/dawn_native/RenderEncoderBase.cpp

@@ -15,6 +15,7 @@
 #include "dawn_native/RenderEncoderBase.h"
 
 #include "common/Constants.h"
+#include "common/Log.h"
 #include "dawn_native/Buffer.h"
 #include "dawn_native/CommandEncoder.h"
 #include "dawn_native/CommandValidation.h"
@@ -95,6 +96,14 @@ namespace dawn_native {
                 }
             }
 
+            if (static_cast<uint64_t>(firstIndex) + indexCount >
+                mCommandBufferState.GetIndexBufferSize() /
+                    IndexFormatSize(mCommandBufferState.GetIndexFormat())) {
+                // Index range is out of bounds
+                // Treat as no-op and skip issuing draw call
+                dawn::WarningLog() << "Index range is out of bounds";
+                return {};
+            }
             DrawIndexedCmd* draw = allocator->Allocate<DrawIndexedCmd>(Command::DrawIndexed);
             draw->indexCount = indexCount;
             draw->instanceCount = instanceCount;
@@ -235,7 +244,7 @@ namespace dawn_native {
                 }
             }
 
-            mCommandBufferState.SetIndexBuffer(format);
+            mCommandBufferState.SetIndexBuffer(format, size);
 
             SetIndexBufferCmd* cmd =
                 allocator->Allocate<SetIndexBufferCmd>(Command::SetIndexBuffer);

src/tests/end2end/DrawIndexedTests.cpp

@@ -114,6 +114,35 @@ TEST_P(DrawIndexedTest, Uint32) {
     Test(6, 1, 0, 0, 0, 0, filled, filled);
 }
 
+// Out of bounds drawIndexed are treated as no-ops instead of invalid operations
+// Some agreements: https://github.com/gpuweb/gpuweb/issues/955
+TEST_P(DrawIndexedTest, OutOfBounds) {
+    RGBA8 filled(0, 255, 0, 255);
+    RGBA8 notFilled(0, 0, 0, 0);
+
+    // a valid draw.
+    Test(6, 1, 0, 0, 0, 0, filled, filled);
+    // indexCount is 0 but firstIndex out of bound
+    Test(0, 1, 20, 0, 0, 0, notFilled, notFilled);
+    // indexCount + firstIndex out of bound
+    Test(6, 1, 7, 0, 0, 0, notFilled, notFilled);
+    // only firstIndex out of bound
+    Test(6, 1, 20, 0, 0, 0, notFilled, notFilled);
+    // firstIndex much larger than the bound
+    Test(6, 1, 10000, 0, 0, 0, notFilled, notFilled);
+    // only indexCount out of bound
+    Test(20, 1, 0, 0, 0, 0, notFilled, notFilled);
+    // indexCount much larger than the bound
+    Test(10000, 1, 0, 0, 0, 0, notFilled, notFilled);
+    // max uint32_t indexCount and firstIndex
+    Test(std::numeric_limits<uint32_t>::max(), 1, std::numeric_limits<uint32_t>::max(), 0, 0, 0,
+         notFilled, notFilled);
+    // max uint32_t indexCount and small firstIndex
+    Test(std::numeric_limits<uint32_t>::max(), 1, 2, 0, 0, 0, notFilled, notFilled);
+    // small indexCount and max uint32_t firstIndex
+    Test(2, 1, std::numeric_limits<uint32_t>::max(), 0, 0, 0, notFilled, notFilled);
+}
+
 // Test the parameter 'baseVertex' of DrawIndexed() works.
 TEST_P(DrawIndexedTest, BaseVertex) {
     DAWN_SKIP_TEST_IF(HasToggleEnabled("disable_base_vertex"));