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>
This commit is contained in:
Natasha Lee 2020-06-16 17:53:38 +00:00 committed by Commit Bot service account
parent ac6bd4c98f
commit 3fe84b5b33
4 changed files with 26 additions and 13 deletions

View File

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

View File

@ -19,10 +19,14 @@
namespace dawn_native { namespace dawn_native {
void AssertAndIgnoreDeviceLossError(MaybeError maybeError) { void IgnoreErrors(MaybeError maybeError) {
if (maybeError.IsError()) { if (maybeError.IsError()) {
std::unique_ptr<ErrorData> errorData = maybeError.AcquireError(); 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);
} }
} }

View File

@ -114,7 +114,7 @@ namespace dawn_native {
break break
// Assert that errors are device loss so that we can continue with destruction // 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); wgpu::ErrorType ToWGPUErrorType(InternalErrorType type);
InternalErrorType FromWGPUErrorType(wgpu::ErrorType type); InternalErrorType FromWGPUErrorType(wgpu::ErrorType type);

View File

@ -14,6 +14,8 @@
#include "dawn_native/d3d12/D3D12Error.h" #include "dawn_native/d3d12/D3D12Error.h"
#include <iomanip>
#include <sstream>
#include <string> #include <string>
namespace dawn_native { namespace d3d12 { namespace dawn_native { namespace d3d12 {
@ -22,12 +24,19 @@ namespace dawn_native { namespace d3d12 {
return {}; 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) { if (result == DXGI_ERROR_DEVICE_REMOVED) {
return DAWN_DEVICE_LOST_ERROR(message); return DAWN_DEVICE_LOST_ERROR(messageStream.str());
} else { } else {
return DAWN_INTERNAL_ERROR(message); return DAWN_INTERNAL_ERROR(messageStream.str());
} }
} }