Support allocation failures when interpreting MIR

[syvb]

This closes #79601 by handling the case where memory allocation fails during MIR interpretation, and translates that failure into an InterpError. The error message is "tried to allocate more memory than available to compiler" to make it clear that the memory shortage is happening at compile-time by the compiler itself, and that it is not a runtime issue.

Now that memory allocation can fail, it would be neat if Miri could simulate low-memory devices to make it easy to see how much memory a Rust program needs.

Note that this breaks Miri because it assumes that allocation can never fail.

[rust-highfive]

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

[rust-highfive]

r? @varkor

(rust-highfive has picked a reviewer for you, use r? to override)

[RalfJung]

The original plan we had for having a memory limit in Miri was to make this a configurable limit. The other two limits (stack size, execution steps) are completely controlled by the interpreter. The idea is that interpretation should be completely deterministic and repeatable.

This new error, however, depends on arbitrary factors from the environment: the same evaluation can sometimes succeed, and sometimes fail with MemoryExhausted. There is no precedent for that, and I am honestly not sure if this is even okay to do. The compiler generally assumes that if evaluation finishes, then it is deterministic and could not have finished in any other way.

[oli-obk]

We kind of do have precedent. Consider include_bytes!. It can fail if the file isn't there, and during the next compilation the file can suddenly be there and compilation can succeed. This could be similar enough?

[varkor]

r? @oli-obk

[RalfJung]

Consider include_bytes!. It can fail if the file isn't there, and during the next compilation the file can suddenly be there and compilation can succeed. This could be similar enough?

Maybe? I don't know the invariants of the query system well enough. But this can lead to the same CTFE query having different results in different crates.

[oli-obk]

But this can lead to the same CTFE query having different results in different crates.

oh right. There's no precedent for that. Maybe this should be a fatal error reported directly at the problematic site instead of it being a regular CTFE error? So the query is never left, but instead the compilation is aborted.

not sure how to proceed here. cc @rust-lang/compiler It is very easy to make CTFE run out of RAM, thus crashing rustc. Specifically for CTFE we could catch these out of memory situations and report a proper error, but we're unsure of the implications.

[wesleywiser]

But this can lead to the same CTFE query having different results in different crates.

Yeah, that does seem like it could lead to all sorts of issues. From a very quick glance at the PR, it looks like allocation failures in CTFE still result in compilation failing, just with a nicer error message. If compilation still fails, then this seems ok. Are there cases where CTFE will fail but compilation will still succeed?

[RalfJung]

Are there cases where CTFE will fail but compilation will still succeed?

Not all CTFE failures are hard errors (yet). So with allow(const_err) you can still make compilation succeed.

We could probably do something similar to #86194, and make all resource exhaustion failures always hard errors.

[oli-obk]

I'm wondering if, even with a hard error, the query system + incremental compilation can get "stuck". We aren't tracking resource exhaustion as an input to the query system, so the query will stay green and not get reevaluated, but loaded from the cache. You may have to remove your cache or modify the right piece of code to get the system "unstuck".

[syvb]

I've made allocation failures a hard error, and it appears that recompiling with more memory available does work, and that the error doesn't get cached, even when forcing incremental compilation with RUSTC_FORCE_INCREMENTAL=1.

Note that is a lot easier to test this with this diff to make it possible to simulate zero-memory conditions:

diff --git a/compiler/rustc_middle/src/mir/interpret/allocation.rs b/compiler/rustc_middle/src/mir/interpret/allocation.rs
index 7405a70d39a..71d09e16ec4 100644
--- a/compiler/rustc_middle/src/mir/interpret/allocation.rs
+++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs
@@ -125,6 +125,9 @@ pub fn from_bytes_byte_aligned_immutable<'a>(slice: impl Into<Cow<'a, [u8]>>) ->
     /// Try to create an Allocation of `size` bytes, failing if there is not enough memory
     /// available to the compiler to do so.
     pub fn uninit(size: Size, align: Align) -> InterpResult<'static, Self> {
+        if std::env::var(std::ffi::OsStr::new("RUSTC_ALWAYS_FAIL_MIRI_ALLOC")).is_ok() {
+            Err(InterpError::ResourceExhaustion(ResourceExhaustionInfo::MemoryExhausted))?;
+        }
         let mut bytes = Vec::new();
         bytes.try_reserve(size.bytes_usize()).map_err(|_| {

Also note that hard errors currently give a confusing and wrong error message, since there is no differentiation between hard and soft errors in the message generation (EDIT: I opened a PR to fix this: #86340):

error[E0080]: any use of this value will cause an error
 --> src/main.rs:1:2
  |
1 | /  pub const X: () = {
2 | |     let y = [9; 12];
3 | | };
  | |__^ tried to allocate more memory than available to compiler

[wesleywiser]

That's what I would expect. We only write to the incremental cache when the compilation succeeds so as long compilation always fails eventually when there is an allocation failure, there shouldn't be an issue with the incremental system.

[bors]

☔ The latest upstream changes (presumably #86291) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #86399) made this pull request unmergeable. Please resolve the merge conflicts.

[oli-obk]

@bors r+

[bors]

📌 Commit 72c38905ce940f3543d9067c5398b3670f1ba01f has been approved by oli-obk

[syvb]

@RalfJung

Delaying a ICE is a great idea, I've gone ahead and implemented it. ty::tls::with should be okay here since const-eval is querified and calling a queries puts the TyCtxt into TLS.

The fix for that ICE is not to ignore the error but to have some limits in ConstProp that avoids even making these big allocations

This is already done:

rust/compiler/rustc_mir/src/transform/const_prop.rs

Lines 40 to 43 in 868c702

	/// The maximum number of bytes that we'll allocate space for a local or the return value.
	/// Needed for #66397, because otherwise we eval into large places and that can cause OOM or just
	/// Severely regress performance.
	const MAX_ALLOC_LIMIT: u64 = 1024;

[RalfJung]

This is already done:

Great! Then making allocation panic immediately (ruling out MemoryExhausted) should be fine, right?

[RalfJung]

Thanks for sticking with me through all these rounds of review. :)
@bors r=RalfJung,oli-obk

[bors]

📌 Commit d83c46f has been approved by RalfJung,oli-obk

[bors]

⌛ Testing commit d83c46f with merge 39e20f1...

[bors]

☀️ Test successful - checks-actions
Approved by: RalfJung,oli-obk
Pushing 39e20f1 to master...

[rust-highfive]

📣 Toolstate changed by #86255!

Tested on commit 39e20f1.
Direct link to PR: #86255

💔 miri on windows: test-pass → build-fail (cc @RalfJung @oli-obk @eddyb).
💔 miri on linux: test-pass → build-fail (cc @RalfJung @oli-obk @eddyb).