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.

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,

lib/DxilContainer/DxilContainerAssembler.cpp

@@ -1008,6 +1008,93 @@ public:
   }
 };
 
+//////////////////////////////////////////////////////////
+// DxilVersionWriter - Writes VERS part
+class DxilVersionWriter : public DxilPartWriter {
+  hlsl::DxilCompilerVersion m_Header = {};
+  CComHeapPtr<char> m_CommitShaStorage;
+  llvm::StringRef m_CommitSha = "";
+  CComHeapPtr<char> 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<IDxcVersionInfo2> 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<IDxcVersionInfo3> 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<DxilVersionWriter> pVERSWriter = nullptr;
   std::unique_ptr<DxilRDATWriter> pRDATWriter = nullptr;
   std::unique_ptr<DxilPSVWriter> 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<DxilVersionWriter>(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<DxilRDATWriter>(*pModule);
     writer.AddPart(

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) {

tools/clang/tools/dxcompiler/dxcompilerobj.cpp

@@ -98,92 +98,12 @@ static bool ShouldBeCopiedIntoPDB(UINT32 FourCC) {
   return false;
 }
 
-struct CompilerVersionPartWriter {
-  hlsl::DxilCompilerVersion m_Header = {};
-  CComHeapPtr<char> m_CommitShaStorage;
-  llvm::StringRef m_CommitSha = "";
-  CComHeapPtr<char> 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<IDxcVersionInfo2> 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<IDxcVersionInfo3> 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<HRESULT(IStream *)> 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<UINT32, 4> OffsetTable;
-  SmallVector<Part, 4> 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<DxilContainerWriter> containerWriter(NewDxilContainerWriter(false));
+  std::unique_ptr<DxilPartWriter> 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<hlsl::AbstractMemoryStream> 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 @@ public:
               &compiler.getDiagnostics(), &ShaderHashContent, pReflectionStream,
               pRootSigStream, pRootSignatureBlob, pPrivateBlob);
 
+          inputs.pVersionInfo = static_cast<IDxcVersionInfo *>(this);
+
           if (needsValidation) {
             valHR = dxcutil::ValidateAndAssembleToContainer(inputs);
           } else {

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);
   }

tools/clang/tools/dxcompiler/dxcutil.h

@@ -55,6 +55,7 @@ struct AssembleInputs {
                  CComPtr<IDxcBlob> pPrivateBlob = nullptr);
   std::unique_ptr<llvm::Module> pM;
   CComPtr<IDxcBlob> &pOutputContainerBlob;
+  IDxcVersionInfo *pVersionInfo = nullptr;
   IMalloc *pMalloc;
   hlsl::SerializeDxilFlags SerializeFlags;
   CComPtr<hlsl::AbstractMemoryStream> &pModuleBitcode;

tools/clang/tools/dxrfallbackcompiler/dxcutil.cpp

@@ -110,7 +110,7 @@ void AssembleToContainer(AssembleInputs &inputs) {
   CComPtr<AbstractMemoryStream> 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));

tools/clang/unittests/HLSL/DxilContainerTest.cpp

@@ -111,6 +111,7 @@ public:
   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<IDxcCompiler> pCompiler;
+  CComPtr<IDxcBlobEncoding> pSource;
+  CComPtr<IDxcBlob> pProgram;
+  CComPtr<IDxcBlobEncoding> pDisassembly;
+  CComPtr<IDxcOperationResult> 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<IDxcVersionInfo> pVersionInfo;
+  CComPtr<IDxcVersionInfo2> pVersionInfo2;
+  CComPtr<IDxcVersionInfo3> pVersionInfo3;
+  pCompiler.QueryInterface(&pVersionInfo);
+  UINT major;
+  UINT minor;
+  UINT flags;
+  UINT commit_count;
+  CComHeapPtr<char> pCommitHashRef;
+  CComHeapPtr<char> 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<IDxcContainerReflection> 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<IDxcBlob> 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<IDxcCompiler> pCompiler;
   CComPtr<IDxcBlobEncoding> pSource;