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

[naga msl-out] Avoid UB by making all loops bounded. #6545

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

jimblandy
Copy link
Member

In MSL output, avoid undefined behavior due to unbounded loops by adding an unpredictable, never-actually-taken break to the bottom of each loop body, rather than adding an unpredictable, never-actually-taken branch over each loop.

This will probably have more of a performance impact, because it affects each iteration of the loop, but unlike branching over the loop, which leaves infinite loops (and thus undefined behavior) in the output, this actually ensures that no loop presented to Metal is unbounded, so that there is no undefined behavior present that the optimizer could use to make unwelcome inferences.

Fixes #6528.

In MSL output, avoid undefined behavior due to unbounded loops by
adding an unpredictable, never-actually-taken `break` to the bottom of
each loop body, rather than adding an unpredictable,
never-actually-taken branch over each loop.

This will probably have more of a performance impact, because it
affects each iteration of the loop, but unlike branching over the
loop, which leaves infinite loops (and thus undefined behavior) in the
output, this actually ensures that no loop presented to Metal is
unbounded, so that there is no undefined behavior present that the
optimizer could use to make unwelcome inferences.

Fixes gfx-rs#6528.
@jimblandy jimblandy requested a review from a team as a code owner November 14, 2024 19:36
@jimblandy jimblandy added type: bug Something isn't working area: naga back-end Outputs of naga shader conversion naga Shader Translator lang: Metal Metal Shading Language labels Nov 14, 2024
@JMS55
Copy link
Contributor

JMS55 commented Nov 14, 2024

Is this something we can turn off via wgpu/naga for native platforms?

@cwfitzgerald
Copy link
Member

Yes, see #6520

Copy link
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

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

LGTM

naga/src/back/msl/writer.rs Show resolved Hide resolved
naga/src/back/msl/writer.rs Show resolved Hide resolved
@jimblandy jimblandy merged commit 0b82776 into gfx-rs:trunk Nov 18, 2024
27 checks passed
@jimblandy jimblandy deleted the naga-msl-out-break-from-loop branch November 18, 2024 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: naga back-end Outputs of naga shader conversion lang: Metal Metal Shading Language naga Shader Translator type: bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[naga msl-out] Unbounded loop workaround is not adequate
6 participants