rustc's analyses have different order of eval for asm! than what trans emits.

[pnkfelix]

The main work items remaining here is the incorrect linting of the following example (play):

#![feature(asm)]

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
fn main() {
    #![warn(unused_assignments)]
    let mut x: isize = 0;
    let y: isize = 1;
    let mut z: isize;

    unsafe {
        asm!("mov ($1), $0"
             // Is dead_assignment going to complain about z=2 here ...
             : "=r"(*{z=2; &mut x})
             // ... or this z=3 here?
             : "r"(&{z=3; y}));
    }

    // Whichever one it complains about should be the *opposite*
    // of what we observe getting assigned here.
    assert_eq!((x,y,z), (1,1,3));
}

I probably should make sure that the other examples have been turned into tests.

Original bug report follows

---

While making test cases for #14873 and investigating how rustc currently models the control flow of asm! (and eventually filing #14936), I discovered something peculiar: trans emits code that evaluates the output expressions first, and then the input expressions (as illustrated in the test case for =r on #14936), but every rustc analysis that I looked at (liveness, expr_use_visitor, etc) treats asm! as if the input expressions are evaluated first, and then the output expressions.

Here is an illustrative test case: with some comments to try to clarify what is happening in the various cases

#![feature(asm)]

// turning off this lint to avoid distractions from incorrect internal
// analyses.  I will turn it back on in individual cases below to
// illustrate where rustc's analyses are going wrong today.
#![allow(dead_assignment)]

#[cfg(target_arch = "x86")]
#[cfg(target_arch = "x86_64")]
fn main() {
    // (All the test cases are listed here.  All but but the first are
    // no-ops unless enabled via a corresponding `--cfg` option.)
    overwrite_in_both();
    overwrite_in_both_with_liveness();
    augment_in_output();
    augment_in_input();
    crash_and_burn();
    return;

    fn overwrite_in_both() {
        let mut x: int = 0;
        let y: int = 1;
        let mut z: int;

        unsafe {
            asm!("mov ($1), $0"
                 // Which happens first, the output exprs...
                 : "=r"(*{z=2; &mut x})
                 // ... or the input exprs?
                 : "r"(&{z=3; y}));
        }

        assert_eq!((x,y,z), (1,1,3)); // I.e., should `z` be `2` ?
    }

    // Well, let us assume that the above behavior is what we actually want.
    // (If nothing else, it gives us left-to-right order of evaluation
    //  on the expressions fed into `asm!`)

    #[cfg(not(overwrite_in_both_with_liveness))]
    fn overwrite_in_both_with_liveness() { }
    #[cfg(overwrite_in_both_with_liveness)]
    fn overwrite_in_both_with_liveness() {
        #![deny(dead_assignment)]
        let mut x: int = 0;
        let y: int = 1;
        let mut z: int;

        unsafe {
            asm!("mov ($1), $0"
                 // Is dead_assignment going to complain about z=2 here ...
                 : "=r"(*{z=2; &mut x})
                 // ... or this z=3 here?
                 : "r"(&{z=3; y}));
        }

        // Whichever one it complains about should be the *opposite*
        // of what we observe getting assigned here.
        assert_eq!((x,y,z), (1,1,3));
    }



    #[cfg(not(augment_in_input))]
    fn augment_in_input() { }
    #[cfg(augment_in_input)]
    fn augment_in_input() {
        let mut x: int = 0;
        let y: int = 1;
        let mut z: int;

        unsafe {
            asm!("mov ($1), $0"
                 // Under the above assumption, this should work, since ...
                 : "=r"(*{z=2; &mut x})
                 // ... we assign 2 above and then add 3 here, yielding 5.
                 : "r"(&{z+=3; y}));
        }
        assert_eq!((x,y,z), (1,1,5));
    }

    #[cfg(not(augment_in_output))]
    fn augment_in_output() { }
    #[cfg(augment_in_output)]
    fn augment_in_output() {
        let mut x: int = 0;
        let y: int = 1;
        let mut z: int;

        unsafe {
            asm!("mov ($1), $0"
                 // Under the above assumption, should not compile, since ...
                 : "=r"(*{println!("z: {}", z); z+= 2; &mut x})
                 // ... we read z above, before it is assigned a value here.
                 : "r"(&{z=3; y}));
        }
        assert_eq!((x,y,z), (1,1,-314159)); // (deliberate chosen; expect fail)
    }

    #[cfg(not(crash_and_burn))]
    fn crash_and_burn() { }
    #[cfg(crash_and_burn)]
    fn crash_and_burn() {
        let mut x: int = 0;
        let y: int = 1;
        let mut z: ∫

        unsafe {
            asm!("mov ($1), $0"
                 // Under the above assumption, should not compile, since ...
                 : "=r"(*{println!("*z: {}", *z); &mut x})
                 // ... we read z above, before it is assigned a value here.
                 : "r"(&{z=&y; y}));
        }
        assert_eq!((x,y,*z), (1,1,-314159)); // (deliberate chosen; expect fail)
    }

}

