Skip to content

Commit

Permalink
[Validator] enable validator hash by default. (#6853)
Browse files Browse the repository at this point in the history
The changes affect both the internal validator (used within the DXIL
compiler) and external validation tools. Now, by default, validator hash
is enabled for all validation processes.

#6863 was created for tracking the skip hash discussion.

This is second step for #6808.
Fixes #6857
  • Loading branch information
python3kgae committed Aug 16, 2024
1 parent 0e7591a commit 913ae2c
Show file tree
Hide file tree
Showing 13 changed files with 219 additions and 56 deletions.
13 changes: 12 additions & 1 deletion include/dxc/DxilContainer/DxcContainerBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@

#pragma once

#include "dxc/DxilContainer/DxilContainer.h"
// Include Windows header early for DxilHash.h.
#include "dxc/Support/Global.h"
#include "dxc/Support/WinIncludes.h"

#include "dxc/DxilContainer/DxilContainer.h"
#include "dxc/DxilHash/DxilHash.h"
#include "dxc/Support/microcom.h"
#include "dxc/dxcapi.h"
#include "llvm/ADT/SmallVector.h"
Expand Down Expand Up @@ -46,6 +49,7 @@ class DxcContainerBuilder : public IDxcContainerBuilder {
m_warning = warning;
m_RequireValidation = false;
m_HasPrivateData = false;
m_HashFunction = nullptr;
}

protected:
Expand All @@ -66,6 +70,13 @@ class DxcContainerBuilder : public IDxcContainerBuilder {
const char *m_warning;
bool m_RequireValidation;
bool m_HasPrivateData;
// Function to compute hash when valid dxil container is built
// This is nullptr if loaded container has invalid hash
HASH_FUNCTION_PROTO *m_HashFunction;

void DetermineHashFunctionFromContainerContents(
const DxilContainerHeader *ContainerHeader);
void HashAndUpdate(DxilContainerHeader *ContainerHeader);

UINT32 ComputeContainerSize();
HRESULT UpdateContainerHeader(AbstractMemoryStream *pStream,
Expand Down
63 changes: 60 additions & 3 deletions lib/DxilContainer/DxcContainerBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
#include "dxc/DxilContainer/DxilContainer.h"
#include "dxc/Support/ErrorCodes.h"
#include "dxc/Support/FileIOHelper.h"
#include "dxc/Support/Global.h"
#include "dxc/Support/WinIncludes.h"
#include "dxc/Support/dxcapi.impl.h"
#include "dxc/Support/microcom.h"
#include "dxc/dxcapi.h"
Expand Down Expand Up @@ -47,6 +45,10 @@ HRESULT STDMETHODCALLTYPE DxcContainerBuilder::Load(IDxcBlob *pSource) {
pPartHeader->PartSize, &pBlob));
AddPart(DxilPart(pPartHeader->PartFourCC, pBlob));
}
// Collect hash function.
const DxilContainerHeader *Header =
(DxilContainerHeader *)pSource->GetBufferPointer();
DetermineHashFunctionFromContainerContents(Header);
return S_OK;
}
CATCH_CPP_RETURN_HRESULT();
Expand Down Expand Up @@ -164,9 +166,64 @@ DxcContainerBuilder::SerializeContainer(IDxcOperationResult **ppResult) {
{DxcOutputObject::DataOutput(DXC_OUT_OBJECT, pResult, DxcOutNoName),
DxcOutputObject::DataOutput(DXC_OUT_ERRORS, pErrorBlob, DxcOutNoName)},
ppResult));
return S_OK;
}
CATCH_CPP_RETURN_HRESULT();

if (ppResult == nullptr || *ppResult == nullptr)
return S_OK;

HRESULT HR;
(*ppResult)->GetStatus(&HR);
if (FAILED(HR))
return HR;

CComPtr<IDxcBlob> pObject;
IFR((*ppResult)->GetResult(&pObject));

// Add Hash.
LPVOID PTR = pObject->GetBufferPointer();
if (IsDxilContainerLike(PTR, pObject->GetBufferSize()))
HashAndUpdate((DxilContainerHeader *)PTR);
return S_OK;
}

