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

Fix ASAN UAF in DxilConditionalMem2Reg #6910

Conversation

amaiorano
Copy link
Collaborator

ScalarizePreciseVectorAlloca would iterate over all instructions, then for each instruction use, would iterate and potentially erase the instruction. If the erased instruction was the immediate next instruction after the alloca, this would invalidate the outer instruction iterator. Fixed by collecting the allocas in a vector first.

ScalarizePreciseVectorAlloca would iterate over all instructions, then
for each instruction use, would iterate and potentially erase the
instruction. If the erased instruction was the immediate next
instruction after the alloca, this would invalidate the outer
instruction iterator. Fixed by collecting the allocas in a vector first.
@amaiorano amaiorano requested a review from a team as a code owner September 11, 2024 13:57
@amaiorano
Copy link
Collaborator Author

As noted in the test file:

// Unfortunately, we cannot create an IR test for this because dxil-cond-mem2reg
// executes between scalarrepl-param-hlsl and hlsl-dxil-precise, and the former
// temporarily marks empty functions as 'precise' while the latter pass uses this
// information, and then deletes these functions. But splitting the passes in between
// these two fails validation because empty functions cannot have attributes on them.
// So we use a full HLSL test for this.

@amaiorano amaiorano requested a review from dneto0 September 11, 2024 14:06
@amaiorano amaiorano merged commit f11914c into microsoft:main Sep 11, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants