Skip to content

Fix assert due to unreachable discard#7289

Merged
amaiorano merged 2 commits intomicrosoft:mainfrom
amaiorano:fix-assert-unreachable-discard
Apr 2, 2025
Merged

Fix assert due to unreachable discard#7289
amaiorano merged 2 commits intomicrosoft:mainfrom
amaiorano:fix-assert-unreachable-discard

Conversation

@amaiorano
Copy link
Copy Markdown
Collaborator

When emitting discard in an unreachable code context (e.g. after an infinite loop), DXC would assert (if asserts enabled), or trigger a UBSAN failure because the discard instruction would have no parent. When an infinite loop is emitted during CodeGen, the InsertPt is cleared, thus subsequent discard instructions would be created, but no parent set. We skip emitting discard in this case, which follows the same pattern as is done for EmitIfStmt, and EmitSwitchStmt.

When emitting discard in an unreachable code context (e.g. after an
infinite loop), DXC would assert (if asserts enabled), or trigger a
UBSAN failure because the discard instruction would have no parent. When
an infinite loop is emitted during CodeGen, the InsertPt is cleared,
thus subsequent discard instructions would be created, but no parent
set. We skip emitting discard in this case, which follows the same
pattern as is done for EmitIfStmt, and EmitSwitchStmt.
@amaiorano
Copy link
Copy Markdown
Collaborator Author

An example of the problem can be seen on compiler explorer: https://godbolt.org/z/c4fq7eaTz

Because CE uses a release build without asserts and without ubsan, the extra discard is needed before the while loop to showcase the failure. I didn't dig into why exactly, but without it, DXC behaves "correctly". But with asserts or UBSAN enabled, the fact that the second discard has a nullptr parent results in a crash.

@amaiorano amaiorano requested a review from llvm-beanz April 1, 2025 16:04
llvm-beanz
llvm-beanz previously approved these changes Apr 1, 2025
@amaiorano amaiorano enabled auto-merge (squash) April 1, 2025 22:14
@amaiorano amaiorano merged commit 2f357a9 into microsoft:main Apr 2, 2025
12 checks passed
@github-project-automation github-project-automation Bot moved this from New to Done in HLSL Roadmap Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants