impossible borrows in l3_restore_reservoir

[icefoxen]

From boomshroom, reported here: https://www.reddit.com/r/rust/comments/9mioiv/porting_c_to_rust_a_case_study_minimp3/

I've run into a major problem in l3_restore_reservoir. While moving some functions into methods, I noticed one last unsafe I hadn't caught in mp3dec_decode_frame that was trying to subvert the borrow checker for for a Mp3DecScratch. You noticed there was a problem both by the fact that you had to use unsafe, and also the fact that you noticed something fishy with the lifetimes in the function signature.
What's going on here is that Mp3DecScratch contains a Bs, which itself contains a slice reference. In l3_restore_reservoir, you initialize the maindata array in Mp3DecScratch and then pass a reference to that to a new Bs which then gets stored in Mp3DecScratch. Long story short, Mp3DecScratch contains a reference to another field of itself.
This might not seem like much, but it's a huge deal in Rust for several reasons. For one, since maindata is being borrowed by the Bs, that spreads to the rest of the struct, making the entire struct immutable until it dies. Since l3_restore_reservoir takes it by a mutable reference, the compiler doesn't know that that field is no longer being modified and instead locks down the struct entirely.
There's also a separate issue in that normally, structs are allowed to be moved anywhere they please. You can move a value from the stack onto the heap and back again for instance. Since Mp3DecScratch contains a reference to one of its fields, a simple move like this would fail to update the reference and would instead point at the old location. The solution to this would unfortunately require heap allocation, so the array doesn't move with the struct, or the unstable Pin api, which allows you to specify that the struct isn't allowed to move at all. The big issue with that is that it requires the nightly compiler. If you want this to be available without std, then heap allocation isn't available at all without the alloc crate, which is also nightly exclusive.

[icefoxen]

So we have:

#[derive(Copy, Clone, Debug, Default)]
#[repr(C)]
pub struct Bs<'a> {
    pub buf: &'a [u8],
    pub pos: i32,
    pub limit: i32,
}


#[derive(Copy, Clone)]
#[repr(C)]
pub struct Mp3DecScratch<'a> {
    pub bs: Bs<'a>,
    pub maindata: [u8; 2815],
    pub gr_info: [L3GrInfo; 4],
    pub grbuf: [[f32; 576]; 2],
    pub scf: [f32; 40],
    pub syn: [[f32; 64]; 33],
    pub ist_pos: [[u8; 39]; 2],
}

And per the report, the slice in the Bs is initialized to point to maindata, which results in a self-borrowing struct, which is verboten.

I can think of a few potential solutions. For one, maindata is not actually used in very many places; perhaps we could just stuff it into the Bs struct and have an index instead of a slice.

It also looks like Bs is used almost exclusively to pass to get_bits(), there are very few places where buf is accessed directly outside that. So rearranging things might not be such a global change after all.

We're not gonna bother with nightly; I'd use heap allocation first.