MIR dynamic drop flags cause spurious extra drops with optimizations on.

[alexcrichton]

This program generates an assertion failure on nightly Rust:

use std::sync::*;
use std::sync::atomic::*;
use std::sync::atomic::Ordering::*;

fn main() {
    struct Inner(usize);

    // This assertion in theory never trips, there's not mutability in
    // `Arc<Inner>`! Yet this fails.
    impl Drop for Inner {
        fn drop(&mut self) {
            assert_eq!(self.0, 0);
        }
    }
    let b = Arc::new(Inner(0));

    drop(WriteQueue::new(b).poll());
}

pub struct WriteQueue<W>  {
    pending: Option<Sender>,
    state: State<W>,
}

enum State<W> {
    Writing(W, Sender),
    BetweenWrites(W),
    Empty,
}

impl<W> WriteQueue<W> {
    pub fn new(writer: W) -> WriteQueue<W> {
        let (complete, _) = channel();
        WriteQueue {
            pending: Some(complete),
            state: State::BetweenWrites(writer),
        }
    }
}

enum IntermediateState<W> {
    _WriteDone(W),
    StartWrite(Sender),
    _Resolve,
}

impl<W> WriteQueue<W> {
    fn poll(&mut self) -> Result<(), ()> {
        loop {
            let next: IntermediateState<W> = match self.state {
                State::Writing(..) => {
                    return Err(())
                }
                State::BetweenWrites(ref mut _writer) => {
                    let front = self.pending.take();
                    match front {
                        Some(complete) => {
                            IntermediateState::StartWrite(complete)
                        }
                        None => diverge(),
                    }
                }
                State::Empty => diverge(),
            };

            match next {
                IntermediateState::_WriteDone(_w) => diverge(),
                IntermediateState::StartWrite(c) => {
                    let new_state = match ::std::mem::replace(&mut self.state, State::Empty) {
                        State::BetweenWrites(w) => {
                            State::Writing(w, c)
                        }
                        _ => diverge(),
                    };
                    self.state = new_state;
                }
                IntermediateState::_Resolve => {
                    match ::std::mem::replace(&mut self.state, State::Empty) {
                        State::BetweenWrites(_w) => {
                            return Ok(())
                        }
                        _ => diverge(),
                    }
                }
            }
        }
    }
}

fn diverge() -> ! {
    panic!()
}

pub struct Receiver {
    inner: Arc<Inner2>,
}

pub struct Sender {
    _inner: Arc<Inner2>,
}

struct Inner2 {
    complete: AtomicUsize,
}

pub fn channel() -> (Sender, Receiver) {
    let inner = Arc::new(Inner2 {
        complete: AtomicUsize::new(0),
    });
    let receiver = Receiver {
        inner: inner.clone(),
    };
    let sender = Sender {
        _inner: inner,
    };
    (sender, receiver)
}

impl Drop for Receiver {
    fn drop(&mut self) {
        self.inner.complete.store(1, SeqCst);
    }
}

The assertion in the destructor of the main function should never trip, but something's tripping it! This is an adapted test case from rust-lang/futures-rs#296, and although the original issue reproduces on stable/beta/nightly this test case only reproduces on nightly:

$ rustc +nightly -vV
rustc 1.15.0-nightly (8f02c429a 2016-12-15)
binary: rustc
commit-hash: 8f02c429ad3e2ad687a222d1daae2e04bb9bb876
commit-date: 2016-12-15
host: x86_64-unknown-linux-gnu
release: 1.15.0-nightly
LLVM version: 3.9
$ rustc +nightly ./crash.rs -O
$ ./crash
thread 'main' panicked at 'assertion failed: `(left == right)` (left: `1`, right: `0`)', ./crash.rs:12
note: Run with `RUST_BACKTRACE=1` for a backtrace.

[alexcrichton]

From my testing I found that the write in Drop for Receiver was the stray write that caused the assertion to trip, but unfortunately I didn't get much farther in debugging and there's other problems to fix now...

[alexcrichton]

cc @rust-lang/compiler

[eddyb]

struct Good(usize);
impl Drop for Good {
    #[inline(never)]
    fn drop(&mut self) {
        println!("dropping Good({})", self.0);
    }
}

struct Bad(usize);
impl Drop for Bad {
    #[inline(never)]
    fn drop(&mut self) {
        println!("dropping Bad({})", self.0);
    }
}

enum E { A(Bad), B(Good) }

fn main() {
    let mut go = true;
    
    loop {
        let next;
        match go {
            true => next = E::B(Good(123)),
            false => return,
        }

        match next {
            E::A(_) => return,
            E::B(_good) => go = false,
        }
    }
}

Prints, on stable, beta and nightly, with optimizations on:

dropping Good(123)
dropping Bad(123)

Looking at post-drop-elaboration MIR, there's a drop flag for (next as E::B).0 and one all of next.
Now the drop flag for next gets set on the assigment of next and doesn't get reset past the first iteration, but this would be fine because next's variant is checked, and since it's E::B, the (next as E::B).0 drop flag, which is correctly reset on the move to _good, prevents the Good from dropping.

This is why there's no bug with optimizations off. The problem with that though is the StorageDead statements, which translate to llvm.lifetime.end and instruct LLVM that the memory can be reused.
MIR is built such that next doesn't survive from one iteration to the next, and the fact that next is an E::B is lost by LLVM with optimizations on, making the drop flag logic UB and resulting in a Bad drop.

[nikomatsakis]

triage: P-high

@arielb1 will investigate, as he is the dynamic drop master :)