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

Problem with short circuiting #1927

Closed
athas opened this issue Apr 30, 2023 · 1 comment · Fixed by #1928
Closed

Problem with short circuiting #1927

athas opened this issue Apr 30, 2023 · 1 comment · Fixed by #1928
Assignees

Comments

@athas
Copy link
Member

athas commented Apr 30, 2023

Consider this program:

def main k =
  let src = iota k
  let dst = iota 5
  in dst with [1:4] = src

Short-circuiting will put the array src in the same memory as dst, but before constructing dst. This means that the construction of dst will overwrite src.

Worse, if k is too large, we will write out-of-bounds, as short-circuiting does not ensure that the bounds check is done before src is constructed.

This is a pretty bad bug that can result in anything from invalid results to memory corruption.

@athas
Copy link
Member Author

athas commented Apr 30, 2023

Regarding the overwriting, it appears we do not correctly check safety condition 3 for short-circuiting statements of form let y[i] = b^{lu}:

--      3. there is no use of the left-hand side memory block @m_y@
--           during the liveness of @b@, i.e., in between its last use
--           and its creation.

In this case the memory block m_dst is certainly used (to construct dst) during the lifetime of src.

(Incidentally, it's somewhat confusing that the comments use the let y[i] = ...-shorthand, as that's not what the IR actually does.)

@Munksgaard Munksgaard self-assigned this Apr 30, 2023
Munksgaard added a commit that referenced this issue May 1, 2023
Previously, some BasicOps, like Iota didn't have their write sets correctly
computed, which caused our short-circuiting pass to incorrectly short-circuit
some programs involving iota.

Fixes #1927
Munksgaard added a commit that referenced this issue May 1, 2023
Previously Iota didn't have their write sets correctly computed, which caused
our short-circuiting pass to incorrectly short-circuit some programs.

Fixes #1927
Munksgaard added a commit that referenced this issue May 2, 2023
Previously Iota didn't have their write sets correctly computed, which caused
our short-circuiting pass to incorrectly short-circuit some programs.

Fixes #1927
athas pushed a commit that referenced this issue May 2, 2023
Previously Iota didn't have their write sets correctly computed, which caused
our short-circuiting pass to incorrectly short-circuit some programs.

Fixes #1927
razetime pushed a commit to razetime/futhark that referenced this issue May 27, 2023
Previously Iota didn't have their write sets correctly computed, which caused
our short-circuiting pass to incorrectly short-circuit some programs.

Fixes diku-dk#1927
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants