Skip to content

MIR: asm statements don't participate in moveck #45695

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

Closed
arielb1 opened this issue Nov 1, 2017 · 3 comments
Closed

MIR: asm statements don't participate in moveck #45695

arielb1 opened this issue Nov 1, 2017 · 3 comments
Labels
A-borrow-checker Area: The borrow checker A-inline-assembly Area: Inline assembly (`asm!(…)`) I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness

Comments

@arielb1
Copy link
Contributor

arielb1 commented Nov 1, 2017

For example,

#![feature(asm)]

fn main() {
    let y: &mut isize;
    let x = &mut 0isize;
    unsafe {
        asm!("mov $1, $0" : "=r"(y) : "r"(x));
    }
    println!("{} {}", x, y);
}

This errors out in this way (with MIR borrowck enabled):

error[E0382]: use of moved value: `x` (Ast)
 --> fail.rs:9:23
  |
7 |         asm!("mov $1, $0" : "=r"(y) : "r"(x));
  |                                           - value moved here
8 |     }
9 |     println!("{} {}", x, y);
  |                       ^ value used here after move
  |
  = note: move occurs because `x` has type `&mut isize`, which does not implement the `Copy` trait

error[E0381]: borrow of possibly uninitialized variable: `x` (Mir)
 --> fail.rs:9:23
  |
9 |     println!("{} {}", x, y);
  |                       ^ use of possibly uninitialized `x`

error[E0381]: borrow of possibly uninitialized variable: `y` (Mir)
 --> fail.rs:9:26
  |
9 |     println!("{} {}", x, y);
  |                          ^ use of possibly uninitialized `y`

error: aborting due to 3 previous errors

Where only x is uninitialized with the AST borrowck, but both are uninitialized with MIR borrowck

Also, if there's a destructor for an input, it will run right after the asm statement is executed, which doesn't make any sense (the assertion there should fail, not succeed)

#![feature(asm)]

use std::cell::Cell;

#[repr(C)]
struct NoisyDrop<'a>(&'a Cell<&'static str>);
impl<'a> Drop for NoisyDrop<'a> {
    fn drop(&mut self) {
        self.0.set("destroyed!");
    }
}

fn main() {
    let status = Cell::new("alive");
    let _y: Box<NoisyDrop>;
    let x = Box::new(NoisyDrop(&status));
    unsafe {
        asm!("mov $1, $0" : "=r"(_y) : "r"(x));
    }
    assert_eq!(status.get(), "destroyed!"); // this makes 0 sense
}

This is because move checking just ignores inline asm, while it should treat them like AST borrowck does (inputs move data out, outputs move data in)

@arielb1 arielb1 added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness WG-compiler-nll labels Nov 1, 2017
@arielb1 arielb1 changed the title MIR: asm statements don't participate in data movement MIR: asm statements don't participate in moveck Nov 1, 2017
@arielb1 arielb1 added A-inline-assembly Area: Inline assembly (`asm!(…)`) A-borrow-checker Area: The borrow checker labels Nov 1, 2017
@nikomatsakis
Copy link
Contributor

@arielb1

Also, if there's a destructor for an input, it will run right after the asm statement is executed, which doesn't make any sense (the assertion there should fail, not succeed)

To be clear, you are saying this is a pre-existing bug with drop elaboration?

@arielb1 arielb1 added this to the NLL prototype milestone Nov 15, 2017
@nikomatsakis
Copy link
Contributor

Removing from demo milestone, though this seems like a good one to mentor as an intro to how things work.

@nikomatsakis nikomatsakis removed this from the NLL prototype milestone Nov 16, 2017
@arielb1
Copy link
Contributor Author

arielb1 commented Nov 19, 2017

This should be fairly easy to fix, but @matthewjasper is refactoring the area a bit in #46022, so I would wait for him to be done.

bors added a commit that referenced this issue Dec 26, 2017
[MIR Borrowck] Moveck inline asm statements

Closes #45695

New behavior:
* Input operands to `asm!` are moved, direct output operands are initialized.
* Direct, non-read-write outputs match the assignment changes in #46752 (Shallow writes, end borrows).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker A-inline-assembly Area: Inline assembly (`asm!(…)`) I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness
Projects
None yet
Development

No branches or pull requests

2 participants