Merged PR 5226: [DXIL] Add validation for CompilerVersion Dxil Part

After adding the new part to the library dxil containers, there was a lack of validation that the new part was constructed correctly. This PR aims to address this problem by adding a function to dxilvalidation.cpp to validate the new CompilerVersion part. With the validation in dxilvalidation.cpp, the container tests can now assume / rely on the validator that the part is well-formed, when making value checks on the parts.

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,
@@ -5660,8 +5733,10 @@ HRESULT ValidateDxilContainerParts(llvm::Module *pModule,
     case DFCC_CompilerVersion:
       // Either this blob is a PDB, or it is a library with shader model at least 6.8
       if (pDxilModule->GetShaderModel()->IsSM68Plus() && ValCtx.isLibProfile) {
-        // TODO: validate VERS part, emit error if malformed
-        continue;
+        if (!ValidateCompilerVersionPart((void *)GetDxilPartData(pPart), pPart->PartSize))
+        {
+            ValCtx.EmitFormatError(ValidationRule::ContainerPartInvalid, { szFourCC });
+        }
       }
       break;
 

tools/clang/unittests/HLSL/DxilContainerTest.cpp

@@ -2085,49 +2085,112 @@ 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 SM >= 6.8
+// Test to see if the Compiler Version (VERS) part gets added to library shaders
+// with SM >= 6.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_7", nullptr, 0, nullptr, 0, nullptr, &pResult));
+  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_7", 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_NULL(hlsl::GetDxilPartByType(pHeader, hlsl::DxilFourCC::DFCC_CompilerVersion));
-
+
+  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_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(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);
 
-  // TODO: ensure the version info has the expected contents by 
+  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 
+  // 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;
-  pVersionInfo->GetVersion(&major, &minor);
-  VERIFY_IS_TRUE(pVersionHeader->PartFourCC == hlsl::DxilFourCC::DFCC_CompilerVersion);
+  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);
+
+    VERIFY_ARE_EQUAL_STR(pCommitHashStr, pCommitHashRef);
+    if (pDCV->VersionStringListSizeInBytes > sizeof(pCommitHashStr) + 1) {
+      LPCSTR pCustomVersionString = pCommitHashStr + sizeof(pCommitHashStr) + 1;
+      VERIFY_ARE_EQUAL_STR(pCustomVersionString, pCustomVersionStrRef)
+    }
+  }
 }
 
 TEST_F(DxilContainerTest, DxilContainerUnitTest) {