Skip to content

GVN misunderstands aliasing, can create overlapping assignments (again) #141313

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
saethlin opened this issue May 20, 2025 · 2 comments
Open

GVN misunderstands aliasing, can create overlapping assignments (again) #141313

saethlin opened this issue May 20, 2025 · 2 comments
Assignees
Labels
A-mir-opt Area: MIR optimizations A-mir-opt-GVN Area: MIR opt Global Value Numbering (GVN) A-rustlantis A miscompilation found by Rustlantis 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 P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@saethlin
Copy link
Member

saethlin commented May 20, 2025

This looks very similar to #141038, but the compiler that found this has the patch that was linked to that issue.

Reduced example from rustlantis, which is accepted by Miri without optimizations enabled:

#![feature(custom_mir, core_intrinsics)]
#![allow(internal_features)]

use std::intrinsics::mir::*;

#[custom_mir(dialect = "runtime")]
fn main() {
    mir!{
        let _1;
        let _2;
        let _3;
        {
            place!(Field(Variant(_1, 1), 0)) = 0u8;
            _3 = &_1;
            _2 = Field(Variant(*_3, 1), 0);
            _1 = Adt::Some(_2);
            Return()
        }
    }
}

#[allow(dead_code)]
enum Adt {
    None,
    Some(u8),
}

If I run this under Miri with -Zmir-enable-passes=+GVN, I see:

error: Undefined Behavior: `copy_nonoverlapping` called on overlapping ranges
  --> 4243580941269764965.rs:16:13
   |
16 |             _1 = Adt::Some(_2);
   |             ^^^^^^^^^^^^^^^^^^ `copy_nonoverlapping` called on overlapping ranges
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: BACKTRACE:
   = note: inside `main` at 4243580941269764965.rs:16:13: 16:31

The MIR diff for GVN is:

-// MIR for `main` before GVN
+// MIR for `main` after GVN
 
 fn main() -> () {
     let mut _0: ();
     let mut _1: Adt;
     let mut _2: u8;
     let mut _3: &Adt;
 
     bb0: {
         ((_1 as variant#1).0: u8) = const 0_u8;
         _3 = &_1;
         _2 = copy (((*_3) as variant#1).0: u8);
-        _1 = Adt::Some(copy _2);
+        _1 = copy (*_3);
         return;
     }
 }
rustc 1.89.0-nightly (60dabef95 2025-05-19)
binary: rustc
commit-hash: 60dabef95a3de3ec974dcb50926e4bfe743f078f
commit-date: 2025-05-19
host: aarch64-unknown-linux-gnu
release: 1.89.0-nightly
LLVM version: 20.1.5
@saethlin saethlin added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness C-bug Category: This is a bug. A-mir-opt Area: MIR optimizations I-miscompile Issue: Correct Rust code lowers to incorrect machine code A-mir-opt-GVN Area: MIR opt Global Value Numbering (GVN) A-rustlantis A miscompilation found by Rustlantis labels May 20, 2025
@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 20, 2025
@dianqk
Copy link
Member

dianqk commented May 21, 2025

#141251, #141038 and this one is telling me that the _1 of &_1 has to be a ssa local.

@rustbot claim

@apiraino
Copy link
Contributor

Assigning priority (discussion on Zulip).

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels May 21, 2025
@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 21, 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 A-mir-opt-GVN Area: MIR opt Global Value Numbering (GVN) A-rustlantis A miscompilation found by Rustlantis 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 P-medium Medium priority 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

5 participants