// Try hashing the source contained in ContainerHeader using retail and debug
// hashing functions. If either of them match the stored result, set the
// HashFunction to the matching variant. If neither match, set it to null.
void DxcContainerBuilder::DetermineHashFunctionFromContainerContents(
const DxilContainerHeader *ContainerHeader) {
DXASSERT(ContainerHeader != nullptr &&
IsDxilContainerLike(ContainerHeader,
ContainerHeader->ContainerSizeInBytes),
"otherwise load function should have returned an error.");
constexpr uint32_t HashStartOffset =
offsetof(struct DxilContainerHeader, Version);
auto *DataToHash = (const BYTE *)ContainerHeader + HashStartOffset;
UINT AmountToHash = ContainerHeader->ContainerSizeInBytes - HashStartOffset;
BYTE Result[DxilContainerHashSize];
ComputeHashRetail(DataToHash, AmountToHash, Result);
if (0 == memcmp(Result, ContainerHeader->Hash.Digest, sizeof(Result))) {
m_HashFunction = ComputeHashRetail;
} else {
ComputeHashDebug(DataToHash, AmountToHash, Result);
if (0 == memcmp(Result, ContainerHeader->Hash.Digest, sizeof(Result)))
m_HashFunction = ComputeHashDebug;
else
m_HashFunction = nullptr;
}
}

// For Internal hash function.
void DxcContainerBuilder::HashAndUpdate(DxilContainerHeader *ContainerHeader) {
if (m_HashFunction != nullptr) {
DXASSERT(ContainerHeader != nullptr,
"Otherwise serialization should have failed.");
static const UINT32 HashStartOffset =
offsetof(struct DxilContainerHeader, Version);
const BYTE *DataToHash = (const BYTE *)ContainerHeader + HashStartOffset;
UINT AmountToHash = ContainerHeader->ContainerSizeInBytes - HashStartOffset;
m_HashFunction(DataToHash, AmountToHash, ContainerHeader->Hash.Digest);
}
}

