Skip to content

Fix output-position stream bounds in DXIL validation - #9090

Open
Damyan Pepper (damyanp) wants to merge 1 commit into
microsoft:mainfrom
damyanp:damyanp-output-stream-validation-fix
Open

Damyan Pepper (damyanp) wants to merge 1 commit into
microsoft:mainfrom
damyanp:damyanp-output-stream-validation-fix

Conversation

@damyanp

@damyanp Damyan Pepper (damyanp) commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Assisted by GitHub Copilot.

Reuse the bounded stream index for output-position tracking and check stream bounds before accessing the output-position mask. Add raw-bitcode regression coverage for stream limits and position completeness.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@damyanp
Damyan Pepper (damyanp) force-pushed the damyanp-output-stream-validation-fix branch from 3d3b395 to eaf6826 Compare November 3, 2026 17:26
@damyanp
Damyan Pepper (damyanp) marked this pull request as ready for review November 3, 2026 18:15
Copilot AI balanced review requested due to automatic review settings November 3, 2026 18:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are recorded for the DXIL validation fix addressing invalid output-stream metadata.

No open issues 🎉

What changed in this PR

This PR fixes out-of-bounds accesses in DXIL validation when output-position signatures contain invalid stream metadata.

Changes:

  • Bounds-checks stream indices before updating position masks and uses the validated stream index for position tracking.
  • Adds regression tests for valid and invalid geometry/vertex shader streams and incomplete position writes.
  • Documents the validator fix in the release notes.
File Description
tools/​clang/​unittests/​HLSL/​ValidationTest.cpp Adds raw-bitcode tests for stream bounds and position-write validation.
lib/​DxilValidation/​DxilValidation.cpp Prevents out-of-bounds access in output-position tracking.
docs/​ReleaseNotes.md Records the invalid output-stream metadata fix.

Review effort: Balanced

Comment on lines +161 to 164
if (IsOutput && SE.GetSemantic()->GetKind() == DXIL::SemanticKind::Position &&
SE.GetOutputStream() < DXIL::kNumOutputStreams) {
Status.OutputPositionMask[SE.GetOutputStream()] |= 1 << Col;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (IsOutput && SE.GetSemantic()->GetKind() == DXIL::SemanticKind::Position &&
SE.GetOutputStream() < DXIL::kNumOutputStreams) {
Status.OutputPositionMask[SE.GetOutputStream()] |= 1 << Col;
}
if (IsOutput && SE.GetSemantic()->GetKind() == DXIL::SemanticKind::Position &&
SE.GetOutputStream() < DXIL::kNumOutputStreams)
Status.OutputPositionMask[SE.GetOutputStream()] |= 1 << Col;

Comment on lines 6286 to 6288
if (IsOutput && SemanticKind == DXIL::SemanticKind::Position) {
Status.hasOutputPosition[E->GetOutputStream()] = true;
Status.hasOutputPosition[StreamId] = true;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this is semantically the same? StreamId gets cleared to 0 for several things (like !IsOutput) and it seem like maybe we could be slightly changing functionality here?

Also the extra {} should be removed

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

3 participants