commit 28c1fba1c02eb0c8fedb9794645e4a2dc06d1904
author Corentin Wallez <cwallez@chromium.org> Sat Dec 15 10:34:42 2018 +0000
committer Commit Bot service account <commit-bot@chromium.org> Sat Dec 15 10:34:42 2018 +0000
tree d2b372a2edb0cfc781ba8ed3b648cd4b3edf51b3
parent e018292bed9cf0dd25ceb8c044516edeee931415

Validate CommmandBuffers aren't ended mid pass.

Also adds regression tests.

BUG=chromium:914566
BUG=chromium:914641

Change-Id: Ic1f9f2440580c3598831c8b2d1310e81aa944133
Reviewed-on: https://dawn-review.googlesource.com/c/3321
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/CommandBuffer.cpp

diff --git a/src/dawn_native/CommandBuffer.cpp b/src/dawn_native/CommandBuffer.cpp
index c9e6c25..97b5678 100644
--- a/src/dawn_native/CommandBuffer.cpp
+++ b/src/dawn_native/CommandBuffer.cpp
@@ -364,6 +364,10 @@
     // Implementation of the command buffer validation that can be precomputed before submit
 
     MaybeError CommandBufferBuilder::ValidateGetResult() {
+        if (mEncodingState != EncodingState::TopLevel) {
+            return DAWN_VALIDATION_ERROR("Command buffer recording ended mid-pass");
+        }
+
         MoveToIterator();
         mIterator.Reset();
 
@@ -502,6 +506,7 @@
             }
         }
 
+        UNREACHABLE();
         return DAWN_VALIDATION_ERROR("Unfinished compute pass");
     }
 
@@ -609,6 +614,7 @@
             }
         }
 
+        UNREACHABLE();
         return DAWN_VALIDATION_ERROR("Unfinished render pass");
     }
 

src/tests/unittests/validation/CommandBufferValidationTests.cpp

diff --git a/src/tests/unittests/validation/CommandBufferValidationTests.cpp b/src/tests/unittests/validation/CommandBufferValidationTests.cpp
index 8655449..017d4fc 100644
--- a/src/tests/unittests/validation/CommandBufferValidationTests.cpp
+++ b/src/tests/unittests/validation/CommandBufferValidationTests.cpp
@@ -25,10 +25,11 @@
         .GetResult();
 }
 
-// Tests for basic render pass usage
-TEST_F(CommandBufferValidationTest, RenderPass) {
-    auto renderpass = CreateSimpleRenderPass();
+// Test that a command buffer cannot be ended mid render pass
+TEST_F(CommandBufferValidationTest, EndedMidRenderPass) {
+    dawn::RenderPassDescriptor renderpass = CreateSimpleRenderPass();
 
+    // Control case, command buffer ended after the pass is ended.
     {
         dawn::CommandBufferBuilder builder = AssertWillBeSuccess(device.CreateCommandBufferBuilder());
         dawn::RenderPassEncoder pass = builder.BeginRenderPass(renderpass);
@@ -36,11 +37,52 @@
         builder.GetResult();
     }
 
+    // Error case, command buffer ended mid-pass.
     {
         dawn::CommandBufferBuilder builder = AssertWillBeError(device.CreateCommandBufferBuilder());
         dawn::RenderPassEncoder pass = builder.BeginRenderPass(renderpass);
         builder.GetResult();
     }
+
+    // Error case, command buffer ended mid-pass. Trying to use encoders after GetResult
+    // should fail too.
+    {
+        dawn::CommandBufferBuilder builder = AssertWillBeError(device.CreateCommandBufferBuilder());
+        dawn::RenderPassEncoder pass = builder.BeginRenderPass(renderpass);
+        builder.GetResult();
+        // TODO(cwallez@chromium.org) this should probably be a device error, but currently it
+        // produces a builder error.
+        pass.EndPass();
+    }
+}
+
+// Test that a command buffer cannot be ended mid compute pass
+TEST_F(CommandBufferValidationTest, EndedMidComputePass) {
+    // Control case, command buffer ended after the pass is ended.
+    {
+        dawn::CommandBufferBuilder builder = AssertWillBeSuccess(device.CreateCommandBufferBuilder());
+        dawn::ComputePassEncoder pass = builder.BeginComputePass();
+        pass.EndPass();
+        builder.GetResult();
+    }
+
+    // Error case, command buffer ended mid-pass.
+    {
+        dawn::CommandBufferBuilder builder = AssertWillBeError(device.CreateCommandBufferBuilder());
+        dawn::ComputePassEncoder pass = builder.BeginComputePass();
+        builder.GetResult();
+    }
+
+    // Error case, command buffer ended mid-pass. Trying to use encoders after GetResult
+    // should fail too.
+    {
+        dawn::CommandBufferBuilder builder = AssertWillBeError(device.CreateCommandBufferBuilder());
+        dawn::ComputePassEncoder pass = builder.BeginComputePass();
+        builder.GetResult();
+        // TODO(cwallez@chromium.org) this should probably be a device error, but currently it
+        // produces a builder error.
+        pass.EndPass();
+    }
 }
 
 // Test that using a single buffer in multiple read usages in the same pass is allowed.