commit 3fe84b5b33b74481dab7b82001e93bc339238918
author Natasha Lee <natlee@microsoft.com> Tue Jun 16 17:53:38 2020 +0000
committer Commit Bot service account <commit-bot@chromium.org> Tue Jun 16 17:53:38 2020 +0000
tree 94ebf1d4dafc8159e90d158ea3052be9a8f910c4
parent ac6bd4c98fc1ee386a728a1eec92075e5236a355

Fix AssertAndIgnoreDeviceLossError to handle errors injected by fuzzer

Fuzzer recently started sometimes injecting errors with
CheckHRESULT returning E_FAKE_ERROR_FOR_TESTING, which needs to also be ignored
in AssertAndIgnoreDeviceLossError so that we can continue with destruction.

Rename AssertAndIgnoreDeviceLossError to IgnoreErrors

This CL also adds Austin's pretty printing of HRESULT results as hex values
and adds printing of "E_FAKE_ERROR_FOR_TESTING" error.

Bug: chromium:1094448
Change-Id: I6715a5eec02919a3e3b4c86d493b3f7e84c5e206
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/23241
Commit-Queue: Natasha Lee <natlee@microsoft.com>
Reviewed-by: Rafael Cintron <rafael.cintron@microsoft.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>

src/dawn_native/Device.cpp

diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index 7a0d04e..1c30cd5 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -125,8 +125,8 @@
             case State::Alive:
                 // Alive is the only state which can have GPU work happening. Wait for all of it to
                 // complete before proceeding with destruction.
-                // Assert that errors are device loss so that we can continue with destruction
-                AssertAndIgnoreDeviceLossError(WaitForIdleForDestruction());
+                // Ignore errors so that we can continue with destruction
+                IgnoreErrors(WaitForIdleForDestruction());
                 AssumeCommandsComplete();
                 break;
 
@@ -153,8 +153,8 @@
             mFenceSignalTracker->Tick(GetCompletedCommandSerial());
             mMapRequestTracker->Tick(GetCompletedCommandSerial());
             // call TickImpl once last time to clean up resources
-            // assert the errors are device loss so we can continue with destruction
-            AssertAndIgnoreDeviceLossError(TickImpl());
+            // Ignore errors so that we can continue with destruction
+            IgnoreErrors(TickImpl());
         }
 
         // At this point GPU operations are always finished, so we are in the disconnected state.
@@ -187,10 +187,10 @@
             // threads in a multithreaded scenario?
             mState = State::BeingDisconnected;
 
-            // Assert that errors are device losses so that we can continue with destruction.
+            // Ignore errors so that we can continue with destruction
             // Assume all commands are complete after WaitForIdleForDestruction (because they were)
-            AssertAndIgnoreDeviceLossError(WaitForIdleForDestruction());
-            AssertAndIgnoreDeviceLossError(TickImpl());
+            IgnoreErrors(WaitForIdleForDestruction());
+            IgnoreErrors(TickImpl());
             AssumeCommandsComplete();
             ASSERT(mFutureCallbackSerial <= mCompletedSerial);
             mState = State::Disconnected;

src/dawn_native/Error.cpp

diff --git a/src/dawn_native/Error.cpp b/src/dawn_native/Error.cpp
index 13db32e..6dcc3f8 100644
--- a/src/dawn_native/Error.cpp
+++ b/src/dawn_native/Error.cpp
@@ -19,10 +19,14 @@
 
 namespace dawn_native {
 
-    void AssertAndIgnoreDeviceLossError(MaybeError maybeError) {
+    void IgnoreErrors(MaybeError maybeError) {
         if (maybeError.IsError()) {
             std::unique_ptr<ErrorData> errorData = maybeError.AcquireError();
-            ASSERT(errorData->GetType() == InternalErrorType::DeviceLost);
+            // During shutdown and destruction, device lost errors can be ignored.
+            // We can also ignore other unexpected internal errors on shut down and treat it as
+            // device lost so that we can continue with destruction.
+            ASSERT(errorData->GetType() == InternalErrorType::DeviceLost ||
+                   errorData->GetType() == InternalErrorType::Internal);
         }
     }
 

src/dawn_native/Error.h

diff --git a/src/dawn_native/Error.h b/src/dawn_native/Error.h
index 41a1eff..3d5d5c3 100644
--- a/src/dawn_native/Error.h
+++ b/src/dawn_native/Error.h
@@ -114,7 +114,7 @@
     break
 
     // Assert that errors are device loss so that we can continue with destruction
-    void AssertAndIgnoreDeviceLossError(MaybeError maybeError);
+    void IgnoreErrors(MaybeError maybeError);
 
     wgpu::ErrorType ToWGPUErrorType(InternalErrorType type);
     InternalErrorType FromWGPUErrorType(wgpu::ErrorType type);

src/dawn_native/d3d12/D3D12Error.cpp

diff --git a/src/dawn_native/d3d12/D3D12Error.cpp b/src/dawn_native/d3d12/D3D12Error.cpp
index dfeaac0..efc9fe0 100644
--- a/src/dawn_native/d3d12/D3D12Error.cpp
+++ b/src/dawn_native/d3d12/D3D12Error.cpp
@@ -14,6 +14,8 @@
 
 #include "dawn_native/d3d12/D3D12Error.h"
 
+#include <iomanip>
+#include <sstream>
 #include <string>
 
 namespace dawn_native { namespace d3d12 {
@@ -22,12 +24,19 @@
             return {};
         }
 
-        std::string message = std::string(context) + " failed with " + std::to_string(result);
+        std::ostringstream messageStream;
+        messageStream << context << " failed with ";
+        if (result == E_FAKE_ERROR_FOR_TESTING) {
+            messageStream << "E_FAKE_ERROR_FOR_TESTING";
+        } else {
+            messageStream << "0x" << std::uppercase << std::setfill('0') << std::setw(8) << std::hex
+                          << result;
+        }
 
         if (result == DXGI_ERROR_DEVICE_REMOVED) {
-            return DAWN_DEVICE_LOST_ERROR(message);
+            return DAWN_DEVICE_LOST_ERROR(messageStream.str());
         } else {
-            return DAWN_INTERNAL_ERROR(message);
+            return DAWN_INTERNAL_ERROR(messageStream.str());
         }
     }