From 6287d513d1f791715a9e3b1ad01e8287aeca0d6b Mon Sep 17 00:00:00 2001 From: Joshua Batista Date: Mon, 10 Jul 2023 11:01:06 -0700 Subject: [PATCH] Error on diff blob versions part 1 (#5378) This PR is the first of 2 PRs to prevent libraries of different compiler versions from linking together. This first PR implements adding the compiler version information into the Dxil Container itself, to allow for comparison later. It also adds some basic validation to the new part in the Dxil Container. --- .../DxilContainer/DxilContainerAssembler.h | 3 + lib/DxilContainer/DxilContainerAssembler.cpp | 112 +++++++++++ lib/HLSL/DxilValidation.cpp | 86 ++++++++ .../clang/tools/dxcompiler/dxcompilerobj.cpp | 186 +++--------------- tools/clang/tools/dxcompiler/dxcutil.cpp | 2 + tools/clang/tools/dxcompiler/dxcutil.h | 1 + .../tools/dxrfallbackcompiler/dxcutil.cpp | 2 +- .../unittests/HLSL/DxilContainerTest.cpp | 110 +++++++++++ 8 files changed, 339 insertions(+), 163 deletions(-) diff --git a/include/dxc/DxilContainer/DxilContainerAssembler.h b/include/dxc/DxilContainer/DxilContainerAssembler.h index c51f97dbec..6dd812ff82 100644 --- a/include/dxc/DxilContainer/DxilContainerAssembler.h +++ b/include/dxc/DxilContainer/DxilContainerAssembler.h @@ -15,6 +15,7 @@ #include "dxc/DxilContainer/DxilContainer.h" #include "llvm/ADT/StringRef.h" +struct IDxcVersionInfo; struct IStream; class DxilPipelineStateValidation; @@ -51,6 +52,7 @@ DxilPartWriter *NewRootSignatureWriter(const RootSignatureHandle &S); DxilPartWriter *NewFeatureInfoWriter(const DxilModule &M); DxilPartWriter *NewPSVWriter(const DxilModule &M, uint32_t PSVVersion = UINT_MAX); DxilPartWriter *NewRDATWriter(const DxilModule &M); +DxilPartWriter *NewVersionWriter(IDxcVersionInfo *pVersionInfo); // Store serialized ViewID data from DxilModule to PipelineStateValidation. void StoreViewIDStateToPSV(const uint32_t *pInputData, @@ -77,6 +79,7 @@ void WriteProgramPart(const hlsl::ShaderModel *pModel, void SerializeDxilContainerForModule( hlsl::DxilModule *pModule, AbstractMemoryStream *pModuleBitcode, + IDxcVersionInfo *DXCVersionInfo, AbstractMemoryStream *pStream, llvm::StringRef DebugName, SerializeDxilFlags Flags, DxilShaderHash *pShaderHashOut = nullptr, AbstractMemoryStream *pReflectionStreamOut = nullptr, diff --git a/lib/DxilContainer/DxilContainerAssembler.cpp b/lib/DxilContainer/DxilContainerAssembler.cpp index ced7ed22c8..2de867274e 100644 --- a/lib/DxilContainer/DxilContainerAssembler.cpp +++ b/lib/DxilContainer/DxilContainerAssembler.cpp @@ -1008,6 +1008,93 @@ class DxilPSVWriter : public DxilPartWriter { } }; +////////////////////////////////////////////////////////// +// DxilVersionWriter - Writes VERS part +class DxilVersionWriter : public DxilPartWriter { + hlsl::DxilCompilerVersion m_Header = {}; + CComHeapPtr m_CommitShaStorage; + llvm::StringRef m_CommitSha = ""; + CComHeapPtr m_CustomStringStorage; + llvm::StringRef m_CustomString = ""; +public: + DxilVersionWriter(IDxcVersionInfo *pVersion) + { + Init(pVersion); + } + + void Init(IDxcVersionInfo *pVersionInfo) { + m_Header = {}; + + UINT32 Major = 0, Minor = 0; + UINT32 Flags = 0; + IFT(pVersionInfo->GetVersion(&Major, &Minor)); + IFT(pVersionInfo->GetFlags(&Flags)); + + m_Header.Major = Major; + m_Header.Minor = Minor; + m_Header.VersionFlags = Flags; + CComPtr pVersionInfo2; + if (SUCCEEDED(pVersionInfo->QueryInterface(&pVersionInfo2))) { + UINT32 CommitCount = 0; + IFT(pVersionInfo2->GetCommitInfo(&CommitCount, &m_CommitShaStorage)); + m_CommitSha = llvm::StringRef(m_CommitShaStorage.m_pData, strlen(m_CommitShaStorage.m_pData)); + m_Header.CommitCount = CommitCount; + m_Header.VersionStringListSizeInBytes += m_CommitSha.size(); + } + m_Header.VersionStringListSizeInBytes += /*null term*/ 1; + + CComPtr pVersionInfo3; + if (SUCCEEDED(pVersionInfo->QueryInterface(&pVersionInfo3))) { + IFT(pVersionInfo3->GetCustomVersionString(&m_CustomStringStorage)); + m_CustomString = llvm::StringRef(m_CustomStringStorage, strlen(m_CustomStringStorage.m_pData)); + m_Header.VersionStringListSizeInBytes += m_CustomString.size(); + } + m_Header.VersionStringListSizeInBytes += /*null term*/ 1; + } + + static uint32_t PadToDword(uint32_t size, uint32_t *outNumPadding=nullptr) { + uint32_t rem = size % 4; + if (rem) { + uint32_t padding = (4 - rem); + if (outNumPadding) + *outNumPadding = padding; + return size + padding; + } + if (outNumPadding) + *outNumPadding = 0; + return size; + } + + UINT32 size() const override { + return PadToDword(sizeof(m_Header) + m_Header.VersionStringListSizeInBytes); + } + + void write(AbstractMemoryStream *pStream) override { + const uint8_t padByte = 0; + UINT32 uPadding = 0; + UINT32 uSize = PadToDword(sizeof(m_Header) + m_Header.VersionStringListSizeInBytes, &uPadding); + (void)uSize; + + ULONG cbWritten = 0; + IFT(pStream->Write(&m_Header, sizeof(m_Header), &cbWritten)); + + // Write a null terminator even if the string is empty + IFT(pStream->Write(m_CommitSha.data(), m_CommitSha.size(), &cbWritten)); + // Null terminator for the commit sha + IFT(pStream->Write(&padByte, sizeof(padByte), &cbWritten)); + + // Write the custom version string. + IFT(pStream->Write(m_CustomString.data(), m_CustomString.size(), &cbWritten)); + // Null terminator for the custom version string. + IFT(pStream->Write(&padByte, sizeof(padByte), &cbWritten)); + + // Write padding + for (unsigned i = 0; i < uPadding; i++) { + IFT(pStream->Write(&padByte, sizeof(padByte), &cbWritten)); + } + } +}; + using namespace DXIL; class DxilRDATWriter : public DxilPartWriter { @@ -1398,6 +1485,10 @@ DxilPartWriter *hlsl::NewRDATWriter(const DxilModule &M) { return new DxilRDATWriter(M); } +DxilPartWriter *hlsl::NewVersionWriter(IDxcVersionInfo *DXCVersionInfo) { + return new DxilVersionWriter(DXCVersionInfo); +} + class DxilContainerWriter_impl : public DxilContainerWriter { private: class DxilPart { @@ -1575,6 +1666,7 @@ void hlsl::StripAndCreateReflectionStream(Module *pReflectionM, uint32_t *pRefle void hlsl::SerializeDxilContainerForModule( DxilModule *pModule, AbstractMemoryStream *pModuleBitcode, + IDxcVersionInfo *DXCVersionInfo, AbstractMemoryStream *pFinalStream, llvm::StringRef DebugName, SerializeDxilFlags Flags, DxilShaderHash *pShaderHashOut, AbstractMemoryStream *pReflectionStreamOut, @@ -1590,6 +1682,7 @@ void hlsl::SerializeDxilContainerForModule( unsigned ValMajor, ValMinor; pModule->GetValidatorVersion(ValMajor, ValMinor); + bool bValidatorAtLeast_1_8 = DXIL::CompareVersions(ValMajor, ValMinor, 1, 8) >= 0; if (DXIL::CompareVersions(ValMajor, ValMinor, 1, 1) < 0) Flags &= ~SerializeDxilFlags::IncludeDebugNamePart; bool bSupportsShaderHash = DXIL::CompareVersions(ValMajor, ValMinor, 1, 5) >= 0; @@ -1646,8 +1739,11 @@ void hlsl::SerializeDxilContainerForModule( }); } } + + std::unique_ptr pVERSWriter = nullptr; std::unique_ptr pRDATWriter = nullptr; std::unique_ptr pPSVWriter = nullptr; + unsigned int major, minor; pModule->GetDxilVersion(major, minor); RootSignatureWriter rootSigWriter(std::move(pModule->GetSerializedRootSignature())); // Grab RS here @@ -1657,6 +1753,22 @@ void hlsl::SerializeDxilContainerForModule( if (pModule->GetShaderModel()->IsLib()) { DXASSERT(pModule->GetSerializedRootSignature().empty(), "otherwise, library has root signature outside subobject definitions"); + // Write the DxilCompilerVersion (VERS) part. + if (DXCVersionInfo && bValidatorAtLeast_1_8) { + + pVERSWriter = llvm::make_unique(DXCVersionInfo); + + writer.AddPart( + hlsl::DFCC_CompilerVersion, + pVERSWriter->size(), + [&pVERSWriter](AbstractMemoryStream *pStream) { + pVERSWriter->write(pStream); + return S_OK; + } + ); + } + + // Write the DxilRuntimeData (RDAT) part. pRDATWriter = llvm::make_unique(*pModule); writer.AddPart( diff --git a/lib/HLSL/DxilValidation.cpp b/lib/HLSL/DxilValidation.cpp index d1883e4594..9f95bc5661 100644 --- a/lib/HLSL/DxilValidation.cpp +++ b/lib/HLSL/DxilValidation.cpp @@ -5539,6 +5539,79 @@ static void VerifyFeatureInfoMatches(_In_ ValidationContext &ValCtx, VerifyBlobPartMatches(ValCtx, "Feature Info", pWriter.get(), pFeatureInfoData, FeatureInfoSize); } +// return true if the pBlob is a valid, well-formed CompilerVersion part, false +// otherwise +bool ValidateCompilerVersionPart(const void *pBlobPtr, UINT blobSize) { + // The hlsl::DxilCompilerVersion struct is always 16 bytes. (2 2-byte + // uint16's, 3 4-byte uint32's) The blob size should absolutely never be less + // than 16 bytes. + if (blobSize < sizeof(hlsl::DxilCompilerVersion)) { + return false; + } + + const hlsl::DxilCompilerVersion *pDCV = + (const hlsl::DxilCompilerVersion *)pBlobPtr; + if (pDCV->VersionStringListSizeInBytes == 0) { + // No version strings, just make sure there is no extra space. + return blobSize == sizeof(hlsl::DxilCompilerVersion); + } + + // after this point, we know VersionStringListSizeInBytes >= 1, because it is + // a UINT + + UINT EndOfVersionStringIndex = + sizeof(hlsl::DxilCompilerVersion) + pDCV->VersionStringListSizeInBytes; + // Make sure that the buffer size is large enough to contain both the DCV + // struct and the version string but not any larger than necessary + if (PSVALIGN4(EndOfVersionStringIndex) != blobSize) { + return false; + } + + const char *VersionStringsListData = + (const char *)pBlobPtr + sizeof(hlsl::DxilCompilerVersion); + UINT VersionStringListSizeInBytes = pDCV->VersionStringListSizeInBytes; + + // now make sure that any pad bytes that were added are null-terminators. + for (UINT i = VersionStringListSizeInBytes; + i < blobSize - sizeof(hlsl::DxilCompilerVersion); i++) { + if (VersionStringsListData[i] != '\0') { + return false; + } + } + + // Now, version string validation + // first, the final byte of the string should always be null-terminator so + // that the string ends + if (VersionStringsListData[VersionStringListSizeInBytes - 1] != '\0') { + return false; + } + + // construct the first string + // data format for VersionString can be see in the definition for the + // DxilCompilerVersion struct. summary: 2 strings that each end with the null + // terminator, and [0-3] null terminators after the final null terminator + StringRef firstStr(VersionStringsListData); + + // if the second string exists, attempt to construct it. + if (VersionStringListSizeInBytes > (firstStr.size() + 1)) { + StringRef secondStr(VersionStringsListData + firstStr.size() + 1); + + // the VersionStringListSizeInBytes member should be exactly equal to the + // two string lengths, plus the 2 null terminator bytes. + if (VersionStringListSizeInBytes != + firstStr.size() + secondStr.size() + 2) { + return false; + } + } else { + // the VersionStringListSizeInBytes member should be exactly equal to the + // first string length, plus the 1 null terminator byte. + if (VersionStringListSizeInBytes != firstStr.size() + 1) { + return false; + } + } + + return true; +} static void VerifyRDATMatches(_In_ ValidationContext &ValCtx, _In_reads_bytes_(RDATSize) const void *pRDATData, @@ -5657,6 +5730,19 @@ HRESULT ValidateDxilContainerParts(llvm::Module *pModule, case DFCC_FeatureInfo: VerifyFeatureInfoMatches(ValCtx, GetDxilPartData(pPart), pPart->PartSize); break; + case DFCC_CompilerVersion: + // This blob is either a PDB, or a library profile + if (ValCtx.isLibProfile) { + if (!ValidateCompilerVersionPart((void *)GetDxilPartData(pPart), pPart->PartSize)) + { + ValCtx.EmitFormatError(ValidationRule::ContainerPartInvalid, { szFourCC }); + } + } + else { + ValCtx.EmitFormatError(ValidationRule::ContainerPartInvalid, { szFourCC }); + } + break; + case DFCC_RootSignature: pRootSignaturePart = pPart; if (ValCtx.isLibProfile) { diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index 217d12e7c1..06771289d5 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -98,92 +98,12 @@ static bool ShouldBeCopiedIntoPDB(UINT32 FourCC) { return false; } -struct CompilerVersionPartWriter { - hlsl::DxilCompilerVersion m_Header = {}; - CComHeapPtr m_CommitShaStorage; - llvm::StringRef m_CommitSha = ""; - CComHeapPtr m_CustomStringStorage; - llvm::StringRef m_CustomString = ""; - - void Init(IDxcVersionInfo *pVersionInfo) { - m_Header = {}; - - UINT32 Major = 0, Minor = 0; - UINT32 Flags = 0; - IFT(pVersionInfo->GetVersion(&Major, &Minor)); - IFT(pVersionInfo->GetFlags(&Flags)); - - m_Header.Major = Major; - m_Header.Minor = Minor; - m_Header.VersionFlags = Flags; - CComPtr pVersionInfo2; - if (SUCCEEDED(pVersionInfo->QueryInterface(&pVersionInfo2))) { - UINT32 CommitCount = 0; - IFT(pVersionInfo2->GetCommitInfo(&CommitCount, &m_CommitShaStorage)); - m_CommitSha = llvm::StringRef(m_CommitShaStorage.m_pData, strlen(m_CommitShaStorage.m_pData)); - m_Header.CommitCount = CommitCount; - m_Header.VersionStringListSizeInBytes += m_CommitSha.size(); - } - m_Header.VersionStringListSizeInBytes += /*null term*/ 1; - - CComPtr pVersionInfo3; - if (SUCCEEDED(pVersionInfo->QueryInterface(&pVersionInfo3))) { - IFT(pVersionInfo3->GetCustomVersionString(&m_CustomStringStorage)); - m_CustomString = llvm::StringRef(m_CustomStringStorage, strlen(m_CustomStringStorage.m_pData)); - m_Header.VersionStringListSizeInBytes += m_CustomString.size(); - } - m_Header.VersionStringListSizeInBytes += /*null term*/ 1; - } - - static uint32_t PadToDword(uint32_t size, uint32_t *outNumPadding=nullptr) { - uint32_t rem = size % 4; - if (rem) { - uint32_t padding = (4 - rem); - if (outNumPadding) - *outNumPadding = padding; - return size + padding; - } - if (outNumPadding) - *outNumPadding = 0; - return size; - } - - UINT32 GetSize(UINT32 *pPadding = nullptr) const { - return PadToDword(sizeof(m_Header) + m_Header.VersionStringListSizeInBytes, pPadding); - } - - void Write(IStream *pStream) { - const uint8_t padByte = 0; - UINT32 uPadding = 0; - UINT32 uSize = GetSize(&uPadding); - (void)uSize; - - ULONG cbWritten = 0; - IFT(pStream->Write(&m_Header, sizeof(m_Header), &cbWritten)); - - // Write a null terminator even if the string is empty - IFT(pStream->Write(m_CommitSha.data(), m_CommitSha.size(), &cbWritten)); - // Null terminator for the commit sha - IFT(pStream->Write(&padByte, sizeof(padByte), &cbWritten)); - - // Write the custom version string. - IFT(pStream->Write(m_CustomString.data(), m_CustomString.size(), &cbWritten)); - // Null terminator for the custom version string. - IFT(pStream->Write(&padByte, sizeof(padByte), &cbWritten)); - - // Write padding - for (unsigned i = 0; i < uPadding; i++) { - IFT(pStream->Write(&padByte, sizeof(padByte), &cbWritten)); - } - } -}; - static HRESULT CreateContainerForPDB(IMalloc *pMalloc, IDxcBlob *pOldContainer, IDxcBlob *pDebugBlob, IDxcVersionInfo *pVersionInfo, const hlsl::DxilSourceInfo *pSourceInfo, AbstractMemoryStream *pReflectionStream, - IDxcBlob **ppNewContaner) + IDxcBlob **ppNewContainer) { // If the pContainer is not a valid container, give up. if (!hlsl::IsValidDxilContainer((hlsl::DxilContainerHeader *)pOldContainer->GetBufferPointer(), pOldContainer->GetBufferSize())) @@ -192,45 +112,22 @@ static HRESULT CreateContainerForPDB(IMalloc *pMalloc, hlsl::DxilContainerHeader *DxilHeader = (hlsl::DxilContainerHeader *)pOldContainer->GetBufferPointer(); hlsl::DxilProgramHeader *ProgramHeader = nullptr; - struct Part { - typedef std::function WriteProc; - UINT32 uFourCC = 0; - UINT32 uSize = 0; - WriteProc Writer; - - Part(UINT32 uFourCC, UINT32 uSize, WriteProc Writer) : - uFourCC(uFourCC), - uSize(uSize), - Writer(Writer) - {} - }; - - // Compute offset table. - SmallVector OffsetTable; - SmallVector PartWriters; - UINT32 uTotalPartsSize = 0; - - auto AddPart = [&PartWriters, &OffsetTable, &uTotalPartsSize](Part NewPart, UINT32 uSize) { - OffsetTable.push_back(uTotalPartsSize); - uTotalPartsSize += uSize + sizeof(hlsl::DxilPartHeader); - PartWriters.push_back(NewPart); - }; + std::unique_ptr containerWriter(NewDxilContainerWriter(false)); + std::unique_ptr pDxilVersionWriter(NewVersionWriter(pVersionInfo)); for (unsigned i = 0; i < DxilHeader->PartCount; i++) { hlsl::DxilPartHeader *PartHeader = GetDxilContainerPart(DxilHeader, i); if (ShouldBeCopiedIntoPDB(PartHeader->PartFourCC)) { UINT32 uSize = PartHeader->PartSize; const void *pPartData = PartHeader+1; - Part NewPart( - PartHeader->PartFourCC, + containerWriter->AddPart(PartHeader->PartFourCC, uSize, - [pPartData, uSize](IStream *pStream) { + [pPartData, uSize](AbstractMemoryStream *pStream) { ULONG uBytesWritten = 0; IFR(pStream->Write(pPartData, uSize, &uBytesWritten)); return S_OK; } - ); - AddPart(NewPart, uSize); + ); } // Could use any of these. We're mostly after the header version and all that. @@ -247,47 +144,38 @@ static HRESULT CreateContainerForPDB(IMalloc *pMalloc, if (pSourceInfo) { const UINT32 uPartSize = pSourceInfo->AlignedSizeInBytes; - Part NewPart( - hlsl::DFCC_ShaderSourceInfo, + containerWriter->AddPart(hlsl::DFCC_ShaderSourceInfo, uPartSize, [pSourceInfo](IStream *pStream) { ULONG uBytesWritten = 0; pStream->Write(pSourceInfo, pSourceInfo->AlignedSizeInBytes, &uBytesWritten); return S_OK; } - ); - - AddPart(NewPart, uPartSize); + ); } if (pReflectionStream) { const hlsl::DxilPartHeader *pReflectionPartHeader = (const hlsl::DxilPartHeader *)pReflectionStream->GetPtr(); - Part NewPart( - hlsl::DFCC_ShaderStatistics, + + containerWriter->AddPart(hlsl::DFCC_ShaderStatistics, pReflectionPartHeader->PartSize, [pReflectionPartHeader](IStream *pStream) { ULONG uBytesWritten = 0; pStream->Write(pReflectionPartHeader+1, pReflectionPartHeader->PartSize, &uBytesWritten); return S_OK; } - ); - AddPart(NewPart, pReflectionPartHeader->PartSize); - } - - CompilerVersionPartWriter versionWriter; + ); + } + if (pVersionInfo) { - versionWriter.Init(pVersionInfo); - - Part NewPart( - hlsl::DFCC_CompilerVersion, - versionWriter.GetSize(), - [&versionWriter](IStream *pStream) { - versionWriter.Write(pStream); + containerWriter->AddPart(hlsl::DFCC_CompilerVersion, + pDxilVersionWriter->size(), + [&pDxilVersionWriter](AbstractMemoryStream *pStream) { + pDxilVersionWriter->write(pStream); return S_OK; } - ); - AddPart(NewPart, versionWriter.GetSize()); + ); } if (pDebugBlob) { @@ -300,9 +188,8 @@ static HRESULT CreateContainerForPDB(IMalloc *pMalloc, UINT32 uPaddingSize = 0; UINT32 uPartSize = AlignByDword(sizeof(hlsl::DxilProgramHeader) + pDebugBlob->GetBufferSize(), &uPaddingSize); - - Part NewPart( - hlsl::DFCC_ShaderDebugInfoDXIL, + + containerWriter->AddPart(hlsl::DFCC_ShaderDebugInfoDXIL, uPartSize, [uPartSize, ProgramHeader, pDebugBlob, uPaddingSize](IStream *pStream) { hlsl::DxilProgramHeader Header = *ProgramHeader; @@ -321,40 +208,13 @@ static HRESULT CreateContainerForPDB(IMalloc *pMalloc, return S_OK; } ); - AddPart(NewPart, uPartSize); } - // Offset the offset table by the offset table itself - for (unsigned i = 0; i < OffsetTable.size(); i++) - OffsetTable[i] += sizeof(hlsl::DxilContainerHeader) + OffsetTable.size() * sizeof(UINT32); - - // Create the new header - hlsl::DxilContainerHeader NewDxilHeader = *DxilHeader; - NewDxilHeader.PartCount = OffsetTable.size(); - NewDxilHeader.ContainerSizeInBytes = - sizeof(NewDxilHeader) + - OffsetTable.size() * sizeof(UINT32) + - uTotalPartsSize; - - // Write it to the result stream - ULONG uSizeWritten = 0; CComPtr pStrippedContainerStream; IFR(hlsl::CreateMemoryStream(pMalloc, &pStrippedContainerStream)); - IFR(pStrippedContainerStream->Write(&NewDxilHeader, sizeof(NewDxilHeader), &uSizeWritten)); - - // Write offset table - IFR(pStrippedContainerStream->Write(OffsetTable.data(), OffsetTable.size() * sizeof(OffsetTable.data()[0]), &uSizeWritten)); - - for (unsigned i = 0; i < PartWriters.size(); i++) { - auto &Writer = PartWriters[i]; - hlsl::DxilPartHeader PartHeader = {}; - PartHeader.PartFourCC = Writer.uFourCC; - PartHeader.PartSize = Writer.uSize; - IFR(pStrippedContainerStream->Write(&PartHeader, sizeof(PartHeader), &uSizeWritten)); - IFR(Writer.Writer(pStrippedContainerStream)); - } - IFR(pStrippedContainerStream.QueryInterface(ppNewContaner)); + containerWriter->write(pStrippedContainerStream); + IFR(pStrippedContainerStream.QueryInterface(ppNewContainer)); return S_OK; } @@ -1154,6 +1014,8 @@ class DxcCompiler : public IDxcCompiler3, &compiler.getDiagnostics(), &ShaderHashContent, pReflectionStream, pRootSigStream, pRootSignatureBlob, pPrivateBlob); + inputs.pVersionInfo = static_cast(this); + if (needsValidation) { valHR = dxcutil::ValidateAndAssembleToContainer(inputs); } else { diff --git a/tools/clang/tools/dxcompiler/dxcutil.cpp b/tools/clang/tools/dxcompiler/dxcutil.cpp index 863fe04c5f..aef4d01c0b 100644 --- a/tools/clang/tools/dxcompiler/dxcutil.cpp +++ b/tools/clang/tools/dxcompiler/dxcutil.cpp @@ -119,12 +119,14 @@ void AssembleToContainer(AssembleInputs &inputs) { if (inputs.pPrivateBlob) { SerializeDxilContainerForModule( &inputs.pM->GetOrCreateDxilModule(), inputs.pModuleBitcode, + inputs.pVersionInfo, pContainerStream, inputs.DebugName, inputs.SerializeFlags, inputs.pShaderHashOut, inputs.pReflectionOut, inputs.pRootSigOut, inputs.pPrivateBlob->GetBufferPointer(), inputs.pPrivateBlob->GetBufferSize()); } else { SerializeDxilContainerForModule( &inputs.pM->GetOrCreateDxilModule(), inputs.pModuleBitcode, + inputs.pVersionInfo, pContainerStream, inputs.DebugName, inputs.SerializeFlags, inputs.pShaderHashOut, inputs.pReflectionOut, inputs.pRootSigOut); } diff --git a/tools/clang/tools/dxcompiler/dxcutil.h b/tools/clang/tools/dxcompiler/dxcutil.h index 5a33fc498d..a622d3b687 100644 --- a/tools/clang/tools/dxcompiler/dxcutil.h +++ b/tools/clang/tools/dxcompiler/dxcutil.h @@ -55,6 +55,7 @@ struct AssembleInputs { CComPtr pPrivateBlob = nullptr); std::unique_ptr pM; CComPtr &pOutputContainerBlob; + IDxcVersionInfo *pVersionInfo = nullptr; IMalloc *pMalloc; hlsl::SerializeDxilFlags SerializeFlags; CComPtr &pModuleBitcode; diff --git a/tools/clang/tools/dxrfallbackcompiler/dxcutil.cpp b/tools/clang/tools/dxrfallbackcompiler/dxcutil.cpp index 1effdf8ac4..347ea19ed2 100644 --- a/tools/clang/tools/dxrfallbackcompiler/dxcutil.cpp +++ b/tools/clang/tools/dxrfallbackcompiler/dxcutil.cpp @@ -110,7 +110,7 @@ void AssembleToContainer(AssembleInputs &inputs) { CComPtr pContainerStream; IFT(CreateMemoryStream(inputs.pMalloc, &pContainerStream)); SerializeDxilContainerForModule(&inputs.pM->GetOrCreateDxilModule(), - inputs.pModuleBitcode, pContainerStream, inputs.DebugName, inputs.SerializeFlags, + inputs.pModuleBitcode, nullptr, pContainerStream, inputs.DebugName, inputs.SerializeFlags, inputs.pShaderHashOut, inputs.pReflectionOut, inputs.pRootSigOut); inputs.pOutputContainerBlob.Release(); IFT(pContainerStream.QueryInterface(&inputs.pOutputContainerBlob)); diff --git a/tools/clang/unittests/HLSL/DxilContainerTest.cpp b/tools/clang/unittests/HLSL/DxilContainerTest.cpp index c2fb48b57e..38f0640595 100644 --- a/tools/clang/unittests/HLSL/DxilContainerTest.cpp +++ b/tools/clang/unittests/HLSL/DxilContainerTest.cpp @@ -111,6 +111,7 @@ class DxilContainerTest : public ::testing::Test { TEST_METHOD(DisassemblyWhenValidThenOK) TEST_METHOD(ValidateFromLL_Abs2) TEST_METHOD(DxilContainerUnitTest) + TEST_METHOD(DxilContainerCompilerVersionTest) TEST_METHOD(ContainerBuilder_AddPrivateForceLast) TEST_METHOD(ReflectionMatchesDXBC_CheckIn) @@ -2084,6 +2085,115 @@ TEST_F(DxilContainerTest, ValidateFromLL_Abs2) { CodeGenTestCheck(L"..\\CodeGenHLSL\\container\\abs2_m.ll"); } +// Test to see if the Compiler Version (VERS) part gets added to library shaders +// with validator version >= 1.8 +TEST_F(DxilContainerTest, DxilContainerCompilerVersionTest) { + if (m_ver.SkipDxilVersion(1, 8)) + return; + CComPtr pCompiler; + CComPtr pSource; + CComPtr pProgram; + CComPtr pDisassembly; + CComPtr pResult; + + VERIFY_SUCCEEDED(CreateCompiler(&pCompiler)); + CreateBlobFromText("export float4 main() : SV_Target { return 0; }", + &pSource); + // Test DxilContainer with ShaderDebugInfoDXIL + VERIFY_SUCCEEDED(pCompiler->Compile(pSource, L"hlsl.hlsl", L"main", + L"lib_6_3", nullptr, 0, nullptr, 0, + nullptr, &pResult)); + VERIFY_SUCCEEDED(pResult->GetResult(&pProgram)); + + const hlsl::DxilContainerHeader *pHeader = hlsl::IsDxilContainerLike( + pProgram->GetBufferPointer(), pProgram->GetBufferSize()); + VERIFY_IS_TRUE( + hlsl::IsValidDxilContainer(pHeader, pProgram->GetBufferSize())); + VERIFY_IS_NOT_NULL( + hlsl::IsDxilContainerLike(pHeader, pProgram->GetBufferSize())); + VERIFY_IS_NOT_NULL( + hlsl::GetDxilPartByType(pHeader, hlsl::DxilFourCC::DFCC_CompilerVersion)); + + pResult.Release(); + pProgram.Release(); + + // Test DxilContainer without ShaderDebugInfoDXIL + VERIFY_SUCCEEDED(pCompiler->Compile(pSource, L"hlsl.hlsl", L"main", + L"lib_6_8", nullptr, 0, nullptr, 0, + nullptr, &pResult)); + VERIFY_SUCCEEDED(pResult->GetResult(&pProgram)); + + pHeader = hlsl::IsDxilContainerLike(pProgram->GetBufferPointer(), + pProgram->GetBufferSize()); + VERIFY_IS_TRUE( + hlsl::IsValidDxilContainer(pHeader, pProgram->GetBufferSize())); + VERIFY_IS_NOT_NULL( + hlsl::IsDxilContainerLike(pHeader, pProgram->GetBufferSize())); + VERIFY_IS_NOT_NULL( + hlsl::GetDxilPartByType(pHeader, hlsl::DxilFourCC::DFCC_CompilerVersion)); + const hlsl::DxilPartHeader *pVersionHeader = + hlsl::GetDxilPartByType(pHeader, hlsl::DxilFourCC::DFCC_CompilerVersion); + + // ensure the version info has the expected contents by + // querying IDxcVersion interface from pCompiler and comparing + // against the contents inside of pProgram + + // Gather "true" information + CComPtr pVersionInfo; + CComPtr pVersionInfo2; + CComPtr pVersionInfo3; + pCompiler.QueryInterface(&pVersionInfo); + UINT major; + UINT minor; + UINT flags; + UINT commit_count; + CComHeapPtr pCommitHashRef; + CComHeapPtr pCustomVersionStrRef; + VERIFY_SUCCEEDED(pVersionInfo->GetVersion(&major, &minor)); + VERIFY_SUCCEEDED(pVersionInfo->GetFlags(&flags)); + VERIFY_SUCCEEDED(pCompiler.QueryInterface(&pVersionInfo2)); + VERIFY_SUCCEEDED( + pVersionInfo2->GetCommitInfo(&commit_count, &pCommitHashRef)); + VERIFY_SUCCEEDED(pCompiler.QueryInterface(&pVersionInfo3)); + VERIFY_SUCCEEDED( + pVersionInfo3->GetCustomVersionString(&pCustomVersionStrRef)); + + // test the "true" information against what's in the blob + VERIFY_IS_TRUE(pVersionHeader->PartFourCC == + hlsl::DxilFourCC::DFCC_CompilerVersion); + // test the rest of the contents (major, minor, etc.) + CComPtr containerReflection; + uint32_t partCount; + IFT(m_dllSupport.CreateInstance(CLSID_DxcContainerReflection, + &containerReflection)); + IFT(containerReflection->Load(pProgram)); + IFT(containerReflection->GetPartCount(&partCount)); + UINT part_index; + VERIFY_SUCCEEDED(containerReflection->FindFirstPartKind( + hlsl::DFCC_CompilerVersion, &part_index)); + + CComPtr pBlob; + IFT(containerReflection->GetPartContent(part_index, &pBlob)); + void *pBlobPtr = pBlob->GetBufferPointer(); + hlsl::DxilCompilerVersion *pDCV = (hlsl::DxilCompilerVersion *)pBlobPtr; + VERIFY_ARE_EQUAL(major, pDCV->Major); + VERIFY_ARE_EQUAL(minor, pDCV->Minor); + VERIFY_ARE_EQUAL(flags, pDCV->VersionFlags); + VERIFY_ARE_EQUAL(commit_count, pDCV->CommitCount); + + if (pDCV->VersionStringListSizeInBytes != 0) { + LPCSTR pCommitHashStr = (LPCSTR)pDCV + sizeof(hlsl::DxilCompilerVersion); + uint32_t uCommitHashLen = (uint32_t)strlen(pCommitHashStr); + VERIFY_ARE_EQUAL_STR(pCommitHashStr, pCommitHashRef); + + // + 2 for the two null terminators that are included in this size: + if (pDCV->VersionStringListSizeInBytes > uCommitHashLen + 2) { + LPCSTR pCustomVersionString = pCommitHashStr + uCommitHashLen + 1; + VERIFY_ARE_EQUAL_STR(pCustomVersionString, pCustomVersionStrRef) + } + } +} + TEST_F(DxilContainerTest, DxilContainerUnitTest) { CComPtr pCompiler; CComPtr pSource;