Miscompilation where binding only some fields leaks the others

I found a bug where Rust leaks values (never calls drop). This is the root cause of a memory leak we saw in a Fuchsia test, caught by LeakSanitizer: https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=87961.

This is as far as I could minimize it:

#![allow(unused)]

struct S(u32);

impl Drop for S {
    fn drop(&mut self) {
        println!("dropping {}", self.0);
    }
}

enum E {
    A,
    B((S, S)),
}

fn main() {
    let mut foo = Some(E::A);
    match foo {
        Some(E::A) => (),
        _ => return,
    }
    foo.insert(E::B((S(1), S(2))));
    match foo {
        Some(E::B((x, _))) => {}
        _ => {}
    }
}

Expected output:

dropping 2
dropping 1

Actual output:

dropping 1

It leaked S(2). If you change (x, _) to (_, x) in the second match statement, it leaks S(1) instead. If you bind neither (_, _) or both (x, y), then both get dropped as expected.

If you use Box<u8> instead of S, you can detect a memory leak using LeakSanitizer or Valgrind.

Meta

I reproduced this on stable:

rustc 1.56.1 (59eed8a2a 2021-11-01)
binary: rustc
commit-hash: 59eed8a2aac0230a8b53e89d4e99d55912ba6b35
commit-date: 2021-11-01
host: x86_64-unknown-linux-gnu
release: 1.56.1
LLVM version: 13.0.0

And on nightly:

rustc 1.58.0-nightly (8b09ba6a5 2021-11-09)
binary: rustc
commit-hash: 8b09ba6a5d5c644fe0f1c27c7f9c80b334241707
commit-date: 2021-11-09
host: x86_64-unknown-linux-gnu
release: 1.58.0-nightly
LLVM version: 13.0.0

Note that the miscompilation goes away if you change foo.insert((E::B) to foo = Some(E::B). So I suspect the unreachable_unchecked in insert is what's causing this somehow.

rust/library/core/src/option.rs

Line 1211 in e5cfe84

unsafe { self.as_mut().unwrap_unchecked() }

Also, the miscompilation is in MIR, not LLVM, because Miri also fails to run the destructor.

Very slightly smaller (I removed the fields in S):

struct S;

impl Drop for S {
    fn drop(&mut self) {
        println!("dropping");
    }
}

enum E {
    A,
    B((S, S)),
}

fn main() {
    let mut foo = Some(E::A);
    match foo {
        Some(E::A) => (),
        Some(E::B(_)) | None => return,
    }
    foo.insert(E::B((S, S)));
    match foo {
        Some(E::B((x, _))) => {}
        _ => {},
    };
}

Oh strangely this is not related to the unwrap_unchecked, it still reproduces with a normal unwrap. https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=6e6a87916e94f4012b2d93615a7ce1d1

Here's a version without std::option::Option (just an in-crate option): https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=025985867f32378dad0785597377a680

@rustbot

A quick manual bisection on godbolt shows that this regresses in Rust 1.43 (is correct in Rust 1.42).

@rustbot label: regression-from-stable-to-stable

@ecstatic-morse

Bisection reveals that this issue is introduced in #68528.

I think the issue is that precise_enum_drop_elaboration didn't re-mark all variants as possibly active after the mutable reference to the enum is used. The miscompilation goes away with -Z precise_enum_drop_elaboration=n. I filed #90756 to disable precise_enum_drop_elaboration by default, and we can turn this back on once the issue is fixed.

cc @ecstatic-morse
@rustbot label: +A-mir

Add raw pointer variant of rust-lang#90752 with incorrect behavior

@rustbot

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-critical

FYI This is addressed by PR #90788, which will be part of the Rust 1.58 release (stable in six weeks).

@mk12

Checking progress, @mk12 did you have a chance to test against the new stable 1.58 if it solves the issue? thanks

@apiraino

@apiraino The example I gave in my first comment now behaves as expected on the Rust playground with 1.58.1 stable. I'm fairly sure this solves the original Fuchsia bug, but it's difficult to reproduce that because a workaround was used at the time and that code has changed a lot since.

I'm going to close this issue as we think it's solved. In case it can be reopened or follow-up issues can be created

Navigation Menu

Repository navigation

Miscompilation where binding only some fields leaks the others #90752

Meta

13 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Miscompilation where binding only some fields leaks the others #90752

Description

Meta

Activity

jyn514 commented on Nov 10, 2021

jyn514 commented on Nov 10, 2021

jyn514 commented on Nov 10, 2021

jyn514 commented on Nov 10, 2021

jyn514 commented on Nov 10, 2021

nbdd0121 commented on Nov 10, 2021

nbdd0121 commented on Nov 10, 2021

13 remaining items

apiraino commented on Nov 18, 2021

pnkfelix commented on Dec 2, 2021

apiraino commented on Jan 20, 2022

mk12 commented on Jan 22, 2022

apiraino commented on Jan 27, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions