Remove initialized-bytes tracking from BorrowedBuf and BorrowedCursor

[joshtriplett]

As discussed extensively in libs-api, the initialized-bytes tracking primarily benefits calls to read_buf that end up initializing the buffer and calling read, at the expense of calls to read_buf that don't need to initialize the buffer. Essentially, this optimizes for the past at the expense of the future. If people observe performance issues using read_buf (or something that calls it) with a given Read impl, they can fix those performance issues by implementing read_buf for that Read.

Update the documentation to stop talking about initialized-but-unfilled bytes.

Remove all functions that just deal with those bytes and their tracking, and remove usage of those methods.

Make BorrowedCursor::as_mut safe, as it's no longer a safety problem if the caller uninitializes bytes.

Remove BorrowedCursor::advance as there's no longer a safe case for advancing within initialized-but-unfilled bytes. Rename BorrowedCursor::advance_unchecked to advance.

Update tests.

r? @Amanieu

[rust-log-analyzer]

The job pr-check-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[zachs18]

As-is, this is unsound with the combination of the existing From<&'a mut [u8]> for BorrowedBuf<'a> and the newly-safe BorrowedCursor::as_mut, as you can do buf.unfilled().as_mut()[0] = MaybeUninit::uninit(); to write uninit into a borrowed &mut [u8] in safe code.

reproducer with Miri output

#![feature(core_io_borrowed_buf)]
#![forbid(unsafe_code)]
use core::io::BorrowedBuf;
use std::mem::MaybeUninit;

fn main() {
    let mut bytes = [0; 32];
    let mut buf = BorrowedBuf::from(&mut bytes[..]);
    buf.unfilled().as_mut()[1] = MaybeUninit::uninit();
    dbg!(bytes);
}

$ cargo +dev-compiler-2-stage1 miri run
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.00s
     Running `/home/zachary/opt_mount/zachary/Programming/rust-compiler-2/build/x86_64-unknown-linux-gnu/stage1/bin/cargo-miri runner /home/zachary/opt_mount/zachary/cargo-target/miri/x86_64-unknown-linux-gnu/debug/borrowedbuf-thing`
error: Undefined Behavior: constructing invalid value at [1]: encountered uninitialized memory, but expected an integer
  --> src/main.rs:10:5
   |
10 |     dbg!(bytes);
   |     ^^^^^^^^^^^ Undefined Behavior occurred here
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: this error originates in the macro `dbg` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

---

If we allow making a BorrowedBuf from both a &mut [u8] and &mut [MaybeUninit<u8>], then I think BorrowedBuf either needs to remember the difference (by keeping track of filled-length and initialized-length separately), or we need to make there not be a difference (e.g. by not allowing users to ever write uninitilized data into a BorrowedBuf soundly, e.g. leaving BorrowedCursor::as_mut as unsafe, or replacing it with as_mut_ptr -> *mut u8 (with a doc comment saying not to de-initialize) or removing it or something).

[Amanieu]

@zachs18 Nice catch: you are correct that we need to ensure that we never de-initialize the buffer, which means that we can't expose unfilled. I think it should just be removed and that we should instead have a method which returns a raw pointer to the underlying data (for use with system calls). This leaves only 2 ways of filling the buffer:

• Using append, which only initializes and never de-initializes.
• Using a raw pointer and advance, in which case the user must ensure they never de-initialize any part of the buffer.