rust-osdev / ucs2-rs

UCS-2 conversion utilities for Rust.
Mozilla Public License 2.0
6 stars 5 forks source link

Technically possibly reachable panic(s) #2

Closed izik1 closed 5 years ago

izik1 commented 5 years ago

The conditions for returning Err(Error::BufferUnderflow), which are responsible to the panics getting in compiled are arguably unreachable to the point that core::hint::unreachable_unchecked should be used instead of return Err(Error::BufferUnderflow), however, under the assumption that they are somehow reachable...

https://github.com/GabrielMajeri/ucs2-rs/blob/master/src/lib.rs#L77-L79

If len == i + 1 this branch won't be taken, hence llvm inserting a panic for the i + 1 array access.

https://github.com/GabrielMajeri/ucs2-rs/blob/master/src/lib.rs#L63-L66

LLVM inserts a panic for the i + 1 array access due to the use of == instead of >=, not sure why, but it does, presumably llvm can't quite prove that that is okay. (which it definitely is)

convenient link to compiler explorer with before/after removing the first mentioned panic: https://rust.godbolt.org/z/25yb-K

I once again believe these panics to be absolutely unreachable, along with the code paths for return Err(Error::BufferUnderflow) and even return Err(Error::InvalidData), because it is UB to have a &str that isn't valid UTF-8, also see playground for a reference that Rust considers a string with an incomplete character "invalid"