Fix Validation for RDAT and other issues with Subobjects (#1706)

- re-serialize module when changed by removing either subobjects or
  root signature metadata
- improve error detection/handling for loading/creating subobjects
- load subobjects from RDAT if not in DxilModule for RDAT comparison
- fix subobjects missing from desc in DxilRuntimeReflection
- default ReducibilityAnalysis to pass so it doesn't fail when no
  functions are present
- report error on subobject if validator (< 1.4) will not accept
  them, so they will not be captured
- container validation for required/disallowed parts for libraries

include/dxc/DXIL/DxilModule.h

@@ -147,10 +147,10 @@ public:
   // Includes: vs/hs/ds/gs/ps/cs as well as the patch constant function.
   bool IsEntryThatUsesSignatures(const llvm::Function *F) const ;
 
-  // Remove Root Signature from module metadata
-  void StripRootSignatureFromMetadata();
-  // Remove Subobjects from module metadata
-  void StripSubobjectsFromMetadata();
+  // Remove Root Signature from module metadata, return true if changed
+  bool StripRootSignatureFromMetadata();
+  // Remove Subobjects from module metadata, return true if changed
+  bool StripSubobjectsFromMetadata();
   // Update validator version metadata to current setting
   void UpdateValidatorVersionMetadata();
 

include/dxc/DxilContainer/DxilContainerReader.h

@@ -18,7 +18,7 @@ namespace RDAT {
   class SubobjectTableReader;
 }
 
-void LoadSubobjectsFromRDAT(DxilSubobjects &subobjects,
+bool LoadSubobjectsFromRDAT(DxilSubobjects &subobjects,
   RDAT::SubobjectTableReader *pSubobjectTableReader);
 
 } // namespace hlsl

include/dxc/DxilContainer/DxilRuntimeReflection.h

@@ -583,7 +583,7 @@ private:
 
 public:
   DxilRuntimeData();
-  DxilRuntimeData(const char *ptr, size_t size);
+  DxilRuntimeData(const void *ptr, size_t size);
   // initializing reader from RDAT. return true if no error has occured.
   bool InitFromRDAT(const void *pRDAT, size_t size);
   // read prerelease data:

include/dxc/DxilContainer/DxilRuntimeReflection.inl

@@ -91,7 +91,7 @@ public:
 
 DxilRuntimeData::DxilRuntimeData() : DxilRuntimeData(nullptr, 0) {}
 
-DxilRuntimeData::DxilRuntimeData(const char *ptr, size_t size)
+DxilRuntimeData::DxilRuntimeData(const void *ptr, size_t size)
     : m_TableCount(0), m_StringReader(), m_IndexTableReader(), m_RawBytesReader(),
       m_ResourceTableReader(), m_FunctionTableReader(),
       m_SubobjectTableReader(), m_Context() {
@@ -342,6 +342,9 @@ const DxilLibraryDesc DxilRuntimeReflection_impl::GetLibraryReflection() {
     reflection.NumFunctions =
         m_RuntimeData.GetFunctionTableReader()->GetNumFunctions();
     reflection.pFunction = m_Functions.data();
+    reflection.NumSubobjects =
+        m_RuntimeData.GetSubobjectTableReader()->GetCount();
+    reflection.pSubobjects = m_Subobjects.data();
   }
   return reflection;
 }

lib/Analysis/ReducibilityAnalysis.cpp

@@ -50,10 +50,10 @@ public:
 
   ReducibilityAnalysis()
       : FunctionPass(ID), m_Action(IrreducibilityAction::ThrowException),
-        m_bReducible(false) {}
+        m_bReducible(true) {}
 
   explicit ReducibilityAnalysis(IrreducibilityAction Action)
-      : FunctionPass(ID), m_Action(Action), m_bReducible(false) {}
+      : FunctionPass(ID), m_Action(Action), m_bReducible(true) {}
 
   virtual bool runOnFunction(Function &F);
 

lib/DXIL/DxilModule.cpp

@@ -1011,11 +1011,13 @@ bool DxilModule::IsEntryThatUsesSignatures(const llvm::Function *F) const {
   return IsPatchConstantShader(F);
 }
 
-void DxilModule::StripRootSignatureFromMetadata() {
+bool DxilModule::StripRootSignatureFromMetadata() {
   NamedMDNode *pRootSignatureNamedMD = GetModule()->getNamedMetadata(DxilMDHelper::kDxilRootSignatureMDName);
   if (pRootSignatureNamedMD) {
     GetModule()->eraseNamedMetadata(pRootSignatureNamedMD);
+    return true;
   }
+  return false;
 }
 
 DxilSubobjects *DxilModule::GetSubobjects() {
@@ -1031,11 +1033,13 @@ void DxilModule::ResetSubobjects(DxilSubobjects *subobjects) {
   m_pSubobjects.reset(subobjects);
 }
 
-void DxilModule::StripSubobjectsFromMetadata() {
+bool DxilModule::StripSubobjectsFromMetadata() {
   NamedMDNode *pSubobjectsNamedMD = GetModule()->getNamedMetadata(DxilMDHelper::kDxilSubobjectsMDName);
   if (pSubobjectsNamedMD) {
     GetModule()->eraseNamedMetadata(pSubobjectsNamedMD);
+    return true;
   }
+  return false;
 }
 
 void DxilModule::UpdateValidatorVersionMetadata() {

lib/DXIL/DxilSubobject.cpp

@@ -311,8 +311,8 @@ DxilSubobject &DxilSubobjects::CreateHitGroup(llvm::StringRef Name,
 
 DxilSubobject &DxilSubobjects::CreateSubobject(Kind kind, llvm::StringRef Name) {
   Name = GetSubobjectString(Name);
-  DXASSERT(FindSubobject(Name) == nullptr,
-    "otherwise, name collision between subobjects");
+  IFTBOOLMSG(FindSubobject(Name) == nullptr, DXC_E_GENERAL_INTERNAL_ERROR, "Subobject name collision");
+  IFTBOOLMSG(!Name.empty(), DXC_E_GENERAL_INTERNAL_ERROR, "Empty Subobject name");
   std::unique_ptr<DxilSubobject> ptr(new DxilSubobject(*this, kind, Name));
   DxilSubobject &ref = *ptr;
   m_Subobjects[Name] = std::move(ptr);

lib/DxilContainer/DxilContainerAssembler.cpp

@@ -1526,28 +1526,36 @@ void hlsl::SerializeDxilContainerForModule(DxilModule *pModule,
   std::unique_ptr<DxilPSVWriter> pPSVWriter = nullptr;
   unsigned int major, minor;
   pModule->GetDxilVersion(major, minor);
+  RootSignatureWriter rootSigWriter(pModule->GetSerializedRootSignature());
+
+  bool bModuleDirty = false;
   if (pModule->GetShaderModel()->IsLib()) {
+    DXASSERT(pModule->GetSerializedRootSignature().empty(),
+             "otherwise, library has root signature outside subobject definitions");
     // Write the DxilRuntimeData (RDAT) part.
     pRDATWriter = llvm::make_unique<DxilRDATWriter>(*pModule);
     writer.AddPart(
         DFCC_RuntimeData, pRDATWriter->size(),
         [&](AbstractMemoryStream *pStream) { pRDATWriter->write(pStream); });
-    pModule->StripSubobjectsFromMetadata();
+    bModuleDirty |= pModule->StripSubobjectsFromMetadata();
   } else {
     // Write the DxilPipelineStateValidation (PSV0) part.
     pPSVWriter = llvm::make_unique<DxilPSVWriter>(*pModule);
     writer.AddPart(
         DFCC_PipelineStateValidation, pPSVWriter->size(),
         [&](AbstractMemoryStream *pStream) { pPSVWriter->write(pStream); });
-  }
-  // Write the root signature (RTS0) part.
-  RootSignatureWriter rootSigWriter(pModule->GetSerializedRootSignature());
-  CComPtr<AbstractMemoryStream> pInputProgramStream = pModuleBitcode;
-  if (!pModule->GetSerializedRootSignature().empty()) {
-    writer.AddPart(
+    // Write the root signature (RTS0) part.
+    if (!pModule->GetSerializedRootSignature().empty()) {
+      writer.AddPart(
         DFCC_RootSignature, rootSigWriter.size(),
         [&](AbstractMemoryStream *pStream) { rootSigWriter.write(pStream); });
-    pModule->StripRootSignatureFromMetadata();
+      bModuleDirty |= pModule->StripRootSignatureFromMetadata();
+    }
+  }
+
+  // If metadata was stripped, re-serialize the module.
+  CComPtr<AbstractMemoryStream> pInputProgramStream = pModuleBitcode;
+  if (bModuleDirty) {
     pInputProgramStream.Release();
     IFT(CreateMemoryStream(DxcGetThreadMallocNoRef(), &pInputProgramStream));
     raw_stream_ostream outStream(pInputProgramStream.p);

lib/DxilContainer/DxilContainerReader.cpp

@@ -18,57 +18,66 @@
 
 namespace hlsl {
 
-void LoadSubobjectsFromRDAT(DxilSubobjects &subobjects, RDAT::SubobjectTableReader *pSubobjectTableReader) {
+bool LoadSubobjectsFromRDAT(DxilSubobjects &subobjects, RDAT::SubobjectTableReader *pSubobjectTableReader) {
   if (!pSubobjectTableReader)
-    return;
+    return false;
+  bool result = true;
   for (unsigned i = 0; i < pSubobjectTableReader->GetCount(); ++i) {
-    auto reader = pSubobjectTableReader->GetItem(i);
-    DXIL::SubobjectKind kind = reader.GetKind();
-    bool bLocalRS = false;
-    switch (kind) {
-    case DXIL::SubobjectKind::StateObjectConfig:
-      subobjects.CreateStateObjectConfig(reader.GetName(),
-        reader.GetStateObjectConfig_Flags());
-      break;
-    case DXIL::SubobjectKind::LocalRootSignature:
-      bLocalRS = true;
-    case DXIL::SubobjectKind::GlobalRootSignature: {
-      const void *pOutBytes;
-      uint32_t OutSizeInBytes;
-      reader.GetRootSignature(&pOutBytes, &OutSizeInBytes);
-      subobjects.CreateRootSignature(reader.GetName(), bLocalRS, pOutBytes, OutSizeInBytes);
-      break;
-    }
-    case DXIL::SubobjectKind::SubobjectToExportsAssociation: {
-      uint32_t NumExports = reader.GetSubobjectToExportsAssociation_NumExports();
-      std::vector<llvm::StringRef> Exports;
-      Exports.resize(NumExports);
-      for (unsigned i = 0; i < NumExports; ++i) {
-        Exports[i] = reader.GetSubobjectToExportsAssociation_Export(i);
+    try {
+      auto reader = pSubobjectTableReader->GetItem(i);
+      DXIL::SubobjectKind kind = reader.GetKind();
+      bool bLocalRS = false;
+      switch (kind) {
+      case DXIL::SubobjectKind::StateObjectConfig:
+        subobjects.CreateStateObjectConfig(reader.GetName(),
+          reader.GetStateObjectConfig_Flags());
+        break;
+      case DXIL::SubobjectKind::LocalRootSignature:
+        bLocalRS = true;
+      case DXIL::SubobjectKind::GlobalRootSignature: {
+        const void *pOutBytes;
+        uint32_t OutSizeInBytes;
+        if (!reader.GetRootSignature(&pOutBytes, &OutSizeInBytes)) {
+          result = false;
+          continue;
+        }
+        subobjects.CreateRootSignature(reader.GetName(), bLocalRS, pOutBytes, OutSizeInBytes);
+        break;
       }
-      subobjects.CreateSubobjectToExportsAssociation(reader.GetName(),
-        reader.GetSubobjectToExportsAssociation_Subobject(),
-        Exports.data(), NumExports);
-      break;
-    }
-    case DXIL::SubobjectKind::RaytracingShaderConfig:
-      subobjects.CreateRaytracingShaderConfig(reader.GetName(),
-        reader.GetRaytracingShaderConfig_MaxPayloadSizeInBytes(),
-        reader.GetRaytracingShaderConfig_MaxAttributeSizeInBytes());
-      break;
-    case DXIL::SubobjectKind::RaytracingPipelineConfig:
-      subobjects.CreateRaytracingPipelineConfig(reader.GetName(),
-        reader.GetRaytracingPipelineConfig_MaxTraceRecursionDepth());
-      break;
-    case DXIL::SubobjectKind::HitGroup:
-      subobjects.CreateHitGroup(reader.GetName(),
-        reader.GetHitGroup_Type(),
-        reader.GetHitGroup_AnyHit(),
-        reader.GetHitGroup_ClosestHit(),
-        reader.GetHitGroup_Intersection());
+      case DXIL::SubobjectKind::SubobjectToExportsAssociation: {
+        uint32_t NumExports = reader.GetSubobjectToExportsAssociation_NumExports();
+        std::vector<llvm::StringRef> Exports;
+        Exports.resize(NumExports);
+        for (unsigned i = 0; i < NumExports; ++i) {
+          Exports[i] = reader.GetSubobjectToExportsAssociation_Export(i);
+        }
+        subobjects.CreateSubobjectToExportsAssociation(reader.GetName(),
+          reader.GetSubobjectToExportsAssociation_Subobject(),
+          Exports.data(), NumExports);
         break;
+      }
+      case DXIL::SubobjectKind::RaytracingShaderConfig:
+        subobjects.CreateRaytracingShaderConfig(reader.GetName(),
+          reader.GetRaytracingShaderConfig_MaxPayloadSizeInBytes(),
+          reader.GetRaytracingShaderConfig_MaxAttributeSizeInBytes());
+        break;
+      case DXIL::SubobjectKind::RaytracingPipelineConfig:
+        subobjects.CreateRaytracingPipelineConfig(reader.GetName(),
+          reader.GetRaytracingPipelineConfig_MaxTraceRecursionDepth());
+        break;
+      case DXIL::SubobjectKind::HitGroup:
+        subobjects.CreateHitGroup(reader.GetName(),
+          reader.GetHitGroup_Type(),
+          reader.GetHitGroup_AnyHit(),
+          reader.GetHitGroup_ClosestHit(),
+          reader.GetHitGroup_Intersection());
+          break;
+      }
+    } catch (hlsl::Exception &) {
+      result = false;
     }
   }
+  return result;
 }
 
 

lib/HLSL/DxilValidation.cpp

@@ -9,20 +9,23 @@
 //                                                                           //
 ///////////////////////////////////////////////////////////////////////////////
 
+#include "dxc/Support/Global.h"
+#include "dxc/Support/WinIncludes.h"
+#include "dxc/Support/FileIOHelper.h"
+
 #include "dxc/HLSL/DxilValidation.h"
 #include "dxc/DxilContainer/DxilContainerAssembler.h"
+#include "dxc/DxilContainer/DxilRuntimeReflection.h"
+#include "dxc/DxilContainer/DxilContainerReader.h"
 #include "dxc/HLSL/DxilGenerationPass.h"
 #include "dxc/DXIL/DxilOperations.h"
 #include "dxc/DXIL/DxilModule.h"
 #include "dxc/DXIL/DxilShaderModel.h"
 #include "dxc/DxilContainer/DxilContainer.h"
 #include "dxc/DXIL/DxilFunctionProps.h"
-#include "dxc/Support/Global.h"
 #include "dxc/DXIL/DxilUtil.h"
 #include "dxc/DXIL/DxilInstructions.h"
 #include "llvm/Analysis/ReducibilityAnalysis.h"
-#include "dxc/Support/WinIncludes.h"
-#include "dxc/Support/FileIOHelper.h"
 #include "dxc/DXIL/DxilEntryProps.h"
 
 #include "llvm/ADT/ArrayRef.h"
@@ -5245,11 +5248,34 @@ static void VerifyFeatureInfoMatches(_In_ ValidationContext &ValCtx,
   VerifyBlobPartMatches(ValCtx, "Feature Info", pWriter.get(), pFeatureInfoData, FeatureInfoSize);
 }
 
+
 static void VerifyRDATMatches(_In_ ValidationContext &ValCtx,
                               _In_reads_bytes_(RDATSize) const void *pRDATData,
                               _In_ uint32_t RDATSize) {
+  const char *PartName = "Runtime Data (RDAT)";
+  // If DxilModule subobjects already loaded, validate these against the RDAT blob,
+  // otherwise, load subobject into DxilModule to generate reference RDAT.
+  if (!ValCtx.DxilMod.GetSubobjects()) {
+    RDAT::DxilRuntimeData rdat(pRDATData, RDATSize);
+    auto *pSubobjReader = rdat.GetSubobjectTableReader();
+    if (pSubobjReader && pSubobjReader->GetCount() > 0) {
+      ValCtx.DxilMod.ResetSubobjects(new DxilSubobjects());
+      if (!LoadSubobjectsFromRDAT(*ValCtx.DxilMod.GetSubobjects(), pSubobjReader)) {
+        ValCtx.EmitFormatError(ValidationRule::ContainerPartMatches, { PartName });
+        return;
+      }
+    }
+  }
+
   unique_ptr<DxilPartWriter> pWriter(NewRDATWriter(ValCtx.DxilMod, 0));
-  VerifyBlobPartMatches(ValCtx, "Runtime Data (RDAT)", pWriter.get(), pRDATData, RDATSize);
+  VerifyBlobPartMatches(ValCtx, PartName, pWriter.get(), pRDATData, RDATSize);
+
+  // Verify no errors when runtime reflection from RDAT:
+  RDAT::DxilRuntimeReflection *pReflection = RDAT::CreateDxilRuntimeReflection();
+  if (!pReflection->InitFromRDAT(pRDATData, RDATSize)) {
+    ValCtx.EmitFormatError(ValidationRule::ContainerPartMatches, { PartName });
+    return;
+  }
 }
 
 _Use_decl_annotations_
@@ -5325,16 +5351,28 @@ HRESULT ValidateDxilContainerParts(llvm::Module *pModule,
     switch (pPart->PartFourCC)
     {
     case DFCC_InputSignature:
-      VerifySignatureMatches(ValCtx, DXIL::SignatureKind::Input, GetDxilPartData(pPart), pPart->PartSize);
+      if (ValCtx.isLibProfile) {
+        ValCtx.EmitFormatError(ValidationRule::ContainerPartInvalid, { szFourCC });
+      } else {
+        VerifySignatureMatches(ValCtx, DXIL::SignatureKind::Input, GetDxilPartData(pPart), pPart->PartSize);
+      }
       break;
     case DFCC_OutputSignature:
-      VerifySignatureMatches(ValCtx, DXIL::SignatureKind::Output, GetDxilPartData(pPart), pPart->PartSize);
+      if (ValCtx.isLibProfile) {
+        ValCtx.EmitFormatError(ValidationRule::ContainerPartInvalid, { szFourCC });
+      } else {
+        VerifySignatureMatches(ValCtx, DXIL::SignatureKind::Output, GetDxilPartData(pPart), pPart->PartSize);
+      }
       break;
     case DFCC_PatchConstantSignature:
-      if (bTess) {
-        VerifySignatureMatches(ValCtx, DXIL::SignatureKind::PatchConstant, GetDxilPartData(pPart), pPart->PartSize);
+      if (ValCtx.isLibProfile) {
+        ValCtx.EmitFormatError(ValidationRule::ContainerPartInvalid, { szFourCC });
       } else {
-        ValCtx.EmitFormatError(ValidationRule::ContainerPartMatches, {"Program Patch Constant Signature"});
+        if (bTess) {
+          VerifySignatureMatches(ValCtx, DXIL::SignatureKind::PatchConstant, GetDxilPartData(pPart), pPart->PartSize);
+        } else {
+          ValCtx.EmitFormatError(ValidationRule::ContainerPartMatches, {"Program Patch Constant Signature"});
+        }
       }
       break;
     case DFCC_FeatureInfo:
@@ -5342,10 +5380,17 @@ HRESULT ValidateDxilContainerParts(llvm::Module *pModule,
       break;
     case DFCC_RootSignature:
       pRootSignaturePart = pPart;
+      if (ValCtx.isLibProfile) {
+        ValCtx.EmitFormatError(ValidationRule::ContainerPartInvalid, { szFourCC });
+      }
       break;
     case DFCC_PipelineStateValidation:
       pPSVPart = pPart;
-      VerifyPSVMatches(ValCtx, GetDxilPartData(pPart), pPart->PartSize);
+      if (ValCtx.isLibProfile) {
+        ValCtx.EmitFormatError(ValidationRule::ContainerPartInvalid, { szFourCC });
+      } else {
+        VerifyPSVMatches(ValCtx, GetDxilPartData(pPart), pPart->PartSize);
+      }
       break;
 
     // Skip these
@@ -5359,7 +5404,11 @@ HRESULT ValidateDxilContainerParts(llvm::Module *pModule,
 
     // Runtime Data (RDAT) for libraries
     case DFCC_RuntimeData:
-      VerifyRDATMatches(ValCtx, GetDxilPartData(pPart), pPart->PartSize);
+      if (ValCtx.isLibProfile) {
+        VerifyRDATMatches(ValCtx, GetDxilPartData(pPart), pPart->PartSize);
+      } else {
+        ValCtx.EmitFormatError(ValidationRule::ContainerPartInvalid, { szFourCC });
+      }
       break;
 
     case DFCC_Container:
@@ -5370,41 +5419,46 @@ HRESULT ValidateDxilContainerParts(llvm::Module *pModule,
   }
 
   // Verify required parts found
-  if (FourCCFound.find(DFCC_InputSignature) == FourCCFound.end() && !ValCtx.isLibProfile) {
-    VerifySignatureMatches(ValCtx, DXIL::SignatureKind::Input, nullptr, 0);
-  }
-  if (FourCCFound.find(DFCC_OutputSignature) == FourCCFound.end() && !ValCtx.isLibProfile) {
-    VerifySignatureMatches(ValCtx, DXIL::SignatureKind::Output, nullptr, 0);
-  }
-  if (bTess && FourCCFound.find(DFCC_PatchConstantSignature) == FourCCFound.end() &&
-      pDxilModule->GetPatchConstantSignature().GetElements().size())
-  {
-    ValCtx.EmitFormatError(ValidationRule::ContainerPartMissing, {"Program Patch Constant Signature"});
-  }
-  if (FourCCFound.find(DFCC_FeatureInfo) == FourCCFound.end()) {
-    // Could be optional, but RS1 runtime doesn't handle this case properly.
-    ValCtx.EmitFormatError(ValidationRule::ContainerPartMissing, {"Feature Info"});
-  }
-
-  // Validate Root Signature
-  if (pPSVPart) {
-    if (pRootSignaturePart) {
-      try {
-        RootSignatureHandle RS;
-        RS.LoadSerialized((const uint8_t*)GetDxilPartData(pRootSignaturePart), pRootSignaturePart->PartSize);
-        RS.Deserialize();
-        IFTBOOL(VerifyRootSignatureWithShaderPSV(RS.GetDesc(),
-                                                  pDxilModule->GetShaderModel()->GetKind(),
-                                                  GetDxilPartData(pPSVPart), pPSVPart->PartSize,
-                                                  DiagStream), DXC_E_INCORRECT_ROOT_SIGNATURE);
-      } catch (...) {
-        ValCtx.EmitError(ValidationRule::ContainerRootSignatureIncompatible);
-      }
+  if (ValCtx.isLibProfile) {
+    if (FourCCFound.find(DFCC_RuntimeData) == FourCCFound.end()) {
+      ValCtx.EmitFormatError(ValidationRule::ContainerPartMissing, { "Runtime Data (RDAT)" });
     }
   } else {
-    // Not for lib.
-    if (!ValCtx.isLibProfile)
+    if (FourCCFound.find(DFCC_InputSignature) == FourCCFound.end()) {
+      VerifySignatureMatches(ValCtx, DXIL::SignatureKind::Input, nullptr, 0);
+    }
+    if (FourCCFound.find(DFCC_OutputSignature) == FourCCFound.end()) {
+      VerifySignatureMatches(ValCtx, DXIL::SignatureKind::Output, nullptr, 0);
+    }
+    if (bTess && FourCCFound.find(DFCC_PatchConstantSignature) == FourCCFound.end() &&
+        pDxilModule->GetPatchConstantSignature().GetElements().size())
+    {
+      ValCtx.EmitFormatError(ValidationRule::ContainerPartMissing, { "Program Patch Constant Signature" });
+    }
+    if (FourCCFound.find(DFCC_FeatureInfo) == FourCCFound.end()) {
+      // Could be optional, but RS1 runtime doesn't handle this case properly.
+      ValCtx.EmitFormatError(ValidationRule::ContainerPartMissing, { "Feature Info" });
+    }
+
+    // Validate Root Signature
+    if (pPSVPart) {
+      if (pRootSignaturePart) {
+        try {
+          RootSignatureHandle RS;
+          RS.LoadSerialized((const uint8_t*)GetDxilPartData(pRootSignaturePart), pRootSignaturePart->PartSize);
+          RS.Deserialize();
+          IFTBOOL(VerifyRootSignatureWithShaderPSV(RS.GetDesc(),
+                                                   pDxilModule->GetShaderModel()->GetKind(),
+                                                   GetDxilPartData(pPSVPart), pPSVPart->PartSize,
+                                                   DiagStream),
+                  DXC_E_INCORRECT_ROOT_SIGNATURE);
+        } catch (...) {
+          ValCtx.EmitError(ValidationRule::ContainerRootSignatureIncompatible);
+        }
+      }
+    } else {
       ValCtx.EmitFormatError(ValidationRule::ContainerPartMissing, {"Pipeline State Validation"});
+    }
   }
 
   if (ValCtx.Failed) {

tools/clang/lib/CodeGen/CGHLSLMS.cpp

@@ -2216,6 +2216,15 @@ void CGMSHLSLRuntime::addResource(Decl *D) {
 void CGMSHLSLRuntime::addSubobject(Decl *D) {
   VarDecl *VD = dyn_cast<VarDecl>(D);
   DXASSERT(VD != nullptr, "must be a global variable");
+
+  if (CGM.getCodeGenOpts().HLSLValidatorMajorVer == 1 &&
+      CGM.getCodeGenOpts().HLSLValidatorMinorVer < 4) {
+    // subobjects unsupported with this validator
+    DiagnosticsEngine &Diags = CGM.getDiags();
+    unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error, "subobjects are not supported by current validator version");
+    Diags.Report(D->getLocStart(), DiagID);
+    return;
+  }
  
   DXIL::SubobjectKind subobjKind;
   DXIL::HitGroupType hgType;
@@ -2233,7 +2242,14 @@ void CGMSHLSLRuntime::addSubobject(Decl *D) {
   }
 
   if (InitListExpr *initListExpr = dyn_cast<InitListExpr>(initExpr)) {
-    CreateSubobject(subobjKind, VD->getName(), initListExpr->getInits(), initListExpr->getNumInits(), hgType);
+    try {
+      CreateSubobject(subobjKind, VD->getName(), initListExpr->getInits(), initListExpr->getNumInits(), hgType);
+    } catch (hlsl::Exception&) {
+      DiagnosticsEngine &Diags = CGM.getDiags();
+      unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error, "internal error creating subobject");
+      Diags.Report(initExpr->getLocStart(), DiagID);
+      return;
+    }
   }
   else {
     DiagnosticsEngine &Diags = CGM.getDiags();
@@ -2500,7 +2516,7 @@ void CGMSHLSLRuntime::CreateSubobject(DXIL::SubobjectKind kind, const StringRef
       DXASSERT_NOMSG(argCount == 2);
       StringRef subObjName, exports;
       if (!GetAsConstantString(args[0], &subObjName, true) ||
-          !GetAsConstantString(args[1], &exports, true))
+          !GetAsConstantString(args[1], &exports, false))
         return;
 
       std::vector<StringRef> exportList = ParseSubobjectExportsAssociations(exports);

tools/clang/test/CodeGenHLSL/quick-test/subobjects.hlsl

@@ -1,10 +1,11 @@
-// RUN: %dxc -T lib_6_4 -Vd %s | FileCheck %s
+// RUN: %dxc -T lib_6_3 %s | FileCheck %s
 
 // CHECK: ; GlobalRootSignature grs = { <48 bytes> };
 // CHECK: ; StateObjectConfig soc = { STATE_OBJECT_FLAG_ALLOW_LOCAL_DEPENDENCIES_ON_EXTERNAL_DEFINITIONS };
 // CHECK: ; LocalRootSignature lrs = { <48 bytes> };
 // CHECK: ; SubobjectToExportsAssociation sea = { "grs", { "a", "b", "foo", "c" }  };
 // CHECK: ; SubobjectToExportsAssociation sea2 = { "grs", { }  };
+// CHECK: ; SubobjectToExportsAssociation sea3 = { "grs", { }  };
 // CHECK: ; RaytracingShaderConfig rsc = { MaxPayloadSizeInBytes = 128, MaxAttributeSizeInBytes = 64 };
 // CHECK: ; RaytracingPipelineConfig rpc = { MaxTraceRecursionDepth = 512 };
 // CHECK: ; HitGroup trHitGt = { HitGroupType = Triangle, Anyhit = "a", Closesthit = "b", Intersection = "" };
@@ -14,7 +15,9 @@ GlobalRootSignature grs = {"CBV(b0)"};
 StateObjectConfig soc = { STATE_OBJECT_FLAGS_ALLOW_LOCAL_DEPENDENCIES_ON_EXTERNAL_DEFINITONS };
 LocalRootSignature lrs = {"UAV(u0, visibility = SHADER_VISIBILITY_GEOMETRY), RootFlags(LOCAL_ROOT_SIGNATURE)"};
 SubobjectToExportsAssociation sea = { "grs", "a;b;foo;c" };
+// Empty association is well-defined: it creates a default association
 SubobjectToExportsAssociation sea2 = { "grs", ";" };
+SubobjectToExportsAssociation sea3 = { "grs", "" };
 RaytracingShaderConfig rsc = { 128, 64 };
 RaytracingPipelineConfig rpc = { 512 };
 TriangleHitGroup trHitGt = { "a", "b" };

tools/clang/tools/dxcompiler/dxcdisassembler.cpp

@@ -1578,7 +1578,9 @@ HRESULT Disassemble(IDxcBlob *pProgram, raw_string_ostream &Stream) {
       if (RDAT::SubobjectTableReader *pSubobjectTableReader =
         runtimeData.GetSubobjectTableReader()) {
         dxilModule.ResetSubobjects(new DxilSubobjects());
-        LoadSubobjectsFromRDAT(*dxilModule.GetSubobjects(), pSubobjectTableReader);
+        if (!LoadSubobjectsFromRDAT(*dxilModule.GetSubobjects(), pSubobjectTableReader)) {
+          Stream << "; error occurred while loading Subobjects from RDAT.\n";
+        }
       }
     }
     if (dxilModule.GetSubobjects()) {

tools/clang/unittests/HLSL/CompilerTest.cpp

@@ -5983,7 +5983,6 @@ TEST_F(CompilerTest, SubobjectCodeGenErrors) {
     { "GlobalRootSignature grs2 = {\"\"};", "1:29: error: empty string not expected here" },
     { "LocalRootSignature lrs2 = {\"\"};",  "1:28: error: empty string not expected here" },
     { "SubobjectToExportsAssociation sea2 = { \"\", \"x\" };", "1:40: error: empty string not expected here" },
-    { "SubobjectToExportsAssociation sea3 = { \"x\", \"\" };", "1:45: error: empty string not expected here" },
     { "string s; SubobjectToExportsAssociation sea4 = { \"x\", s };", "1:55: error: cannot convert to constant string" },
     { "extern int v; RaytracingPipelineConfig rpc2 = { v + 16 };", "1:49: error: cannot convert to constant unsigned int" },
     { "string s; TriangleHitGroup trHitGt2_8 = { s, \"foo\" };", "1:43: error: cannot convert to constant string" },