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

miri: fix ICE when running out of address space #107756

Merged
merged 1 commit into from
Feb 8, 2023

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Feb 7, 2023

Fixes rust-lang/miri#2769
r? @oli-obk

I didn't add a test since that requires oli-obk/ui_test#38 (host must be 64bit and target 32bit). Also the test takes ~30s, so I am not sure if we want to have it in the test suite?

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 7, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 7, 2023

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

The Miri subtree was changed

cc @rust-lang/miri

@RalfJung
Copy link
Member Author

RalfJung commented Feb 7, 2023

Ah, the Vec-based version of the test is actually instantaneous:

//@only-32bit
//@only-host-x86_64
fn main() {
    for _ in 0..4 {
        let mut a: Vec<u8> = Vec::with_capacity(1024 * 1024 * 1024);
        let _ptr = a.spare_capacity_mut().as_ptr();
    }
}

Still blocked on oli-obk/ui_test#38 though.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 7, 2023

🤣 the following rustc program takes 40s to compile on my machine, and uses around 10GB of RAM on a "32 bit machine" (the const evaluator's emulated target):

const FOO: () = {
    foo(10);
};

const fn foo(x: usize) {
    if x != 0 {
        let mut array = [0_u8; usize::MAX / 4];
        foo(x - 1);
        array[array.len() - 1] = 42;
    }
}

fn main() {
    FOO;
}

obvious in hindsight, and could even be useful for embedded systems doing lotsa work at compile-time, but it makes absolutely no sense outside of the "all addresses are symbolic" scheme

@oli-obk
Copy link
Contributor

oli-obk commented Feb 7, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Feb 7, 2023

📌 Commit 2900ba1 has been approved by oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 7, 2023
@RalfJung
Copy link
Member Author

RalfJung commented Feb 7, 2023

Oh yeah, in CTFE we can totally have more than 4GB of virtual address space with 32bit pointers. With perfect provenance tracking we can basically assign the same physical address multiple times and there is no possibility of confusion. (We don't actually assign any physical address of course, but we could imagine that we did.)

@oli-obk
Copy link
Contributor

oli-obk commented Feb 7, 2023

@bors rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2023
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#107719 (Remove `arena_cache` modifier from `upstream_monomorphizations_for`)
 - rust-lang#107740 (Avoid locking the global context across the `after_expansion` callback)
 - rust-lang#107746 (Split fn_ctxt/adjust_fulfillment_errors from fn_ctxt/checks)
 - rust-lang#107749 (allow quick-edit convenience)
 - rust-lang#107750 (make more readable)
 - rust-lang#107755 (remove binder from query constraints)
 - rust-lang#107756 (miri: fix ICE when running out of address space)
 - rust-lang#107764 (llvm-16: Use Triple.h from new header location.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d044c1b into rust-lang:master Feb 8, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 8, 2023
@RalfJung RalfJung deleted the miri-out-of-addresses branch February 11, 2023 21:31
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 16, 2024
interpret: remove outdated comment

In rust-lang#107756, allocation became generally fallible, so the "only panic if there is provenance" no longer applies.

r? `@oli-obk`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 16, 2024
interpret: remove outdated comment

In rust-lang#107756, allocation became generally fallible, so the "only panic if there is provenance" no longer applies.

r? ``@oli-obk``
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2024
Rollup merge of rust-lang#124024 - RalfJung:interpret-comment, r=oli-obk

interpret: remove outdated comment

In rust-lang#107756, allocation became generally fallible, so the "only panic if there is provenance" no longer applies.

r? ``@oli-obk``
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Apr 17, 2024
interpret: remove outdated comment

In rust-lang/rust#107756, allocation became generally fallible, so the "only panic if there is provenance" no longer applies.

r? ``@oli-obk``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Miri reports UB in safe code due to address space exhaustion (?)
4 participants