Repository navigation
Miscompilation caused by incorrectly-deduced readonly on virtual call #137646
Copy link
Copy link
Closed
Labels
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.C-bugCategory: This is a bug.Category: This is a bug.I-miscompileIssue: Correct Rust code lowers to incorrect machine codeIssue: Correct Rust code lowers to incorrect machine codeI-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessP-criticalCritical priorityCritical priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.regression-from-stable-to-stablePerformance or correctness regression from one stable version to another.Performance or correctness regression from one stable version to another.
Description
I tried the following code. It requires a binary crate and a library crate.
main.rs
fn main() {
let mut struct_c = lib::StructC::new();
let struct_a = lib::StructA { a: 1, b: 2, c: 3 };
lib::run(&mut struct_c, struct_a);
}main's Cargo.toml
[package]
name = "compilerbug"
version = "0.1.0"
edition = "2024"
[dependencies]
lib = { path = "./lib" }
[profile.dev]
opt-level = 1
incremental = false
lib.rs
#[derive(Clone, Copy)]
pub struct StructA {
pub a: i32,
pub b: i32,
pub c: i32,
}
pub trait TraitA {}
pub struct StructB {}
impl StructB {
pub fn new() -> Self {
Self {}
}
}
impl TraitA for StructB {}
pub trait TraitB {
fn get_trait_a(&self, struct_a: StructA) -> Option<Box<dyn TraitA>> {
Some(Box::new(StructB::new()))
}
}
pub struct StructC {}
impl StructC {
pub fn new() -> Self {
Self {}
}
}
impl TraitB for StructC {
fn get_trait_a(&self, mut struct_a: StructA) -> Option<Box<dyn TraitA>> {
struct_a.a = i32::MAX;
Some(Box::new(StructB::new()))
}
}
#[inline(never)]
fn print_struct_a(struct_a: StructA) {
println!("{}", struct_a.a);
}
#[inline(never)]
pub fn run(trait_b: &mut dyn TraitB, struct_a: StructA) {
print_struct_a(struct_a);
trait_b.get_trait_a(struct_a);
print_struct_a(struct_a);
// Uncommenting the following will fix the code.
// println!("{}", struct_a.a);
}lib's Cargo.toml
[package]
name = "lib"
version = "0.1.0"
edition = "2024"
Since struct_a conforms to Copy, passing struct_a to get_trait_a() should make a copy. Instead, when modifying struct_a inside of get_trait_a(), it modifies the original. When printing the struct a second time, the struct has been modified. To repro the bug, it requires opt-level >= 1 and incremental = false.
I expected the output in my command line to be:
1
1
Instead, this happened:
1
2147483647
Meta
rustc --version --verbose:
rustc 1.85.0 (4d91de4e4 2025-02-17)
binary: rustc
commit-hash: 4d91de4e48198da2e33413efdcd9cd2cc0c46688
commit-date: 2025-02-17
host: aarch64-apple-darwin
release: 1.85.0
LLVM version: 19.1.7
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.C-bugCategory: This is a bug.Category: This is a bug.I-miscompileIssue: Correct Rust code lowers to incorrect machine codeIssue: Correct Rust code lowers to incorrect machine codeI-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessP-criticalCritical priorityCritical priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.regression-from-stable-to-stablePerformance or correctness regression from one stable version to another.Performance or correctness regression from one stable version to another.
Activity
[-]Large Copy types breaks expected Copy semantics[/-][+]Large `Copy` types breaks expected `Copy` semantics[/+]steffahn commentedon Feb 26, 2025
well… among those that’s probably the LLVM bump then, right?
Bump host compiler on x64 dist Linux to LLVM 17.0.2 #116416~ or maybe not (see answers below)theemathas commentedon Feb 26, 2025
Might also be #116454
steffahn commentedon Feb 26, 2025
Interestingly, with the
marked section uncommented (and fixed to refer to
struct_aby the correct name), it’s also still broken for old Rust versions; regressing at the same point as before (2023-10-09), and actually being “fixed” again only at the following point:I don’t know, is that this one perhaps?
llvm::Lldif lld is not enabled #126701moxian commentedon Feb 26, 2025
steffahn commentedon Feb 26, 2025
It also isn’t “fixed” by uncommenting the indicated line, if
opt-level = 1is used.steffahn commentedon Feb 26, 2025
It’s really interesting how it seems to be relevant whether or not the default method body does mutate its argument. E.g.:
here the
println!("{:p}", &mut arr[0]);(or various other things that mutably access or actually mutatearr) do make a difference.moxian commentedon Feb 26, 2025
With
-Zmir-opt-level=0(thanks @saethlin for the pointer) it reproduces way further back to nightly-2023-07-16:Details
- #112157 sounds related maybe?steffahn commentedon Feb 26, 2025
That’s funny, my bisection (of my reduced example) with
-Zmir-opt-level=0also reproduces further back but not quite as far: only up tooh well… this might be because I use an array…
most likely that regression point was related to
because the regression was also mismatching between nightly and stable, suggesting a back-port…
Ah yes, with a tuple instead of an array I do get
nightly-2023-07-16as well :-)moxian commentedon Feb 26, 2025
I looked at LLVM IR, and realized I can't really read it myself, but I can at least share it here to save other folks some trouble:
https://godbolt.org/z/6znK7E3Md (original example, 1.72.0 - 1.73.0 )
https://godbolt.org/z/bjcEGn8s1 (slightly modified steffahn's
[i8;12]reduction, 1.69.0 - 1.70.0)11 remaining items
apiraino commentedon Feb 26, 2025
WG-prioritization assigning priority (Zulip discussion).
@rustbot label -I-prioritize +P-critical
Add a test case for rust-lang#137646
dianqk commentedon Feb 26, 2025
#137669 should fix this.
@saethlin Free to review if you would like.
[-]Large `Copy` types breaks expected `Copy` semantics[/-][+]Miscompilation caused by incorrectly-deduced readonly on virtual call[/+]Auto merge of rust-lang#137669 - DianQK:fn-atts-virtual, r=saethlin
Auto merge of rust-lang#137669 - DianQK:fn-atts-virtual, r=saethlin
Add a test case for rust-lang#137646
dianqk commentedon Mar 12, 2025
#138190 has been merged.