From 6487acdf0f387517ead16274c235fb975b2ad1e9 Mon Sep 17 00:00:00 2001 From: baldurk Date: Thu, 27 Jun 2019 09:14:19 +0100 Subject: [PATCH] Fail to capture if Create*PipelineState has no valid shader code * This typically means that the user hasn't checked correctly for SM6 feature support before trying to upload DXIL shader, which will then result in unpredictable behaviour or crashes on replay. * During capture we detect this and flag it in the overlay text, and prevent capturing. On capture load we fail to load if we detect such a PSO. --- docs/behind_scenes/d3d12_support.rst | 7 ++ renderdoc/api/replay/renderdoc_tostr.inl | 2 +- renderdoc/driver/d3d12/d3d12_device.cpp | 7 ++ renderdoc/driver/d3d12/d3d12_device.h | 2 + renderdoc/driver/d3d12/d3d12_device_wrap.cpp | 76 +++++++++++++++++++ .../driver/shaders/dxbc/dxbc_inspect.cpp | 25 ++++++ renderdoc/driver/shaders/dxbc/dxbc_inspect.h | 1 + 7 files changed, 119 insertions(+), 1 deletion(-) diff --git a/docs/behind_scenes/d3d12_support.rst b/docs/behind_scenes/d3d12_support.rst index c85b03796..5e308edac 100644 --- a/docs/behind_scenes/d3d12_support.rst +++ b/docs/behind_scenes/d3d12_support.rst @@ -24,6 +24,13 @@ RenderDoc has initial support for D3D12, but it contains some caveats. In additi * Pixel history is not implemented. * Shader debugging is not currently supported. +DXIL support +------------ + +Currently DXIL is not supported. RenderDoc uses the standard functionality through ``ID3D12Device::CheckFeatureSupport`` to report that SM6 is unsupported, and a valid application must check this before creating PSOs that use DXIL code since not all versions of Windows 10 support DXIL either. + +If such a PSO is detected, RenderDoc will display an overlay message to that effect and prevent capturing. + See Also -------- diff --git a/renderdoc/api/replay/renderdoc_tostr.inl b/renderdoc/api/replay/renderdoc_tostr.inl index 607b1a399..967574dc0 100644 --- a/renderdoc/api/replay/renderdoc_tostr.inl +++ b/renderdoc/api/replay/renderdoc_tostr.inl @@ -39,7 +39,7 @@ rdcstr DoStringise(const ReplayStatus &el) STRINGISE_ENUM_CLASS_NAMED(FileIOFailed, "File I/O failed"); STRINGISE_ENUM_CLASS_NAMED(FileIncompatibleVersion, "File of incompatible version"); STRINGISE_ENUM_CLASS_NAMED(FileCorrupted, "File corrupted"); - STRINGISE_ENUM_CLASS_NAMED(APIUnsupported, "API is not unsupported"); + STRINGISE_ENUM_CLASS_NAMED(APIUnsupported, "API is unsupported"); STRINGISE_ENUM_CLASS_NAMED(APIInitFailed, "API initialisation failed"); STRINGISE_ENUM_CLASS_NAMED(APIIncompatibleVersion, "Captured API data has an incompatible version"); diff --git a/renderdoc/driver/d3d12/d3d12_device.cpp b/renderdoc/driver/d3d12/d3d12_device.cpp index 6ef867022..8107d189a 100644 --- a/renderdoc/driver/d3d12/d3d12_device.cpp +++ b/renderdoc/driver/d3d12/d3d12_device.cpp @@ -1389,6 +1389,9 @@ HRESULT WrappedID3D12Device::Present(WrappedIDXGISwapChain4 *swap, UINT SyncInte std::string overlayText = RenderDoc::Inst().GetOverlayText(RDCDriver::D3D12, m_FrameCounter, flags); + if(m_InvalidPSO) + overlayText += "ERROR: Invalid PSO created, likely using DXIL which is not supported.\n"; + if(!overlayText.empty()) m_TextRenderer->RenderText(list, 0.0f, 0.0f, overlayText.c_str()); @@ -1409,6 +1412,10 @@ HRESULT WrappedID3D12Device::Present(WrappedIDXGISwapChain4 *swap, UINT SyncInte if(!activeWindow) return S_OK; + // disallow capturing if an invalid PSO has been created + if(m_InvalidPSO) + return S_OK; + // kill any current capture that isn't application defined if(IsActiveCapturing(m_State) && !m_AppControlledCapture) RenderDoc::Inst().EndFrameCapture((ID3D12Device *)this, swapdesc.OutputWindow); diff --git a/renderdoc/driver/d3d12/d3d12_device.h b/renderdoc/driver/d3d12/d3d12_device.h index 90accde1c..7cae8cf55 100644 --- a/renderdoc/driver/d3d12/d3d12_device.h +++ b/renderdoc/driver/d3d12/d3d12_device.h @@ -363,6 +363,8 @@ private: bool m_AppControlledCapture; + bool m_InvalidPSO = false; + Threading::RWLock m_CapTransitionLock; CaptureState m_State; diff --git a/renderdoc/driver/d3d12/d3d12_device_wrap.cpp b/renderdoc/driver/d3d12/d3d12_device_wrap.cpp index b3c477029..a6078454a 100644 --- a/renderdoc/driver/d3d12/d3d12_device_wrap.cpp +++ b/renderdoc/driver/d3d12/d3d12_device_wrap.cpp @@ -378,6 +378,33 @@ bool WrappedID3D12Device::Serialise_CreateGraphicsPipelineState( D3D12_GRAPHICS_PIPELINE_STATE_DESC unwrappedDesc = Descriptor; unwrappedDesc.pRootSignature = Unwrap(unwrappedDesc.pRootSignature); + // check for bytecode - if the user is wrongly using DXIL we will fail to load the capture + { + D3D12_SHADER_BYTECODE *shaders[] = { + &unwrappedDesc.VS, &unwrappedDesc.HS, &unwrappedDesc.DS, + &unwrappedDesc.GS, &unwrappedDesc.PS, + }; + + const char *name[] = {"VS", "HS", "DS", "GS", "PS"}; + + for(size_t i = 0; i < ARRAY_COUNT(shaders); i++) + { + if(shaders[i]->BytecodeLength > 0 && shaders[i]->pShaderBytecode) + { + if(!DXBC::DXBCFile::CheckForShaderCode(shaders[i]->pShaderBytecode, + shaders[i]->BytecodeLength)) + { + RDCERR( + "No shader code found in %s bytecode in pipeline state. " + "DXIL is unsupported and must be checked for using CheckFeatureSupport.", + name[i]); + m_FailedReplayStatus = ReplayStatus::APIReplayFailed; + return false; + } + } + } + } + ID3D12PipelineState *ret = NULL; HRESULT hr = m_pDevice->CreateGraphicsPipelineState(&unwrappedDesc, guid, (void **)&ret); @@ -487,6 +514,32 @@ HRESULT WrappedID3D12Device::CreateGraphicsPipelineState(const D3D12_GRAPHICS_PI if(SUCCEEDED(ret)) { + // check for bytecode - if the user is wrongly using DXIL we will prevent capturing + { + D3D12_SHADER_BYTECODE *shaders[] = { + &unwrappedDesc.VS, &unwrappedDesc.HS, &unwrappedDesc.DS, + &unwrappedDesc.GS, &unwrappedDesc.PS, + }; + + const char *name[] = {"VS", "HS", "DS", "GS", "PS"}; + + for(size_t i = 0; i < ARRAY_COUNT(shaders); i++) + { + if(shaders[i]->BytecodeLength > 0 && shaders[i]->pShaderBytecode) + { + if(!DXBC::DXBCFile::CheckForShaderCode(shaders[i]->pShaderBytecode, + shaders[i]->BytecodeLength)) + { + RDCERR( + "No shader code found in %s bytecode in pipeline state. " + "DXIL is unsupported and must be checked for using CheckFeatureSupport.", + name[i]); + m_InvalidPSO = true; + } + } + } + } + WrappedID3D12PipelineState *wrapped = new WrappedID3D12PipelineState(real, this); if(IsCaptureMode(m_State)) @@ -594,6 +647,18 @@ bool WrappedID3D12Device::Serialise_CreateComputePipelineState( D3D12_COMPUTE_PIPELINE_STATE_DESC unwrappedDesc = Descriptor; unwrappedDesc.pRootSignature = Unwrap(unwrappedDesc.pRootSignature); + // check for bytecode - if the user is wrongly using DXIL we will hard-fail instead of producing + // a corrupted capture. + if(!DXBC::DXBCFile::CheckForShaderCode(unwrappedDesc.CS.pShaderBytecode, + unwrappedDesc.CS.BytecodeLength)) + { + RDCERR( + "No shader code found in CS bytecode in pipeline state. " + "DXIL is unsupported and must be checked for using CheckFeatureSupport."); + m_FailedReplayStatus = ReplayStatus::APIReplayFailed; + return false; + } + ID3D12PipelineState *ret = NULL; HRESULT hr = m_pDevice->CreateComputePipelineState(&unwrappedDesc, guid, (void **)&ret); @@ -647,6 +712,17 @@ HRESULT WrappedID3D12Device::CreateComputePipelineState(const D3D12_COMPUTE_PIPE if(SUCCEEDED(ret)) { + // check for bytecode - if the user is wrongly using DXIL we will hard-fail instead of producing + // a corrupted capture. + if(!DXBC::DXBCFile::CheckForShaderCode(unwrappedDesc.CS.pShaderBytecode, + unwrappedDesc.CS.BytecodeLength)) + { + RDCERR( + "No shader code found in CS bytecode in pipeline state. " + "DXIL is unsupported and must be checked for using CheckFeatureSupport."); + m_InvalidPSO = true; + } + WrappedID3D12PipelineState *wrapped = new WrappedID3D12PipelineState(real, this); if(IsCaptureMode(m_State)) diff --git a/renderdoc/driver/shaders/dxbc/dxbc_inspect.cpp b/renderdoc/driver/shaders/dxbc/dxbc_inspect.cpp index e2bb32299..454f7746b 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_inspect.cpp +++ b/renderdoc/driver/shaders/dxbc/dxbc_inspect.cpp @@ -495,6 +495,31 @@ bool DXBCFile::CheckForDebugInfo(const void *ByteCode, size_t ByteCodeLength) return false; } +bool DXBCFile::CheckForShaderCode(const void *ByteCode, size_t ByteCodeLength) +{ + FileHeader *header = (FileHeader *)ByteCode; + + char *data = (char *)ByteCode; // just for convenience + + if(header->fourcc != FOURCC_DXBC) + return false; + + if(header->fileLength != (uint32_t)ByteCodeLength) + return false; + + uint32_t *chunkOffsets = (uint32_t *)(header + 1); // right after the header + + for(uint32_t chunkIdx = 0; chunkIdx < header->numChunks; chunkIdx++) + { + uint32_t *fourcc = (uint32_t *)(data + chunkOffsets[chunkIdx]); + + if(*fourcc == FOURCC_SHEX || *fourcc == FOURCC_SHDR) + return true; + } + + return false; +} + std::string DXBCFile::GetDebugBinaryPath(const void *ByteCode, size_t ByteCodeLength) { std::string debugPath; diff --git a/renderdoc/driver/shaders/dxbc/dxbc_inspect.h b/renderdoc/driver/shaders/dxbc/dxbc_inspect.h index 7b8f58d88..8579b2105 100644 --- a/renderdoc/driver/shaders/dxbc/dxbc_inspect.h +++ b/renderdoc/driver/shaders/dxbc/dxbc_inspect.h @@ -392,6 +392,7 @@ public: static void GetHash(uint32_t hash[4], const void *ByteCode, size_t BytecodeLength); static bool CheckForDebugInfo(const void *ByteCode, size_t ByteCodeLength); + static bool CheckForShaderCode(const void *ByteCode, size_t ByteCodeLength); static std::string GetDebugBinaryPath(const void *ByteCode, size_t ByteCodeLength); private: