1.17.0 regression: loop mutable vs immutable alias.

[dpc]

https://github.com/dpc/mioco/blob/0a12aae802256468f30a43cde2689e8846176914/examples/http-server.rs#L47

This code compiles on 1.16.0, but errors on 1.17.0:

error[E0502]: cannot borrow `buf` as mutable because it is also borrowed as immutable
  --> examples/http-server.rs:49:63
   |
49 |                                 let len = try!(conn.read(&mut buf[buf_i..]));
   |                                                               ^^^ mutable borrow occurs here
...
58 |                                 let res = req.parse(&buf[0..buf_i]).unwrap();
   |                                                      --- immutable borrow occurs here
...
77 |                         });
   |                         - immutable borrow ends here

Reproduced and confirmed by another person on #rust. I'm working on minimizing, but since it's late, I might not be able to deliver today.

[kennytm]

Reduced test case:

fn parse<'b>(_r: *mut [&'b u8], _b: &'b u8) {}

fn main() {
    let mut buf = 0u8;
    let mut headers = [];
    loop {
        {&mut buf;}
        parse(&mut headers, &buf);
    }
}

Changing the type of _r from *mut [&'b u8] to *mut [&'b u8; 0] causes 1.16.0 to emit error also. Is it possible this is in fact a soundness fix?

[dpc]

Where is the * coming from? Is &mut self really *mut Self?

[alexcrichton]

cc @rust-lang/compiler

[djzin]

Looks like it could be related to #40319

[eddyb]

Changing the type of _r from *mut [&'b u8] to *mut [&'b u8; 0] causes 1.16.0 to emit error also. Is it possible this is in fact a soundness fix?

Yupp! This is #40319, which fixed the slice case - it had accidentally been allowed before.

[djzin]

Pretty sure it is that - this compiles and runs on rust 1.16, and is clearly unsound

fn parse<'b>(r: &mut [&'b u8], b: &'b u8) {
    r[0] = b;
}

fn main() {
    let mut buf = 0u8;
    let mut headers = [&0];
    parse(&mut headers, &buf);
    let immutable: [&u8; 1] = headers;
    assert_eq!(*immutable[0], 0);
    buf = 1;
    assert_eq!(*immutable[0], 1);
}

[eddyb]

What's with the 1.17.1 in the title, btw? I can edit it but maybe it's intentional.

[dpc]

Sorry. Just a typo.

Anyway, I don't get it. res (line 58) holds immutable reference that is dropped at the end of loop scope, so why can't it be used on read (line 49)? Thanks for your help anyway!

[eddyb]

@dpc Well, the bug was found by a user of httparse, see seanmonstar/httparse#34.
I believe the explanation is that headers will actually contain pointers into buf.

[dpc]

Oh, so the lifetime 'a is spanning furhter than the loop itself... I see... Thanks.

[dpc]

So I guess all is well. Closing. :)