rvalue destructors inside of a bare block run at the wrong time

[sfackler]

** UPDATE by @nikomatsakis **

This is almost-but-not-quite a dup of #8861, see this comment for details.

** ORIGINAL POST **

This program:

struct Foo { a: int }

impl Drop for Foo {
    fn drop(&mut self) {
        println!("{}", self.a);
    }
}

fn main() {
    {
        let _1 = Foo { a: 1 };
        let _2 = Foo { a: 2 };
        match Foo { a: 3 } {
            _ => {}
        }
    }
    let _4 = Foo { a: 4 };
}

should output

3
2
1
4

but instead outputs

2
1
3
4

It appears that if the bare block around the first part of main is removed, everything runs in the correct order. Moving the "3" Foo into a variable also makes everything run in the right order.

This was originally reported to me in sfackler/rust-postgres#31 as a segfault, but it looks like that's probably just due to heap corruption from a use-after-free caused by this bug.

Update

IR:

join:                                             ; preds = %case_body
  call void @_ZN3Foo14glue_drop.146917h02f639dd107d06daE(%struct.Foo* %_2)
  call void @_ZN3Foo14glue_drop.146917h02f639dd107d06daE(%struct.Foo* %_1)
  call void @_ZN3Foo14glue_drop.146917h02f639dd107d06daE(%struct.Foo* %0) ; <-- 3
  %7 = getelementptr inbounds %struct.Foo* %_4, i32 0, i32 1
  store i8 1, i8* %7
  %8 = getelementptr inbounds %struct.Foo* %_4, i32 0, i32 0
  store i64 4, i64* %8
  call void @_ZN3Foo14glue_drop.146917h02f639dd107d06daE(%struct.Foo* %_4)
  ret void

[flaper87]

This sounds bad. Nominating

EDIT: Looks like something may be wrong in rustc::middle::region::resolve_expr for ExprMatch.

[flaper87]

cc @nikomatsakis

[sfackler]

This also pops up in for loops (see the original issue on rust-postgres), so I don't believe it's an issue exclusively with ExprMatch. Nevermind, I forgot that the for sugar wraps in a single branch match.

[nikomatsakis]

This is actually the expected behavior (if you fully grok the rules), but it's an interesting side-effect.

[nikomatsakis]

@sfackler I'd like to know more about the details of the use-after-free.

[nikomatsakis]

The reason this occurs is that:

1. The match introduces a temporary.
2. The match occurs as the tail expression in a block.
3. The lifetime of temporaries in the tail expression of a block is equal to the innermost enclosing statement -- in this case, that's the outer block.

[sfackler]

@nikomatsakis In the example in sfackler/rust-postgres#31, the iterator has a reference to the statement, which in turn has a reference to the connection. The destructor for the iterator uses the connection to clean up the associated state on the server. But, since the connection has already been dropped, it ends up writing to a freed buffer, and then tries to send on a closed socket.

[sfackler]

If the destructor order is working as intended, it seems like the bug may actually be in the lifetime verification infrastructure, since the lifetime of the iterator type should not be allowed to outlive the statement, and transitively the connection.

[nikomatsakis]

That's what I'm trying to understand. If there is an unsound interaction here. I can imagine where something confusing might crop up, I have to read and better understand the example.

[sfackler]

Here's a very simple version of the types used in that example:

struct Connection {
    conn: BufferedStream<TcpStream>
}

struct Statement<'conn> {
    conn: &'conn Connection
}

struct Rows<'stmt> {
    stmt: &'stmt Statement<'stmt>
}

impl<'stmt> Drop for Rows<'stmt> {
    fn drop(&mut self) {
        self.stmt.conn.conn.write("drop my server side state please".as_bytes());
        self.stmt.conn.conn.flush();
    }
}