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

CopyProp miscompilation when src is moved more than once #137936

Open
tmiasko opened this issue Mar 3, 2025 · 2 comments
Open

CopyProp miscompilation when src is moved more than once #137936

tmiasko opened this issue Mar 3, 2025 · 2 comments
Labels
A-mir-opt Area: MIR optimizations C-bug Category: This is a bug. I-miscompile Issue: Correct Rust code lowers to incorrect machine code I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tmiasko
Copy link
Contributor

tmiasko commented Mar 3, 2025

#![feature(custom_mir, core_intrinsics, freeze)]
#![allow(internal_features, unused_assignments)]
extern crate core;
use core::intrinsics::mir::*;
use core::mem::MaybeUninit;
use core::marker::Freeze;

#[custom_mir(dialect = "analysis", phase = "post-cleanup")]
fn f<T: Copy + Freeze>(a: MaybeUninit<T>) {
    mir! {
        {
            let b = Move(a);
            let c = Move(a);
            Call(RET = g(Move(b), Move(c)), ReturnTo(ret), UnwindContinue())
        }
        ret = {
            Return()
        }
    }
}

fn g<T: Copy + Freeze>(mut a: T, mut b: T) {
    assert_ne!(&mut a as *mut _ as usize, &mut b as *mut _ as usize);
}

fn main() {
    f(MaybeUninit::<[u8;  64]>::uninit());
}

The CopyProp introduces following changes:

--- a.f.005-021.CopyProp.before.mir	2025-03-03 13:42:20.628850669 +0100
+++ a.f.005-021.CopyProp.after.mir	2025-03-03 13:42:20.628850669 +0100
@@ -1,17 +1,15 @@
-// MIR for `f` before CopyProp
+// MIR for `f` after CopyProp
 
 fn f(_1: MaybeUninit<T>) -> () {
     let mut _0: ();
     let mut _2: std::mem::MaybeUninit<T>;
     let mut _3: std::mem::MaybeUninit<T>;
 
     bb0: {
-        _2 = move _1;
-        _3 = move _1;
-        _0 = g::<MaybeUninit<T>>(move _2, move _3) -> [return: bb1, unwind continue];
+        _0 = g::<MaybeUninit<T>>(move _1, move _1) -> [return: bb1, unwind continue];
     }
 
     bb1: {
         return;
     }
 }
$ rustc a.rs -Zmir-opt-level=0 && ./a
$ rustc a.rs -Zmir-opt-level=1 && ./a

thread 'main' panicked at a.rs:23:5:
assertion `left != right` failed
  left: 140729201385720
 right: 140729201385720
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
@tmiasko tmiasko added A-mir-opt Area: MIR optimizations C-bug Category: This is a bug. requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 3, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 3, 2025
@tmiasko tmiasko added I-miscompile Issue: Correct Rust code lowers to incorrect machine code and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 3, 2025
@bjorn3
Copy link
Member

bjorn3 commented Mar 3, 2025

Isn't this kind of expected? You shouldn't move a local twice without reinitializing it. And I don't think MIR building would ever emit this pattern.

@tmiasko
Copy link
Contributor Author

tmiasko commented Mar 3, 2025

The example has well defined behavior according to the spec. I don't think it is that far fetched from being result of some optimization, consider e.g.:

fn f<T: Freeze>(mut a: MaybeUninit<T>) {
    let b = a;
    a = const { MaybeUninit::uninit() };
    let c = a;
    g(b, c);
}

@tmiasko tmiasko added the I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness label Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mir-opt Area: MIR optimizations C-bug Category: This is a bug. I-miscompile Issue: Correct Rust code lowers to incorrect machine code I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants