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.

Sign up for GitHub

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

Jump to bottom

interpret: Fix writing uninit to an allocation #96162

Merged
merged 4 commits into from
Apr 20, 2022
Merged

interpret: Fix writing uninit to an allocation #96162

merged 4 commits into from
Apr 20, 2022

Conversation

RalfJung
Copy link
Member

When calling mark_init, we need to also be mindful of what happens with the relocations! Specifically, when we de-init memory, we need to clear relocations in that range as well or else strange things will happen (and printing will not show the de-init, since relocations take precedence there).

Fixes rust-lang/miri#2068.

Here's the Miri testcase that this fixes (requires -Zmiri-disable-validation):

use std::mem::MaybeUninit;

fn main() { unsafe {
    let mut x = MaybeUninit::<i64>::uninit();
    // Put in a ptr.
    x.as_mut_ptr().cast::<&i32>().write_unaligned(&0);
    // Overwrite parts of that pointer with 'uninit' through a Scalar.
    let ptr = x.as_mut_ptr().cast::<i32>();
    *ptr = MaybeUninit::uninit().assume_init();
    // Reading this back should hence work fine.
    let _c = *ptr;
} }

Previously this failed with

error: unsupported operation: unable to turn pointer into raw bytes
  --> ../miri/uninit.rs:11:14
   |
11 |     let _c = *ptr;
   |              ^^^^ unable to turn pointer into raw bytes
   |
   = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
           
   = note: inside `main` at ../miri/uninit.rs:11:14

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 17, 2022
@rust-highfive
Copy link
Collaborator

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
Copy link
Collaborator

r? @davidtwco

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 17, 2022
@RalfJung
Copy link
Member Author

r? @oli-obk

@rust-highfive rust-highfive assigned oli-obk and unassigned davidtwco Apr 17, 2022
RalfJung
RalfJung commented Apr 17, 2022
View reviewed changes
compiler/rustc_middle/src/mir/interpret/allocation.rs
@@ -599,6 +605,9 @@ impl<Tag: Copy, Extra> Allocation<Tag, Extra> {
/// Applies a relocation copy.
/// The affected range, as defined in the parameters to `prepare_relocation_copy` is expected
/// to be clear of relocations.
///
/// This is dangerous to use as it can violate internal `Allocation` invariants!
/// It only exists to support an efficient implementation of `mem_copy_repeatedly`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried finding a nice safe API we could add here to support mem_copy_repeatedly and make all this "compressed range" stuff private, but couldn't come up with anything good.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warnings are there now, so we can look at it again in the future

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@oli-obk
Copy link
Contributor

oli-obk commented Apr 19, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Apr 19, 2022

📌 Commit 05489e7 has been approved by oli-obk

@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 Apr 19, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 19, 2022
@Dylan-DPC
Rollup merge of rust-lang#96162 - RalfJung:mark-uninit, r=oli-obk
357bd4a 
interpret: Fix writing uninit to an allocation

When calling `mark_init`, we need to also be mindful of what happens with the relocations! Specifically, when we de-init memory, we need to clear relocations in that range as well or else strange things will happen (and printing will not show the de-init, since relocations take precedence there).

Fixes rust-lang/miri#2068.

Here's the Miri testcase that this fixes (requires `-Zmiri-disable-validation`):
```rust
use std::mem::MaybeUninit;

fn main() { unsafe {
    let mut x = MaybeUninit::<i64>::uninit();
    // Put in a ptr.
    x.as_mut_ptr().cast::<&i32>().write_unaligned(&0);
    // Overwrite parts of that pointer with 'uninit' through a Scalar.
    let ptr = x.as_mut_ptr().cast::<i32>();
    *ptr = MaybeUninit::uninit().assume_init();
    // Reading this back should hence work fine.
    let _c = *ptr;
} }
```
Previously this failed with
```
error: unsupported operation: unable to turn pointer into raw bytes
  --> ../miri/uninit.rs:11:14
   |
11 |     let _c = *ptr;
   |              ^^^^ unable to turn pointer into raw bytes
   |
   = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support

   = note: inside `main` at ../miri/uninit.rs:11:14
```
@Dylan-DPC Dylan-DPC mentioned this pull request Apr 19, 2022
Closed
@Dylan-DPC Dylan-DPC mentioned this pull request Apr 19, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 20, 2022
@bors
Auto merge of rust-lang#96224 - Dylan-DPC:rollup-h2h3j93, r=Dylan-DPC
a2df8ba 
Rollup of 6 pull requests

Successful merges:

 - rust-lang#95740 (asm: Add a kreg0 register class on x86 which includes k0)
 - rust-lang#95813 (Remove extra space before a where clause)
 - rust-lang#96029 (Refactor loop into iterator; simplify negation logic.)
 - rust-lang#96162 (interpret: Fix writing uninit to an allocation)
 - rust-lang#96165 (Miri provenance cleanup)
 - rust-lang#96205 (Use futex locks on emscripten.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f7d8f5b into rust-lang:master Apr 20, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 20, 2022
@RalfJung RalfJung deleted the mark-uninit branch April 26, 2022 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk left review comments

Assignees

@oli-obk oli-obk

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
Milestone
1.62.0
Development

Successfully merging this pull request may close these issues.

Miri reports "unsupported operation: unable to turn pointer into raw bytes" in futures-lite-1.12.0
7 participants
@RalfJung @rust-highfive @rust-log-analyzer @oli-obk @bors @davidtwco @rustbot