UINT32 DxcContainerBuilder::ComputeContainerSize() {
Expand Down
24 changes: 24 additions & 0 deletions tools/clang/tools/dxa/dxa.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ static cl::opt<bool> DumpReflection("dumpreflection",
cl::desc("Dump reflection"),
cl::init(false));

static cl::opt<bool> DumpHash("dumphash", cl::desc("Dump validation hash"),
cl::init(false));

class DxaContext {

private:
Expand All @@ -88,6 +91,7 @@ class DxaContext {
void DumpRS();
void DumpRDAT();
void DumpReflection();
void DumpValidationHash();
};

void DxaContext::Assemble() {
Expand Down Expand Up @@ -466,6 +470,23 @@ void DxaContext::DumpReflection() {
printf("%s", ss.str().c_str());
}

void DxaContext::DumpValidationHash() {
CComPtr<IDxcBlobEncoding> pSource;
ReadFileIntoBlob(m_dxcSupport, StringRefWide(InputFilename), &pSource);
if (!hlsl::IsValidDxilContainer(
(hlsl::DxilContainerHeader *)pSource->GetBufferPointer(),
pSource->GetBufferSize())) {
printf("Invalid input file, use binary DxilContainer.");
return;
}
hlsl::DxilContainerHeader *pDxilContainerHeader =
(hlsl::DxilContainerHeader *)pSource->GetBufferPointer();
printf("Validation hash: 0x");
for (size_t i = 0; i < hlsl::DxilContainerHashSize; i++) {
printf("%02x", pDxilContainerHeader->Hash.Digest[i]);
}
}

using namespace hlsl::options;

#ifdef _WIN32
Expand Down Expand Up @@ -527,6 +548,9 @@ int main(int argc, const char **argv) {
} else if (DumpReflection) {
pStage = "Dump Reflection";
context.DumpReflection();
} else if (DumpHash) {
pStage = "Dump Validation Hash";
context.DumpValidationHash();
} else {
pStage = "Assembling";
context.Assemble();
Expand Down
2 changes: 1 addition & 1 deletion tools/clang/tools/dxclib/dxc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ int DxcContext::VerifyRootSignature() {
IFT(pContainerBuilder->AddPart(hlsl::DxilFourCC::DFCC_RootSignature,
pRootSignature));
CComPtr<IDxcOperationResult> pOperationResult;
IFT(pContainerBuilder->SerializeContainer(&pOperationResult));
pContainerBuilder->SerializeContainer(&pOperationResult);
HRESULT status = E_FAIL;
CComPtr<IDxcBlob> pResult;
IFT(pOperationResult->GetStatus(&status));
Expand Down
2 changes: 1 addition & 1 deletion tools/clang/tools/dxcompiler/dxclinker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ HRESULT STDMETHODCALLTYPE DxcLinker::Link(
HRESULT valHR = S_OK;
dxcutil::AssembleInputs inputs(
std::move(pM), pOutputBlob, DxcGetThreadMallocNoRef(),
SerializeFlags, pOutputStream, opts.DebugFile, &Diag,
SerializeFlags, pOutputStream, 0, opts.DebugFile, &Diag,
&ShaderHashContent, pReflectionStream, pRootSigStream, nullptr,
nullptr);
if (needsValidation) {
Expand Down
2 changes: 1 addition & 1 deletion tools/clang/tools/dxcompiler/dxcompilerobj.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1039,7 +1039,7 @@ class DxcCompiler : public IDxcCompiler3,

dxcutil::AssembleInputs inputs(
std::move(serializeModule), pOutputBlob, m_pMalloc,
SerializeFlags, pOutputStream, opts.GetPDBName(),
SerializeFlags, pOutputStream, 0, opts.GetPDBName(),
&compiler.getDiagnostics(), &ShaderHashContent, pReflectionStream,
pRootSigStream, pRootSignatureBlob, pPrivateBlob,
opts.SelectValidator);
Expand Down
39 changes: 14 additions & 25 deletions tools/clang/tools/dxcompiler/dxcutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,18 @@ AssembleInputs::AssembleInputs(
std::unique_ptr<llvm::Module> &&pM, CComPtr<IDxcBlob> &pOutputContainerBlob,
IMalloc *pMalloc, hlsl::SerializeDxilFlags SerializeFlags,
CComPtr<hlsl::AbstractMemoryStream> &pModuleBitcode,
llvm::StringRef DebugName, clang::DiagnosticsEngine *pDiag,
hlsl::DxilShaderHash *pShaderHashOut, AbstractMemoryStream *pReflectionOut,
AbstractMemoryStream *pRootSigOut, CComPtr<IDxcBlob> pRootSigBlob,
CComPtr<IDxcBlob> pPrivateBlob,
uint32_t ValidationFlags, llvm::StringRef DebugName,
clang::DiagnosticsEngine *pDiag, hlsl::DxilShaderHash *pShaderHashOut,
AbstractMemoryStream *pReflectionOut, AbstractMemoryStream *pRootSigOut,
CComPtr<IDxcBlob> pRootSigBlob, CComPtr<IDxcBlob> pPrivateBlob,
hlsl::options::ValidatorSelection SelectValidator)
: pM(std::move(pM)), pOutputContainerBlob(pOutputContainerBlob),
pMalloc(pMalloc), SerializeFlags(SerializeFlags),
pModuleBitcode(pModuleBitcode), DebugName(DebugName), pDiag(pDiag),
pShaderHashOut(pShaderHashOut), pReflectionOut(pReflectionOut),
pRootSigOut(pRootSigOut), pRootSigBlob(pRootSigBlob),
pPrivateBlob(pPrivateBlob), SelectValidator(SelectValidator) {}
ValidationFlags(ValidationFlags), pModuleBitcode(pModuleBitcode),
DebugName(DebugName), pDiag(pDiag), pShaderHashOut(pShaderHashOut),
pReflectionOut(pReflectionOut), pRootSigOut(pRootSigOut),
pRootSigBlob(pRootSigBlob), pPrivateBlob(pPrivateBlob),
SelectValidator(SelectValidator) {}

void GetValidatorVersion(unsigned *pMajor, unsigned *pMinor,
hlsl::options::ValidatorSelection SelectValidator) {
Expand Down Expand Up @@ -174,18 +175,6 @@ HRESULT ValidateAndAssembleToContainer(AssembleInputs &inputs) {
pValidator.QueryInterface(&pValidator2);
}

if (bInternalValidator &&
inputs.SelectValidator != hlsl::options::ValidatorSelection::Internal) {
if (inputs.pDiag) {
unsigned diagID = inputs.pDiag->getCustomDiagID(
clang::DiagnosticsEngine::Level::Warning,
"DXIL signing library (dxil.dll,libdxil.so) not found. Resulting "
"DXIL will not be "
"signed for use in release environments.\r\n");
inputs.pDiag->Report(diagID);
}
}

if (bInternalValidator || pValidator2) {
// If using the internal validator or external validator supports
// IDxcValidator2, we'll use the modules directly. In this case, we'll want
Expand Down Expand Up @@ -222,13 +211,13 @@ HRESULT ValidateAndAssembleToContainer(AssembleInputs &inputs) {
CComPtr<IDxcOperationResult> pValResult;
// Important: in-place edit is required so the blob is reused and thus
// dxil.dll can be released.
inputs.ValidationFlags |= DxcValidatorFlags_InPlaceEdit;
if (bInternalValidator) {
IFT(RunInternalValidator(pValidator, llvmModuleWithDebugInfo.get(),
inputs.pOutputContainerBlob,
DxcValidatorFlags_InPlaceEdit, &pValResult));
inputs.ValidationFlags, &pValResult));
} else {
if (pValidator2 && llvmModuleWithDebugInfo) {

// If metadata was stripped, re-serialize the input module.
CComPtr<AbstractMemoryStream> pDebugModuleStream;
IFT(CreateMemoryStream(DxcGetThreadMallocNoRef(), &pDebugModuleStream));
Expand All @@ -241,11 +230,11 @@ HRESULT ValidateAndAssembleToContainer(AssembleInputs &inputs) {
debugModule.Size = pDebugModuleStream->GetPtrSize();

IFT(pValidator2->ValidateWithDebug(inputs.pOutputContainerBlob,
DxcValidatorFlags_InPlaceEdit,
&debugModule, &pValResult));
inputs.ValidationFlags, &debugModule,
&pValResult));
} else {
IFT(pValidator->Validate(inputs.pOutputContainerBlob,
DxcValidatorFlags_InPlaceEdit, &pValResult));
inputs.ValidationFlags, &pValResult));
}
}
IFT(pValResult->GetStatus(&valHR));
Expand Down
2 changes: 2 additions & 0 deletions tools/clang/tools/dxcompiler/dxcutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ struct AssembleInputs {
CComPtr<IDxcBlob> &pOutputContainerBlob, IMalloc *pMalloc,
hlsl::SerializeDxilFlags SerializeFlags,
CComPtr<hlsl::AbstractMemoryStream> &pModuleBitcode,
uint32_t ValidationFlags = 0,
llvm::StringRef DebugName = llvm::StringRef(),
clang::DiagnosticsEngine *pDiag = nullptr,
hlsl::DxilShaderHash *pShaderHashOut = nullptr,
Expand All @@ -61,6 +62,7 @@ struct AssembleInputs {
IDxcVersionInfo *pVersionInfo = nullptr;
IMalloc *pMalloc;
hlsl::SerializeDxilFlags SerializeFlags;
uint32_t ValidationFlags = 0;
CComPtr<hlsl::AbstractMemoryStream> &pModuleBitcode;
llvm::StringRef DebugName = llvm::StringRef();
clang::DiagnosticsEngine *pDiag;
Expand Down
1 change: 1 addition & 0 deletions tools/clang/tools/dxcvalidator/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ set( LLVM_LINK_COMPONENTS
dxcsupport
DXIL
DxilContainer
DxilHash
DxilValidation
Option # option library
Support # just for assert and raw streams
Expand Down
Loading

0 comments on commit 913ae2c

Please sign in to comment.