Pewarta Proxy

📋 Server Request & Proxy Logs

Loading active proxy settings...
Developed by Khairul Adha (adhexa.id)
Mengambil log server...

⚙️ Pengaturan Proxy Target

Skip to content

Fix null pointer dereferences on malformed bitcode and DXIL - #9026

Open
mjkrol wants to merge 1 commit into
microsoft:mainfrom
mjkrol:fix/null-derefs-malformed-bitcode
Open

mjkrol wants to merge 1 commit into
microsoft:mainfrom
mjkrol:fix/null-derefs-malformed-bitcode

Conversation

@mjkrol

@mjkrol mjkrol commented Oct 12, 2026

Copy link
Copy Markdown

Malformed bitcode or DXIL can currently crash the bitcode reader, or terminate it with an uncaught exception, instead of returning a diagnostic. This adds the missing checks so that such input is rejected with an error.

  • BitcodeReader: check for null types and values returned by getTypeByID and getValueFwdRef in metadata, constants and switch parsing. Validate function return types and constant cast operands before building them. Replace an assert on function-local metadata with an error.
  • report_fatal_error_handler: tolerate a missing diagnostic handler.
  • DxilResourceBase::GetHLSLType: fail on a missing symbol or type.
  • EmitWarningOrErrorOnFunction and ValidationContext::EmitFnFormatError: tolerate a null function.

Testing: added DXC/invalid-bitcode.test with three malformed bitcode inputs, found by flipping bits in a small valid module. Without this change each one ends in an uncaught exception with no diagnostic. With it, each reports the expected error. The LitDXILValidation, DXC, CodeGenDXIL and HLSLFileCheckLit lit suites pass on Windows (MSVC, x64). The one exception is batch_compile.test, which I couldn't run locally. I have not run the TAEF unit tests.

Assisted-by: Claude Code (test inputs and test file)

Copilot AI balanced review requested due to automatic review settings October 12, 2026 15:29
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@mjkrol

mjkrol commented Oct 12, 2026

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="Broadcom"

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.

🟡 Changes recommended

Several malformed metadata, cast, function-type, and switch inputs can still reach null dereferences or assertions.

6 open findings
What changed in this PR

Hardens malformed bitcode and DXIL handling to return diagnostics instead of crashing.

Changes:

  • Adds defensive type, value, cast, and diagnostic checks.
  • Makes DXIL validation paths tolerate missing functions or symbols.
  • Adds three malformed-bitcode regression fixtures.
File Description
lib/​Bitcode/​Reader/​BitcodeReader.cpp Adds malformed-input validation.
lib/​DXIL/​DxilResourceBase.cpp Validates resource symbols and types.
lib/​DXIL/​DxilUtilDbgInfoAndMisc.cpp Supports diagnostics without a function.
lib/​DxilValidation/​DxilValidationUtils.cpp Guards debug-function lookup.
tools/​clang/​test/​DXC/​invalid-bitcode.test Verifies malformed-input diagnostics.
tools/​clang/​test/​DXC/​Inputs/​invalid-bitcode-function-return-type.bc Tests invalid function return types.
tools/​clang/​test/​DXC/​Inputs/​invalid-bitcode-constexpr-cast.bc Tests invalid constant casts.
tools/​clang/​test/​DXC/​Inputs/​invalid-bitcode-null-type.bc Tests null type handling.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/Bitcode/Reader/BitcodeReader.cpp
Comment thread lib/Bitcode/Reader/BitcodeReader.cpp
Comment thread lib/Bitcode/Reader/BitcodeReader.cpp Outdated
Comment thread lib/Bitcode/Reader/BitcodeReader.cpp Outdated
Comment thread lib/Bitcode/Reader/BitcodeReader.cpp Outdated
Comment thread tools/clang/test/DXC/invalid-bitcode.test Outdated
Copilot AI balanced review requested due to automatic review settings October 12, 2026 18:38
@mjkrol
mjkrol force-pushed the fix/null-derefs-malformed-bitcode branch from d0a71dd to 8d14fff Compare October 12, 2026 18:38
@damyanp

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

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.

Comment thread lib/Bitcode/Reader/BitcodeReader.cpp
Comment thread lib/Bitcode/Reader/BitcodeReader.cpp
Comment thread tools/clang/test/Bitcode/invalid-bitcode.test Outdated
Copilot AI balanced review requested due to automatic review settings October 13, 2026 05:52
@mjkrol
mjkrol force-pushed the fix/null-derefs-malformed-bitcode branch from 8d14fff to c0fee21 Compare October 13, 2026 05:52

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.

🟡 Changes recommended

Short switch records can still read out of bounds, and fatal errors without a handler can still escape without the underlying diagnostic.

1 open finding
3 resolved since last review

🧠 Review effort: Balanced

Comment thread lib/Bitcode/Reader/BitcodeReader.cpp
Copilot AI balanced review requested due to automatic review settings October 13, 2026 06:12
@mjkrol
mjkrol force-pushed the fix/null-derefs-malformed-bitcode branch from c0fee21 to 5d173db Compare October 13, 2026 06:12

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.

