ConstProp misoptimises pointer-typed enum field

[cbeuw]

Fuzzer generated MIR, reduced, and UB-free under Miri (for real this time 😅)

#![feature(custom_mir, core_intrinsics)]
extern crate core;
use core::intrinsics::mir::*;

#[custom_mir(dialect = "runtime", phase = "initial")]
fn fn4() {
    mir! {
    let _1: isize;
    let _12: Adt55;
    let unit: ();
    {
    _12 = Adt55::Variant1 { fld0: 0, fld1: 0};
    SetDiscriminant(_12, 0);
    place!(Field::<*mut isize>(Variant(_12, 0), 0)) = core::ptr::addr_of_mut!(_1);
    Call(unit = fn19(Field::<*mut isize>(Variant(_12, 0), 0)), bb11, UnwindUnreachable())
    }
    bb11 = {
    Return()
    }
    }
}
#[custom_mir(dialect = "runtime", phase = "initial")]
pub fn fn19(mut _1: *mut isize) {
    mir! {
    {
    (*_1) = 1;
    Return()
    }
    }
}
pub fn main() {
    fn4();
    println!("here");
}
#[derive(Debug, Copy, Clone)]
pub enum Adt55 {
    Variant0 { fld0: *mut isize },
    Variant1 { fld0: u8, fld1: u64 },
}

Segfaults with ConstProp enabled:

$ rustc -Zmir-opt-level=0 -Copt-level=0 -Zmir-enable-passes=+ConstProp repro.rs &&
 ./repro
Segmentation fault (core dumped)

Field::<*mut isize>(Variant(_12, 0), 0)), which is a valid pointer, somehow got propagated as 0:

// MIR for `fn4` before ConstProp

fn fn4() -> () {
    let mut _0: ();
    let mut _1: isize;
    let mut _2: Adt55;
    let mut _3: ();

    bb0: {
        _2 = Adt55::Variant1 { fld0: const 0_u8, fld1: const 0_u64 };
        discriminant(_2) = 0;
        ((_2 as variant#0).0: *mut isize) = &raw mut _1;
        _3 = fn19(((_2 as variant#0).0: *mut isize)) -> [return: bb1, unwind unreachable];
    }

    bb1: {
        return;
    }
}

// MIR for `fn4` after ConstProp

fn fn4() -> () {
    let mut _0: ();
    let mut _1: isize;
    let mut _2: Adt55;
    let mut _3: ();

    bb0: {
        _2 = Adt55::Variant1 { fld0: const 0_u8, fld1: const 0_u64 };
        discriminant(_2) = 0;
        ((_2 as variant#0).0: *mut isize) = &raw mut _1;
        _3 = fn19(const {0x0 as *mut isize}) -> [return: bb1, unwind unreachable];
    }

    bb1: {
        return;
    }
}

[workingjubilee]

Field::<*mut isize>(Variant(_12, 0), 0)), which is a valid pointer, somehow got propagated as 0:

That seems undesirable.

Is there a surface Rust program that can produce this MIR? Asking primarily for prioritization purposes.

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

[tmiasko]

Minimized test case:

#![feature(custom_mir, core_intrinsics, inline_const)]
extern crate core;
use core::intrinsics::mir::*;

#[custom_mir(dialect = "runtime", phase = "initial")]
fn size_of<T>() -> usize {
    mir! {
        let a : usize;
        {
            a = 0;
            a = const { std::mem::size_of::<T>() };
            RET = a;
            Return()
        }
    }
}

fn main() {
    assert_eq!(size_of::<u32>(), std::mem::size_of::<u32>());
}

$ rustc    a.rs && ./a
$ rustc -O a.rs && ./a
thread 'main' panicked at a.rs:19:5:
assertion `left == right` failed
  left: 0
 right: 4
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[lqd]

Here's an example using stable surface syntax (playground).

#![allow(unused_assignments)]

struct SizeOfConst<T>(std::marker::PhantomData<T>);
impl<T> SizeOfConst<T> {
    const SIZE: usize = std::mem::size_of::<T>();
}

fn size_of<T>() -> usize {
    let mut a = 0;
    a = SizeOfConst::<T>::SIZE;
    a
}

fn main() {
    assert_eq!(size_of::<u32>(), std::mem::size_of::<u32>());
}

Seems to have been introduced in 1.70, with #108872 in particular.

[Urgau]

Using @lqd minimization it seems to be a LLVM miscompilation as Alive2 complains about the optimizations applied.

Nevermind seems like I was wrong.

define noundef i64 @src() {
  %a = alloca i64, align 8
  store i64 0, ptr %a, align 8
  store i64 4, ptr %a, align 8
  %_0 = load i64, ptr %a, align 8
  ret i64 %_0
}

define noundef i64 @tgt() {
  ret i64 0
}

[aDotInTheVoid]

It's not an LLVM miscompilation, the code is already ret i64 0 by the time it goes to LLVM. https://godbolt.org/z/r1W13h1sY.

With rustc +nightly ./lol.rs -Z dump-mir=size_of -O

--- ./mir_dump/lol.size_of.005-015.ConstProp.before.mir 2023-11-28 14:38:07.519258465 +0000
+++ ./mir_dump/lol.size_of.005-015.ConstProp.after.mir  2023-11-28 14:38:07.519258465 +0000
@@ -1,4 +1,4 @@
-// MIR for `size_of` before ConstProp
+// MIR for `size_of` after ConstProp

 fn size_of() -> usize {
     let mut _0: usize;
@@ -11,7 +11,7 @@
         StorageLive(_1);
         _1 = const 0_usize;
         _1 = const _;
-        _0 = _1;
+        _0 = const 0_usize;
         StorageDead(_1);
         return;
     }

So it's defiantly happening in MIR ConstProp

[aDotInTheVoid]

The problem seems to be that when running on the statement _1 = const _;, the frames arn't updated, so when it runs on _0 = _1;, it thinks 0x0 is their for local _1.

│ │ ├─┐rustc_mir_transform::const_prop::visit_statement statement=_1 = const _, location=bb0[2]
│ │ │ ├─  0ms TRACE rustc_mir_transform::const_prop initial frame, frame=[LocalState { value: Live(Immediate(Uninit)), ty: None }, LocalState { value: Live(Immediate(Scalar(0x0000000000000000))), ty: Some(usize) }]
│ │ │ ├─┐rustc_mir_transform::const_prop::visit_assign place=_1, rvalue=const _, location=bb0[2]
│ │ │ │ ├─┐rustc_mir_transform::const_prop::visit_operand operand=const _, location=bb0[2]
│ │ │ │ │ ├─  0ms TRACE rustc_mir_transform::const_prop about to do it
│ │ │ │ ├─┘
│ │ │ │ ├─┐rustc_mir_transform::const_prop::check_rvalue rvalue=const _
│ │ │ │ ├─┘
│ │ │ ├─┘
│ │ │ ├─  0ms TRACE rustc_mir_transform::const_prop final frame, frame=[LocalState { value: Live(Immediate(Uninit)), ty: None }, LocalState { value: Live(Immediate(Scalar(0x0000000000000000))), ty: Some(usize) }]
│ │ ├─┘

./x build library --keep-stage 1 && RUSTC_LOG=rustc_mir_transform::const_prop=trace,rustc_const_eval::interpret=trace ./build/host/stage1/bin/rustc ./tests/mir-opt/const_prop/issue_118328.rs -O 2> log.txt, on aDotInTheVoid@aef5074 to add logging and test.

(full log)

[aDotInTheVoid]

@rustbot claim

[RalfJung]

Cc @cjgillot