#[cfg(not(target_arch = "x86"), not(target_arch = "x86_64"))]
pub fn main() {}

Transcript of various runs on above code:

% rustc --version
rustc 0.11.0-pre (79fca99 2014-06-17 04:46:26 +0000)
host: x86_64-apple-darwin
% rustc  ~/Dev/Rust/asm-flow.rs && ./asm-flow 
% rustc  ~/Dev/Rust/asm-flow.rs --cfg overwrite_in_both_with_liveness && ./asm-flow 
/Users/fklock/Dev/Rust/asm-flow.rs:54:26: 54:27 error: value assigned to `z` is never read
/Users/fklock/Dev/Rust/asm-flow.rs:54                  : "r"(&{z=3; y}));
                                                               ^
note: in expansion of asm!
/Users/fklock/Dev/Rust/asm-flow.rs:50:13: 54:36 note: expansion site
/Users/fklock/Dev/Rust/asm-flow.rs:44:17: 44:32 note: lint level defined here
/Users/fklock/Dev/Rust/asm-flow.rs:44         #![deny(dead_assignment)]
                                                      ^~~~~~~~~~~~~~~
/Users/fklock/Dev/Rust/asm-flow.rs:54:26: 54:27 error: value assigned to `z` is never read
/Users/fklock/Dev/Rust/asm-flow.rs:54                  : "r"(&{z=3; y}));
                                                               ^
note: in expansion of asm!
/Users/fklock/Dev/Rust/asm-flow.rs:50:13: 54:36 note: expansion site
/Users/fklock/Dev/Rust/asm-flow.rs:44:17: 44:32 note: lint level defined here
/Users/fklock/Dev/Rust/asm-flow.rs:44         #![deny(dead_assignment)]
                                                      ^~~~~~~~~~~~~~~
error: aborting due to 2 previous errors
% rustc  ~/Dev/Rust/asm-flow.rs --cfg augment_in_input && ./asm-flow 
/Users/fklock/Dev/Rust/asm-flow.rs:77:26: 77:30 error: use of possibly uninitialized variable: `z`
/Users/fklock/Dev/Rust/asm-flow.rs:77                  : "r"(&{z+=3; y}));
                                                               ^~~~
note: in expansion of asm!
/Users/fklock/Dev/Rust/asm-flow.rs:73:13: 77:37 note: expansion site
error: aborting due to previous error
% rustc  ~/Dev/Rust/asm-flow.rs --cfg augment_in_output && ./asm-flow 
z: 0
task '<main>' failed at 'assertion failed: `(left == right) && (right == left)` (left: `(1, 1, 3)`, right: `(1, 1, -314159)`)', /Users/fklock/Dev/Rust/asm-flow.rs:97
% rustc  ~/Dev/Rust/asm-flow.rs --cfg crash_and_burn && ./asm-flow 
Segmentation fault: 11
% 

