Fixed QueryInterface for ID3D12ShaderReflection interface (#5445)

QueryInterface would accept any interface IID, instead of only the ones
it recognizes. Fixed by adding PublicAPI::Invalid, returned from
IIDToAPI when not one of the recognized APIs, and only accepting
IUnknown or the original interface IID provided at create time.

CreateDxil{Shader|Library}Reflection would also completely ignore IID.
Fixed by only accepting supported IIDs.

Get*ParameterDesc for signature elements also incorrectly computed
structure size (due to alignment) to memcpy for D3D11_43 version. Fixed
by copying only up to the end of the last field in the old structure
version.

CreateReflectionObjectsForSignature had a special case avoiding filling
in the MinPrecision member for D3D11_43, though this is unnecessary,
since for that API version, the struct will be truncated to exclude the
MinPrecision member when copying out the desc. Removed unnecessary
special case.

Added DxilContainerTest::CheckReflectionQueryInterface test for Create
and QI scenarios for supported and unsupported IIDs. This also tests
that the Get*ParameterDesc methods don't write beyond the structure size
for the interface version.

Fixes #3887
This commit is contained in:
Tex Riddell 2023-09-26 19:20:50 -07:00 коммит произвёл GitHub
Родитель e4b9b88c0c
Коммит 8e70fa2082
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
2 изменённых файлов: 335 добавлений и 21 удалений

Просмотреть файл

@ -99,7 +99,7 @@ public:
class CShaderReflectionConstantBuffer;
class CShaderReflectionType;
enum class PublicAPI { D3D12 = 0, D3D11_47 = 1, D3D11_43 = 2 };
enum class PublicAPI { D3D12 = 0, D3D11_47 = 1, D3D11_43 = 2, Invalid };
#ifdef ADD_16_64_BIT_TYPES
// Disable warning about value not being valid in enum
@ -182,8 +182,10 @@ public:
PublicAPI m_PublicAPI;
void SetPublicAPI(PublicAPI value) { m_PublicAPI = value; }
static PublicAPI IIDToAPI(REFIID iid) {
PublicAPI api = PublicAPI::D3D12;
if (IsEqualIID(IID_ID3D11ShaderReflection_43, iid))
PublicAPI api = PublicAPI::Invalid;
if (IsEqualIID(__uuidof(ID3D12ShaderReflection), iid))
api = PublicAPI::D3D12;
else if (IsEqualIID(IID_ID3D11ShaderReflection_43, iid))
api = PublicAPI::D3D11_43;
else if (IsEqualIID(IID_ID3D11ShaderReflection_47, iid))
api = PublicAPI::D3D11_47;
@ -193,17 +195,24 @@ public:
DXC_MICROCOM_TM_CTOR(DxilShaderReflection)
HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid,
void **ppvObject) override {
HRESULT hr =
DoBasicQueryInterface<ID3D12ShaderReflection>(this, iid, ppvObject);
if (hr == E_NOINTERFACE) {
// ID3D11ShaderReflection is identical to ID3D12ShaderReflection, except
// for some shorter data structures in some out parameters.
PublicAPI api = IIDToAPI(iid);
if (api == m_PublicAPI) {
*ppvObject = (ID3D12ShaderReflection *)this;
this->AddRef();
hr = S_OK;
}
HRESULT hr = E_NOINTERFACE;
// There is non-standard handling of QueryInterface:
// - although everything uses the same vtable as ID3D12ShaderReflection,
// there are differences in behavior depending on the API version, and
// there are 3 of these - it's not just d3d11 vs d3d12.
// - when the object is created the API version is fixed
// - from that point on, this object can only be QI'd for the matching API
// version.
PublicAPI api = IIDToAPI(iid);
if (api == m_PublicAPI) {
*ppvObject = static_cast<ID3D12ShaderReflection *>(this);
this->AddRef();
hr = S_OK;
} else if (IsEqualIID(__uuidof(IUnknown), iid)) {
*ppvObject = static_cast<IUnknown *>(this);
this->AddRef();
hr = S_OK;
}
return hr;
}
@ -299,10 +308,16 @@ HRESULT CreateDxilShaderReflection(const DxilProgramHeader *pProgramHeader,
void **ppvObject) {
if (!ppvObject)
return E_INVALIDARG;
PublicAPI api = DxilShaderReflection::IIDToAPI(iid);
if (api == PublicAPI::Invalid) {
if (IsEqualIID(__uuidof(IUnknown), iid))
api = PublicAPI::D3D12;
else
return E_NOINTERFACE;
}
CComPtr<DxilShaderReflection> pReflection =
DxilShaderReflection::Alloc(DxcGetThreadMallocNoRef());
IFROOM(pReflection.p);
PublicAPI api = DxilShaderReflection::IIDToAPI(iid);
pReflection->SetPublicAPI(api);
// pRDATPart to be used for transition.
IFR(pReflection->Load(pProgramHeader, pRDATPart));
@ -315,6 +330,9 @@ HRESULT CreateDxilLibraryReflection(const DxilProgramHeader *pProgramHeader,
void **ppvObject) {
if (!ppvObject)
return E_INVALIDARG;
if (!IsEqualIID(__uuidof(ID3D12LibraryReflection), iid) &&
!IsEqualIID(__uuidof(IUnknown), iid))
return E_NOINTERFACE;
CComPtr<DxilLibraryReflection> pReflection =
DxilLibraryReflection::Alloc(DxcGetThreadMallocNoRef());
IFROOM(pReflection.p);
@ -2160,9 +2178,7 @@ void DxilShaderReflection::CreateReflectionObjectsForSignature(
Desc.ComponentType =
CompTypeToRegisterComponentType(SigElem->GetCompType());
Desc.Mask = SigElem->GetColsAsMask();
// D3D11_43 does not have MinPrecison.
if (m_PublicAPI != PublicAPI::D3D11_43)
Desc.MinPrecision = CompTypeToMinPrecision(SigElem->GetCompType());
Desc.MinPrecision = CompTypeToMinPrecision(SigElem->GetCompType());
if (m_bUsageInMetadata) {
unsigned UsageMask = SigElem->GetUsageMask();
if (SigElem->IsAllocated())
@ -2585,7 +2601,8 @@ HRESULT DxilShaderReflection::GetInputParameterDesc(
else
memcpy(pDesc, &m_InputSignature[ParameterIndex],
// D3D11_43 does not have MinPrecison.
sizeof(D3D12_SIGNATURE_PARAMETER_DESC) - sizeof(D3D_MIN_PRECISION));
offsetof(D3D12_SIGNATURE_PARAMETER_DESC, Stream) +
sizeof(D3D12_SIGNATURE_PARAMETER_DESC::Stream));
return S_OK;
}
@ -2599,7 +2616,8 @@ HRESULT DxilShaderReflection::GetOutputParameterDesc(
else
memcpy(pDesc, &m_OutputSignature[ParameterIndex],
// D3D11_43 does not have MinPrecison.
sizeof(D3D12_SIGNATURE_PARAMETER_DESC) - sizeof(D3D_MIN_PRECISION));
offsetof(D3D12_SIGNATURE_PARAMETER_DESC, Stream) +
sizeof(D3D12_SIGNATURE_PARAMETER_DESC::Stream));
return S_OK;
}
@ -2613,7 +2631,8 @@ HRESULT DxilShaderReflection::GetPatchConstantParameterDesc(
else
memcpy(pDesc, &m_PatchConstantSignature[ParameterIndex],
// D3D11_43 does not have MinPrecison.
sizeof(D3D12_SIGNATURE_PARAMETER_DESC) - sizeof(D3D_MIN_PRECISION));
offsetof(D3D12_SIGNATURE_PARAMETER_DESC, Stream) +
sizeof(D3D12_SIGNATURE_PARAMETER_DESC::Stream));
return S_OK;
}

Просмотреть файл

@ -102,6 +102,7 @@ public:
TEST_METHOD(CompileWhenOkThenCheckRDAT2)
TEST_METHOD(CompileWhenOkThenCheckReflection1)
TEST_METHOD(DxcUtils_CreateReflection)
TEST_METHOD(CheckReflectionQueryInterface)
TEST_METHOD(CompileWhenOKThenIncludesFeatureInfo)
TEST_METHOD(CompileWhenOKThenIncludesSignatures)
TEST_METHOD(CompileWhenSigSquareThenIncludeSplit)
@ -1911,6 +1912,300 @@ TEST_F(DxilContainerTest, DxcUtils_CreateReflection) {
}
}
TEST_F(DxilContainerTest, CheckReflectionQueryInterface) {
// Minimum version 1.3 required for library support.
if (m_ver.SkipDxilVersion(1, 3))
return;
// Check that QueryInterface for shader and library reflection accepts/rejects
// interfaces properly
// Also check that DxilShaderReflection::Get*ParameterDesc methods do not
// write out-of-bounds for earlier interface version.
// Use domain shader to test DxilShaderReflection::Get*ParameterDesc because
// it can use all three signatures.
const char *shaderSource = R"(
struct PSInput {
noperspective float4 pos : SV_POSITION;
};
// Patch constant signature
struct HS_CONSTANT_DATA_OUTPUT {
float edges[3] : SV_TessFactor;
float inside : SV_InsideTessFactor;
};
// Patch input signature
struct HS_CONTROL_POINT {
float3 pos : POSITION;
};
// Domain shader
[domain("tri")]
void main(
OutputPatch<HS_CONTROL_POINT, 3> TrianglePatch,
HS_CONSTANT_DATA_OUTPUT pcIn,
float3 bary : SV_DomainLocation,
out PSInput output) {
output.pos = float4((bary.x * TrianglePatch[0].pos +
bary.y * TrianglePatch[1].pos +
bary.z * TrianglePatch[2].pos), 1);
}
)";
CComPtr<IDxcUtils> pUtils;
VERIFY_SUCCEEDED(m_dllSupport.CreateInstance(CLSID_DxcUtils, &pUtils));
CComPtr<IDxcCompiler> pCompiler;
VERIFY_SUCCEEDED(CreateCompiler(&pCompiler));
struct TestDescStruct {
D3D12_SIGNATURE_PARAMETER_DESC paramDesc;
// `pad` is used to ensure Get*ParameterDesc does not write out-of-bounds.
// It should still have the 0xFE byte pattern.
uint32_t pad;
TestDescStruct() { Clear(); }
void Clear() {
// fill this structure with 0xFE bytes
memset(this, 0xFE, sizeof(TestDescStruct));
}
bool CheckRemainingBytes(size_t offset) {
// Check that bytes in this struct after offset are still 0xFE
uint8_t *pBytes = (uint8_t *)this;
for (size_t i = offset; i < sizeof(TestDescStruct); i++) {
if (pBytes[i] != 0xFE)
return false;
}
return true;
}
bool CheckBytesAfterStream() {
// Check that bytes in this struct after Stream are still 0xFE
return CheckRemainingBytes(offsetof(TestDescStruct, paramDesc.Stream) +
sizeof(paramDesc.Stream));
}
bool CheckBytesAfterParamDesc() {
// Check that bytes in this struct after paramDesc are still 0xFE
return CheckRemainingBytes(offsetof(TestDescStruct, paramDesc) +
sizeof(paramDesc));
}
};
// ID3D12LibraryReflection
{
CComPtr<IDxcOperationResult> pResult;
CComPtr<IDxcResult> pResultV2;
CComPtr<IDxcBlob> pReflectionPart;
CComPtr<IDxcBlobEncoding> pSource;
CreateBlobFromText(Ref1_Shader, &pSource);
VERIFY_SUCCEEDED(pCompiler->Compile(pSource, L"hlsl.hlsl", L"", L"lib_6_3",
nullptr, 0, nullptr, 0, nullptr,
&pResult));
VERIFY_SUCCEEDED(pResult->QueryInterface(&pResultV2));
VERIFY_SUCCEEDED(pResultV2->GetOutput(
DXC_OUT_REFLECTION, IID_PPV_ARGS(&pReflectionPart), nullptr));
DxcBuffer buffer = {pReflectionPart->GetBufferPointer(),
pReflectionPart->GetBufferSize(), 0};
{
CComPtr<ID3D12LibraryReflection> pLibraryReflection;
VERIFY_SUCCEEDED(
pUtils->CreateReflection(&buffer, IID_PPV_ARGS(&pLibraryReflection)));
CComPtr<ID3D12LibraryReflection> pLibraryReflection2;
VERIFY_SUCCEEDED(pLibraryReflection->QueryInterface(
IID_PPV_ARGS(&pLibraryReflection2)));
CComPtr<IUnknown> pUnknown;
VERIFY_SUCCEEDED(
pLibraryReflection->QueryInterface(IID_PPV_ARGS(&pUnknown)));
CComPtr<ID3D12ShaderReflection> pShaderReflection;
VERIFY_FAILED(
pLibraryReflection->QueryInterface(IID_PPV_ARGS(&pShaderReflection)));
}
{ // Allow IUnknown
CComPtr<IUnknown> pUnknown;
VERIFY_SUCCEEDED(
pUtils->CreateReflection(&buffer, IID_PPV_ARGS(&pUnknown)));
CComPtr<ID3D12LibraryReflection> pLibraryReflection;
VERIFY_SUCCEEDED(
pUnknown->QueryInterface(IID_PPV_ARGS(&pLibraryReflection)));
}
{ // Fail to create with incorrect interface
CComPtr<ID3D12ShaderReflection> pShaderReflection;
VERIFY_ARE_EQUAL(
E_NOINTERFACE,
pUtils->CreateReflection(&buffer, IID_PPV_ARGS(&pShaderReflection)));
}
}
// Supported shader reflection interfaces defined by legacy APIs
const GUID IID_ID3D11ShaderReflection_43 = {
0x0a233719,
0x3960,
0x4578,
{0x9d, 0x7c, 0x20, 0x3b, 0x8b, 0x1d, 0x9c, 0xc1}};
const GUID IID_ID3D11ShaderReflection_47 = {
0x8d536ca1,
0x0cca,
0x4956,
{0xa8, 0x37, 0x78, 0x69, 0x63, 0x75, 0x55, 0x84}};
// Check ShaderReflection interface versions
{
CComPtr<IDxcOperationResult> pResult;
CComPtr<IDxcResult> pResultV2;
CComPtr<IDxcBlob> pReflectionPart;
CComPtr<IDxcBlobEncoding> pSource;
CreateBlobFromText(shaderSource, &pSource);
VERIFY_SUCCEEDED(pCompiler->Compile(pSource, L"hlsl.hlsl", L"main",
L"ds_6_0", nullptr, 0, nullptr, 0,
nullptr, &pResult));
VERIFY_SUCCEEDED(pResult->QueryInterface(&pResultV2));
VERIFY_SUCCEEDED(pResultV2->GetOutput(
DXC_OUT_REFLECTION, IID_PPV_ARGS(&pReflectionPart), nullptr));
DxcBuffer buffer = {pReflectionPart->GetBufferPointer(),
pReflectionPart->GetBufferSize(), 0};
// The interface version supported for QI should only be the one used for
// creating the original reflection interface.
{ // Verify with initial interface ID3D12ShaderReflection
CComPtr<ID3D12ShaderReflection> pShaderReflection;
VERIFY_SUCCEEDED(
pUtils->CreateReflection(&buffer, IID_PPV_ARGS(&pShaderReflection)));
// Verify QI for same interface and IUnknown succeeds:
CComPtr<ID3D12ShaderReflection> pShaderReflection2;
VERIFY_SUCCEEDED(
pShaderReflection->QueryInterface(IID_PPV_ARGS(&pShaderReflection2)));
CComPtr<IUnknown> pUnknown;
VERIFY_SUCCEEDED(
pShaderReflection->QueryInterface(IID_PPV_ARGS(&pUnknown)));
// Verify QI for wrong version of interface fails:
CComPtr<ID3D12ShaderReflection> pShaderReflection43;
VERIFY_FAILED(pShaderReflection->QueryInterface(
IID_ID3D11ShaderReflection_43, (void **)&pShaderReflection43));
CComPtr<ID3D12ShaderReflection> pShaderReflection47;
VERIFY_FAILED(pShaderReflection->QueryInterface(
IID_ID3D11ShaderReflection_47, (void **)&pShaderReflection47));
// Verify QI for wrong interface fails:
CComPtr<ID3D12LibraryReflection> pLibraryReflection;
VERIFY_FAILED(
pShaderReflection->QueryInterface(IID_PPV_ARGS(&pLibraryReflection)));
// Verify Get*ParameterDesc methods do not write out-of-bounds
TestDescStruct testParamDesc;
VERIFY_SUCCEEDED(pShaderReflection->GetInputParameterDesc(
0, &testParamDesc.paramDesc));
VERIFY_IS_TRUE(testParamDesc.CheckBytesAfterParamDesc());
VERIFY_SUCCEEDED(pShaderReflection->GetOutputParameterDesc(
0, &testParamDesc.paramDesc));
VERIFY_IS_TRUE(testParamDesc.CheckBytesAfterParamDesc());
VERIFY_SUCCEEDED(pShaderReflection->GetPatchConstantParameterDesc(
0, &testParamDesc.paramDesc));
VERIFY_IS_TRUE(testParamDesc.CheckBytesAfterParamDesc());
}
{ // Verify with initial interface IID_ID3D11ShaderReflection_47
CComPtr<ID3D12ShaderReflection> pShaderReflection;
VERIFY_SUCCEEDED(pUtils->CreateReflection(
&buffer, IID_ID3D11ShaderReflection_47, (void **)&pShaderReflection));
// Verify QI for same interface and IUnknown succeeds:
CComPtr<ID3D12ShaderReflection> pShaderReflection2;
VERIFY_SUCCEEDED(pShaderReflection->QueryInterface(
IID_ID3D11ShaderReflection_47, (void **)&pShaderReflection2));
CComPtr<IUnknown> pUnknown;
VERIFY_SUCCEEDED(
pShaderReflection->QueryInterface(IID_PPV_ARGS(&pUnknown)));
// Verify QI for wrong version of interface fails:
CComPtr<ID3D12ShaderReflection> pShaderReflection43;
VERIFY_FAILED(pShaderReflection->QueryInterface(
IID_ID3D11ShaderReflection_43, (void **)&pShaderReflection43));
CComPtr<ID3D12ShaderReflection> pShaderReflection12;
VERIFY_FAILED(pShaderReflection->QueryInterface(
IID_PPV_ARGS(&pShaderReflection12)));
// Verify QI for wrong interface fails:
CComPtr<ID3D12LibraryReflection> pLibraryReflection;
VERIFY_FAILED(
pShaderReflection->QueryInterface(IID_PPV_ARGS(&pLibraryReflection)));
// Verify Get*ParameterDesc methods do not write out-of-bounds
TestDescStruct testParamDesc;
VERIFY_SUCCEEDED(pShaderReflection->GetInputParameterDesc(
0, &testParamDesc.paramDesc));
VERIFY_IS_TRUE(testParamDesc.CheckBytesAfterParamDesc());
VERIFY_SUCCEEDED(pShaderReflection->GetOutputParameterDesc(
0, &testParamDesc.paramDesc));
VERIFY_IS_TRUE(testParamDesc.CheckBytesAfterParamDesc());
VERIFY_SUCCEEDED(pShaderReflection->GetPatchConstantParameterDesc(
0, &testParamDesc.paramDesc));
VERIFY_IS_TRUE(testParamDesc.CheckBytesAfterParamDesc());
}
{ // Verify with initial interface IID_ID3D11ShaderReflection_43
CComPtr<ID3D12ShaderReflection> pShaderReflection;
VERIFY_SUCCEEDED(pUtils->CreateReflection(
&buffer, IID_ID3D11ShaderReflection_43, (void **)&pShaderReflection));
// Verify QI for same interface and IUnknown succeeds:
CComPtr<ID3D12ShaderReflection> pShaderReflection2;
VERIFY_SUCCEEDED(pShaderReflection->QueryInterface(
IID_ID3D11ShaderReflection_43, (void **)&pShaderReflection2));
CComPtr<IUnknown> pUnknown;
VERIFY_SUCCEEDED(
pShaderReflection->QueryInterface(IID_PPV_ARGS(&pUnknown)));
// Verify QI for wrong version of interface fails:
CComPtr<ID3D12ShaderReflection> pShaderReflection47;
VERIFY_FAILED(pShaderReflection->QueryInterface(
IID_ID3D11ShaderReflection_47, (void **)&pShaderReflection47));
CComPtr<ID3D12ShaderReflection> pShaderReflection12;
VERIFY_FAILED(pShaderReflection->QueryInterface(
IID_PPV_ARGS(&pShaderReflection12)));
// Verify QI for wrong interface fails:
CComPtr<ID3D12LibraryReflection> pLibraryReflection;
VERIFY_FAILED(
pShaderReflection->QueryInterface(IID_PPV_ARGS(&pLibraryReflection)));
// Verify Get*ParameterDesc methods do not write out-of-bounds
// IID_ID3D11ShaderReflection_43 version of the structure does not have
// any feilds after Stream
TestDescStruct testParamDesc;
VERIFY_SUCCEEDED(pShaderReflection->GetInputParameterDesc(
0, &testParamDesc.paramDesc));
VERIFY_IS_TRUE(testParamDesc.CheckBytesAfterStream());
VERIFY_SUCCEEDED(pShaderReflection->GetOutputParameterDesc(
0, &testParamDesc.paramDesc));
VERIFY_IS_TRUE(testParamDesc.CheckBytesAfterStream());
VERIFY_SUCCEEDED(pShaderReflection->GetPatchConstantParameterDesc(
0, &testParamDesc.paramDesc));
VERIFY_IS_TRUE(testParamDesc.CheckBytesAfterStream());
}
{ // Allow IUnknown for latest interface version
CComPtr<IUnknown> pUnknown;
VERIFY_SUCCEEDED(
pUtils->CreateReflection(&buffer, IID_PPV_ARGS(&pUnknown)));
CComPtr<ID3D12ShaderReflection> pShaderReflection;
VERIFY_SUCCEEDED(
pUnknown->QueryInterface(IID_PPV_ARGS(&pShaderReflection)));
}
{ // Fail to create with incorrect interface
CComPtr<ID3D12LibraryReflection> pLibraryReflection;
VERIFY_ARE_EQUAL(
E_NOINTERFACE,
pUtils->CreateReflection(&buffer, IID_PPV_ARGS(&pLibraryReflection)));
}
}
}
TEST_F(DxilContainerTest, CompileWhenOKThenIncludesFeatureInfo) {
CComPtr<IDxcCompiler> pCompiler;
CComPtr<IDxcBlobEncoding> pSource;