interpret: do not ICE when a promoted fails with OOM

[RalfJung]

Fixes #130687

try-job: aarch64-apple
try-job: dist-x86_64-linux

[rustbot]

r? @fmease

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri, @rust-lang/wg-const-eval

[jieyouxu]

Thanks! You can r=me after deleting the crashes test (which looks like the exact issue this PR correctly addresses :3)

[RalfJung]

@bors r=jieyouxu

[bors]

📌 Commit 2af9273 has been approved by jieyouxu

It is now in the queue for this repository.

[bors]

⌛ Testing commit 2af9273 with merge 2b10cd1...

[bors]

💔 Test failed - checks-actions

[RalfJung]

Looks like we don't have graceful OOM handling on macOS

2024-11-18T12:44:07.5906800Z error: Error: expected failure status (Some(1)) but received status None.
2024-11-18T12:44:07.5907300Z status: signal: 9 (SIGKILL)

[RalfJung]

@bors r=jieyouxu rollup=iffy

[bors]

📌 Commit 12d7fd8 has been approved by jieyouxu

It is now in the queue for this repository.

[bors]

⌛ Testing commit 12d7fd8 with merge 7413c9a...

[RalfJung]

Hm, this time it's a Linux test...why does it fail on the dist runner but works locally and in PR CI?

[jieyouxu]

@RalfJung those look like post-PGO dist tests. AFAIK those are not enabled in PR CI, not without unsetting DIST_TRY_BUILD=1 cc https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/Which.20tests.20are.20exercised.20when.20job.20is.20ran.20as.20arbitrary.20try.3F

[jieyouxu]

It's not immediately obvious why that would fail in post opt-dist when it didn't fail in stage2 tests though... cc @Kobzol do you have any guesses?

[jnkr-ifx]

I see that the dist test has rust.jemalloc enabled in the RUST_CONFIGURE_ARGS while the PR CI does not; could that be the reason for the difference?

The test is failing with a SIGKILL instead of an expected compiler error; indicating the kernel overcommitted the huge allocation, and then OOM-killed the process when it actually tried to access the whole thing.

On my system, jemalloc calls mmap with MAP_NORESERVE which forces overcommit; while glibc malloc does not.

[RalfJung]

But the actually distributed rustc does gracefully handle this allocation failure... playground doesn't SIGKILL.

[RalfJung]

We also have another test triggering this OOM at tests/ui/consts/large_const_alloc.rs, which is enabled on all targets (including macOS). I have no idea why it would fail in this new test.

[jnkr-ifx]

It SIGKILLs for me on my local machine, using the latest stable & nightly compilers installed through rustup. It also SIGKILLs on Godbolt: https://rust.godbolt.org/z/ThsnaGfKW

Perhaps the playground has overcommit disabled?

[RalfJung]

I can try using the exact same size as the other test.

[RalfJung]

@bors try

[RalfJung]

It SIGKILLs for me on my local machine, using the latest stable & nightly compilers installed through rustup.

It shows the nice error (and the ICE) on my machine, using nightly installed via rustup. 🤷

[bors]

⌛ Trying commit c697434 with merge 5b858c9...

[RalfJung]

Oh wait I was testing the (1 << 47) - 1 version. That one shows the nice error. The usize::MAX >> 20 version SIGKILLs for me as well. What the...

[jnkr-ifx]

Yeah, looks like the difference in allocation size is all that's needed to trigger the difference in behavior. strace says usize::MAX >> 20 tries to allocate 20 TiB which succeeds due to overcommit and MAP_NORESERVE; but (1 << 47) - 1 is 128 TiB, which the kernel refuses (maybe due to virtual address space exhaustion?)

[RalfJung]

Yeah something like that.

Let's see if the try builds succeed now (they really should, since this is basically identical to the existing test), then we can land this.

[bors]

☀️ Try build successful - checks-actions
Build commit: 5b858c9 (5b858c9f9d756116971e80ef44d78f0d7a95a68d)

[RalfJung]

@bors r=jieyouxu

[bors]

📌 Commit c697434 has been approved by jieyouxu

It is now in the queue for this repository.

[bors]

⌛ Testing commit c697434 with merge 89b6885...

[bors]

☀️ Test successful - checks-actions
Approved by: jieyouxu
Pushing 89b6885 to master...

[rust-timer]

Finished benchmarking commit (89b6885): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 2.3%, secondary -2.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.3%	[0.8%, 3.9%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.5%	[-2.5%, -2.5%]	1
All ❌✅ (primary)	2.3%	[0.8%, 3.9%]	2

Cycles

Results (secondary -2.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.3%	[-2.3%, -2.3%]	2
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 795.261s -> 794.555s (-0.09%)
Artifact size: 335.30 MiB -> 335.29 MiB (-0.00%)