Notes on the above transcript:

• The first run is just establishing our existing semantics as implemented by trans: we evaluate output expressions first, then input expressions. Thus the assert_eq in overwrite_in_both passes
• The second run establishes that the liveness analysis is incorrectly thinking that we will evaluate the input expressions first, and then the output expressions; that is why it is complaining that the assignment z=3 is unused, even though it is the other assignment z=2 that should be flagged as a dead_assignment.
• The third run establishes a case where the compiler outright errors (rather than lint warns), again due to an incorrect model of the order of evaluation.
• The fourth run points out how the analysis is not catching a read from uninitialized memory (since we print out a value for the println!("z: {}, z); but it is not printing z: 3 as the compiler might think it should; it is instead printing z: 0)
• The fifth run is just driving the point home about the danger here. Yes, this is a crash inside an unsafe block, but the expressions that caused the crash were unrelated to the asm block; they were due to an attempt to dereference an &int before it has been initialized.

[steveklabnik]

I think this has just worked out? A small diff:

#[cfg(any(not(target_arch = "x86"), not(target_arch = "x86_64")))]
pub fn main() {}

transcript:

steve@warmachine:~/tmp$ rustc asm-flow.rs --cfg overwrite_in_both_with_liveness && ./asm-flow
asm-flow.rs:6:10: 6:25 warning: unknown lint: `dead_assignment`, #[warn(unknown_lints)] on by default
asm-flow.rs:6 #![allow(dead_assignment)]
                       ^~~~~~~~~~~~~~~
steve@warmachine:~/tmp$ rustc asm-flow.rs --cfg augment_in_input && ./asm-flow  
asm-flow.rs:6:10: 6:25 warning: unknown lint: `dead_assignment`, #[warn(unknown_lints)] on by default
asm-flow.rs:6 #![allow(dead_assignment)]
                       ^~~~~~~~~~~~~~~
steve@warmachine:~/tmp$ rustc asm-flow.rs --cfg augment_in_output && ./asm-flow 
asm-flow.rs:6:10: 6:25 warning: unknown lint: `dead_assignment`, #[warn(unknown_lints)] on by default
asm-flow.rs:6 #![allow(dead_assignment)]
                       ^~~~~~~~~~~~~~~

[Mark-Simulacrum]

I'm seeing the following. All (compilable) asserts pass. The unused_assignments lint appears incorrect though. I've included some annotated code samples below drawn from the original issue which still represent incorrect behavior. Those that appear to have been fixed are no longer included.

As far as I can tell, we do now correctly detect when reading undefined memory -- so there has been progress. I'm uncertain if we've regressed since @steveklabnik's example above, but it does look like it...

#![feature(asm)]

fn main() {
    let mut x: isize = 0;
    let y: isize = 1;
    let mut z: isize;

    unsafe {
        asm!("mov ($1), $0"
                 : "=r"(*{z=2; &mut x})
                 : "r"(&{z=3; y}));
                 // ^ value assigned to `z` is never read -- this is wrong
    }

     assert_eq!(z, 3); // Passes
}

#![feature(asm)]

fn main() {
    let mut x: isize = 0;
    let y: isize = 1;
    let mut z: isize;

    unsafe {
        asm!("mov ($1), $0"
             : "=r"(*{z=2; &mut x})
             : "r"(&{z+=3; y}));
             // ^ value assigned is never read -- wrong
    }
    assert_eq!(z, 5); // Passes
}

[pnkfelix]

Yes it looks like the main item remaining here is the incorrect control flow model of the unused_assignments lint's treatment of asm!.

[Amanieu]

This is fixed with the new asm! implementation. However for some reason the warning is emitted twice: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=29264c7da298b07df0e441ddbd05d228

[Amanieu]

The double warning is fixed by #72537, so I'm going to close this issue.