Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DXIL.dll crashes on intrinsics that are not defined for a known shader model #6168

Closed
jenatali opened this issue Jan 19, 2024 · 0 comments · Fixed by #6302
Closed

DXIL.dll crashes on intrinsics that are not defined for a known shader model #6168

jenatali opened this issue Jan 19, 2024 · 0 comments · Fixed by #6302
Assignees
Labels
bug Bug, regression, crash crash DXC crashing or hitting an assert sm6.8 Shader Model 6.8 validation Related to validation or signing

Comments

@jenatali
Copy link
Member

Description
Given DXIL that declares itself as shader model 6.7 / DXIL 1.7, but uses intrinsic functions defined by shader model 6.8, attempting to validate that DXIL module will crash with an access violation.

Steps to Reproduce
See the relevant code in dxil::OP:

void OP::RefreshCache() {
  for (Function &F : m_pModule->functions()) {
    if (OP::IsDxilOpFunc(&F) && !F.user_empty()) {
      CallInst *CI = cast<CallInst>(*F.user_begin());
      OpCode OpCode = OP::GetDxilOpFuncCallInst(CI);
      Type *pOverloadType = OP::GetOverloadType(OpCode, &F);
      Function *OpFunc = GetOpFunc(OpCode, pOverloadType);
      (void)(OpFunc);
      DXASSERT_NOMSG(OpFunc == &F);
    }
  }
}

For every intrinsic function, GetOpFunc is called to instantiate it in the cache. Inside GetOpFunc:

Function *OP::GetOpFunc(OpCode opCode, Type *pOverloadType) {
  DXASSERT(0 <= (unsigned)opCode && opCode < OpCode::NumOpCodes,
           "otherwise caller passed OOB OpCode");
  assert(0 <= (unsigned)opCode && opCode < OpCode::NumOpCodes);
  DXASSERT(IsOverloadLegal(opCode, pOverloadType),
           "otherwise the caller requested illegal operation overload (eg HLSL "
           "function with unsupported types for mapped intrinsic function)");
  OpCodeClass opClass = m_OpCodeProps[(unsigned)opCode].opCodeClass;
  Function *&F =
      m_OpCodeClassCache[(unsigned)opClass].pOverloads[pOverloadType];

Those asserts for "otherwise caller..." should be validation, because the caller clearly didn't do any validation. As a result, indexing into m_OpCodeProps can use an out-of-bounds index, meaning that accessing opCodeClass could be an access violation directly, or at best could return garbage. I was seeing it return a pointer-type value, which was then used to index into m_OpCodeClassCache, which failed catastrophically.

My repro was done by accidentally inserting SampleCmpBias instructions into a SM6.7 shader in the Mesa DXIL backend. DXC will not emit such a shader, since it has validation against doing so earlier on in the compilation process.

@jenatali jenatali added bug Bug, regression, crash needs-triage Awaiting triage labels Jan 19, 2024
@pow2clk pow2clk added sm6.8 Shader Model 6.8 validation Related to validation or signing crash DXC crashing or hitting an assert and removed needs-triage Awaiting triage labels Jan 25, 2024
@github-project-automation github-project-automation bot moved this to 🆕 New in HLSL Roadmap Jan 25, 2024
@pow2clk pow2clk added this to the Shader Model 6.8 milestone Jan 25, 2024
@pow2clk pow2clk moved this to Done in HLSL Triage Jan 25, 2024
python3kgae added a commit that referenced this issue Feb 20, 2024
Replace assert on illegal DXIL op with return illegal value. Check the
illegal cases in validation.

Fixes #6168
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in HLSL Roadmap Feb 20, 2024
python3kgae added a commit that referenced this issue Feb 22, 2024
Replace assert on illegal DXIL op with return illegal value. Check the
illegal cases in validation.

Fixes #6168

(cherry picked from commit 5dee81c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug, regression, crash crash DXC crashing or hitting an assert sm6.8 Shader Model 6.8 validation Related to validation or signing
Projects
Archived in project
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants