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

align_offset infinite loop #103361

Closed
lukas-code opened this issue Oct 21, 2022 · 5 comments · Fixed by #103378
Closed

align_offset infinite loop #103361

lukas-code opened this issue Oct 21, 2022 · 5 comments · Fixed by #103378
Labels
C-bug Category: This is a bug. I-hang Issue: The compiler never terminates, due to infinite loops, deadlock, livelock, etc.

Comments

@lukas-code
Copy link
Member

I tried this code: (Playground)

fn main() {
    struct HugeSize([u8; (1 << 47) - 1]);
    let _ = ((1usize << 47) as *const HugeSize).align_offset(1 << 47);
}

I expected to see this happen: The program terminates.

Instead, this happened: The program loops indefinitely.

I encountered this while working on #102795

Meta

rustc --version --verbose:

rustc 1.66.0-nightly (dcb376115 2022-10-20)
binary: rustc
commit-hash: dcb376115066d111dbf5f13d5ac2a2dbe8c12add
commit-date: 2022-10-20
host: x86_64-unknown-linux-gnu
release: 1.66.0-nightly
LLVM version: 15.0.2
@lukas-code lukas-code added the C-bug Category: This is a bug. label Oct 21, 2022
@lukas-code
Copy link
Member Author

lukas-code commented Oct 21, 2022

Hangs here:

loop {
// y = y * (2 - xy) mod n
//
// Note, that we use wrapping operations here intentionally – the original formula
// uses e.g., subtraction `mod n`. It is entirely fine to do them `mod
// usize::MAX` instead, because we take the result `mod n` at the end
// anyway.
inverse = wrapping_mul(inverse, wrapping_sub(2usize, wrapping_mul(x, inverse)));
if going_mod >= m {
return inverse & m_minus_one;
}
going_mod = wrapping_mul(going_mod, going_mod);
}

@rustbot label I-hang

@rustbot rustbot added the I-hang Issue: The compiler never terminates, due to infinite loops, deadlock, livelock, etc. label Oct 21, 2022
@thomcc
Copy link
Member

thomcc commented Oct 21, 2022

CC @nagisa

@nagisa
Copy link
Member

nagisa commented Oct 21, 2022

Thanks for the report.

Priority wise, this seems like it isn’t going to be super high priority to resolve, given that 1<<47 is 128TiB. 128TiB, unfortunately, is half of the maximum supported address space on many modern x86_64 and aarch64 deployments, so it is conceivable that this could be actually hit. Still would be nice to actually fix it.

As for the strategies to fix this, there are a couple of approaches to consider:

  1. We could store the going_mod into a u128 (2 * usize on any supported platform today.)
  • This is not super future-proof, and unfortunately probably somewhat more costly in the non-const-evaluated case.
  1. We could use saturating_mul instead of wrapping_mul. When going_mod overflows on the last iteration, it will become the maximum value that fits in usize and the termination condition will be hit.
  • Not super clear what codegen quality implications are for using saturating_mul;
  1. Compute a widening/checked multiplication and additionally compare the high bits for != 0 in the terminating condition.
  • This is an alteration of 1. I suspect this is going to produce the best code, but that remains to be seen.

@jruderman
Copy link
Contributor

Would it work to change the termination condition from going_mod >= m to going_mod >= m || going_mod == 0? Where going_mod == 0 means it wrapped, so the logical value is out of the type's range, and thus bigger than m.

@jruderman
Copy link
Contributor

Or weirder: store log_2_going_mod instead of going_mod, so the comparison can be done using ctlz(m) and the squaring operation can be replaced with a doubling (bit shift).

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Nov 16, 2022
…tmcm

Fix mod_inv termination for the last iteration

On usize=u64 platforms, the 4th iteration would overflow the `mod_gate` back to 0. Similarly for usize=u32 platforms, the 3rd iteration would overflow much the same way.

I tested various approaches to resolving this, including approaches with `saturating_mul` and `widening_mul` to a double usize. Turns out LLVM likes `mul_with_overflow` the best. In fact now, that LLVM can see the iteration count is limited, it will happily unroll the loop into a nice linear sequence.

You will also notice that the code around the loop got simplified somewhat. Now that LLVM is handling the loop nicely, there isn’t any more reasons to manually unroll the first iteration out of the loop (though looking at the code today I’m not sure all that complexity was necessary in the first place).

Fixes rust-lang#103361
Manishearth added a commit to Manishearth/rust that referenced this issue Nov 18, 2022
…tmcm

Fix mod_inv termination for the last iteration

On usize=u64 platforms, the 4th iteration would overflow the `mod_gate` back to 0. Similarly for usize=u32 platforms, the 3rd iteration would overflow much the same way.

I tested various approaches to resolving this, including approaches with `saturating_mul` and `widening_mul` to a double usize. Turns out LLVM likes `mul_with_overflow` the best. In fact now, that LLVM can see the iteration count is limited, it will happily unroll the loop into a nice linear sequence.

You will also notice that the code around the loop got simplified somewhat. Now that LLVM is handling the loop nicely, there isn’t any more reasons to manually unroll the first iteration out of the loop (though looking at the code today I’m not sure all that complexity was necessary in the first place).

Fixes rust-lang#103361
@bors bors closed this as completed in a3c3f72 Nov 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. I-hang Issue: The compiler never terminates, due to infinite loops, deadlock, livelock, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants