Skip to content

Investigate why this by-move-argument-access is not caught #4189

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

Open
RalfJung opened this issue Feb 12, 2025 · 4 comments
Open

Investigate why this by-move-argument-access is not caught #4189

RalfJung opened this issue Feb 12, 2025 · 4 comments
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug.

Comments

@RalfJung
Copy link
Member

struct Foo([u8;100]);

static mut F: *const Foo = std::ptr::null();

fn escape(x: &Foo) {
    unsafe {
        F = x as _;
    }
}

fn move_arg(mut x: Foo) {
    unsafe {
        x.0[0] = 1;
        assert_eq!((*F).0[0], 0);
    }
}

fn src(x: Foo) {
    escape(&x);
    move_arg(x);
}

fn main() {
    src(Foo([0;100]));
}

(via Zulip)

@RalfJung
Copy link
Member Author

There seem to be multiple confounding factors here:

  • One has to build this with a non-zero mir-opt-level to even get fully in-place argument passing from src to move_arg. Miri runs with opt-level 0 which has extra copies which avoids the UB. The optimization that removes the copies is wrong if we assume Miri semantics. I opened Copy propagation on "move" assignments introduces UB (using Miri/MiniRust semantics) unsafe-code-guidelines#556 for that.
  • But even with -Zmir-opt-level=1, we do not get in-place argument passing in Miri. Somehow, if retags are present, then GVN turns the move into a copy. Before GVN:
fn src(_1: Foo) -> () {
    debug x => _1;
    let mut _0: ();
    let mut _2: &Foo;
    let _3: &Foo;
    let _4: ();
    let mut _5: Foo;
    scope 1 (inlined escape) {
        let mut _6: *const Foo;
        let mut _7: *mut *const Foo;
    }

    bb0: {
        StorageLive(_2);
        StorageLive(_3);
        _3 = &_1;
        _2 = copy _3;
        StorageLive(_6);
        Retag(_2);
        _6 = &raw const (*_2);
        StorageLive(_7);
        _7 = const {alloc1: *mut *const Foo};
        (*_7) = copy _6;
        StorageDead(_7);
        StorageDead(_6);
        StorageDead(_2);
        StorageDead(_3);
        StorageLive(_5);
        _5 = move _1;
        _4 = move_arg(move _5) -> [return: bb1, unwind continue];
    }

    bb1: {
        StorageDead(_5);
        return;
    }
}

After GVN:

fn src(_1: Foo) -> () {
    debug x => _1;
    let mut _0: ();
    let mut _2: &Foo;
    let _3: &Foo;
    let _4: ();
    let mut _5: Foo;
    scope 1 (inlined escape) {
        let mut _6: *const Foo;
        let mut _7: *mut *const Foo;
    }

    bb0: {
        StorageLive(_2);
        nop;
        _3 = &_1;
        _2 = copy _3;
        nop;
        Retag(_2);
        _6 = &raw const (*_2);
        StorageLive(_7);
        _7 = const {alloc1: *mut *const Foo};
        (*_7) = copy _6;
        StorageDead(_7);
        nop;
        StorageDead(_2);
        nop;
        StorageLive(_5);
        _5 = copy _1;
        _4 = move_arg(copy _1) -> [return: bb1, unwind continue];
    }

    bb1: {
        StorageDead(_5);
        return;
    }
}

This can be reproduced with rustc --emit=mir move.rs -O -Zmir-emit-retag -Zdump-mir=src.

@rust-lang/wg-mir-opt @cjgillot @compiler-errors any idea why GVN might be doing this?

@RalfJung RalfJung added C-bug Category: This is a bug. A-interpreter Area: affects the core interpreter labels Mar 19, 2025
@RalfJung
Copy link
Member Author

RalfJung commented Apr 1, 2025

@scottmcm do you have any idea what GVN is doing here?

@scottmcm
Copy link
Member

scottmcm commented Apr 1, 2025

I haven't dug into the example specifically, but whenever GVN re-uses something it removes all the storage markers for that value and changes all the uses to copy rather than move. So maybe that's related?

@RalfJung
Copy link
Member Author

RalfJung commented Apr 1, 2025

What I am wondering is why adding a Retag(_2) changes whether GVN kicks in for _5 (which is the same as _1).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

2 participants