🟡 Changes recommended

Unbounded forward-reference IDs can still exhaust memory, and range switches can retain null destinations.

2 open findings
1 resolved since last review

🧠 Review effort: Balanced

Comment thread lib/Bitcode/Reader/BitcodeReader.cpp
Comment thread lib/Bitcode/Reader/BitcodeReader.cpp
@mjkrol
mjkrol force-pushed the fix/null-derefs-malformed-bitcode branch from 5d173db to 86fe000 Compare October 13, 2026 07:24
Copilot AI balanced review requested due to automatic review settings October 13, 2026 07:24

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.

🟡 Changes recommended

Some malformed-input paths can still escape as exceptions or crash before reaching the new guards.

2 open findings
2 resolved since last review

🧠 Review effort: Balanced

Comment thread lib/Bitcode/Reader/BitcodeReader.cpp
Comment thread lib/DXIL/DxilResourceBase.cpp
Add error checks so that malformed bitcode or DXIL is rejected with a
diagnostic instead of dereferencing a null pointer:

 - BitcodeReader: check for null types and values returned by
   getTypeByID/getValueFwdRef in metadata (including the selective
   named-metadata parser used by IDxcPdbUtils and METADATA_OLD_FN_NODE,
   which also must reference a function-local value), constants and
   switch parsing, validate function return and argument types (for both
   FUNCTION and FUNCTION_OLD type records), switch condition types and
   record sizes (for both switch record formats), constant cast operands
   and cast instructions, and replace an assert on function-local
   metadata with an error.
 - AutoUpgrade: when upgrading a bitcast between pointers in different
   address spaces, go through an integer vector for pointer vectors, and
   do not upgrade casts whose source and destination shapes differ.
 - report_fatal_error_handler: tolerate a missing diagnostic handler.
 - DxilResourceBase::GetHLSLType: fail on a missing symbol or type, and
   DxilMDHelper::LoadDxilResourceBase: reject a non-constant resource
   symbol before it is used.
 - EmitWarningOrErrorOnFunction and ValidationContext::EmitFnFormatError:
   tolerate a null function.

Add Bitcode/invalid-bitcode.test with eighteen malformed bitcode
inputs, and Bitcode/upgrade-addrspace-bitcast.test and
Bitcode/switch-case-range.test with well-formed inputs that must still be
read correctly. Three inputs were found by bit-flipping a small valid
module; the rest were generated directly to reach records the writer no
longer emits, the selective metadata parser, switch and cast
instructions, and address space bitcasts. Before this change, all of the
malformed inputs except an old-format switch on a float condition, which
is silently accepted as invalid IR, crash the reader or end in an
uncaught exception with no diagnostic, as do the address space bitcasts
that should be upgraded. The tests use llvm-dis, which exits cleanly on
a read error, so add it to the check-clang dependencies.

Assisted-by: Claude Code
Copilot AI balanced review requested due to automatic review settings October 13, 2026 08:46
@mjkrol
mjkrol force-pushed the fix/null-derefs-malformed-bitcode branch from 86fe000 to c4176e2 Compare October 13, 2026 08:46

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.

🟡 Changes recommended

A crafted case range can still hang the reader or exhaust memory during expansion.

1 open finding
2 resolved since last review

🧠 Review effort: Balanced

Comment thread lib/Bitcode/Reader/BitcodeReader.cpp
@damyanp

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@llvm-beanz Chris B (llvm-beanz) left a comment

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.

I have two concerns about this PR that I want to raise:

  1. This PR seems to violate our AI tool policy. I believe this to be the case because the description indicates that the author has not configured a development environment correctly to actually test and verify the change.
  2. The inclusion of a bunch of binary files as test collateral is a security issue that gives me strong smells of XZ Utils. We need test cases that are not completely opaque to reviewers.

@github-project-automation github-project-automation Bot moved this from New to In progress in HLSL Roadmap Oct 16, 2026
@mjkrol

mjkrol commented Oct 19, 2026

Copy link
Copy Markdown
Author

I have two concerns about this PR that I want to raise:

  1. This PR seems to violate our AI tool policy. I believe this to be the case because the description indicates that the author has not configured a development environment correctly to actually test and verify the change.

Yes, my local setup didn't have TAEF, so I only ran the lit suites, with SPIR-V disabled.

  1. The inclusion of a bunch of binary files as test collateral is a security issue that gives me strong smells of XZ Utils. We need test cases that are not completely opaque to reviewers.

Would a small bitcode generator, written by AI, work for you? My original work is based on fuzzing binary shaders to identify a single crash and then finiding & fixing similar issues in the neighbourhood.

Also, the Copilot review bot raised a lot of issues that were not about my changes but about existing surrounding code that had other vulns. This PR is the first in a series, so is it OK to address only those comments that point out problems in my changes?

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: In progress

Development

Successfully merging this pull request may close these issues.

4 participants