iximeow / yaxpeax-x86

x86 decoders for the yaxpeax project
BSD Zero Clause License
129 stars 23 forks source link

significantly improve instruction printing efficiency #34

Closed iximeow closed 2 months ago

iximeow commented 2 months ago

this is where much of https://github.com/iximeow/yaxpeax-arch/pull/7 originated.

std::fmt as a primary writing mechanism has.. some limitations:

and some more interesting more fundamental limitations - writing to a T: fmt::Write means implementations don't know if it's possible to write bytes in reverse order (useful for printing digits) or if it's OK to write too many bytes and then only advance len by the correct amount (useful for copying variable-length-but-short strings like register names). these are both perfectly fine to a String or Vec, less fine to do to a file descriptor like stdout.

at the same time, Colorize and traits depending on it are very broken, for reasons described in yaxpeax-arch.

so, this adapts yaxpeax-x86 to use the new DisplaySink type for writing, with optimizations where appropriate and output spans for certain kinds of tokens - registers, integers, opcodes, etc. it's not a perfect replacement for Colorize-to-ANSI-supporting-outputs but it's more flexible and i think can be made right.

along the way this completes the move of safer_unchecked out to yaxpeax-arch (ty @5225225 it's still so useful), cleans up some docs, and comes with a few new test cases.

because of the major version bump of yaxpeax-arch, and because this removes most functionality of the Colorize impl - it prints the correct words, just without coloring - this is itself a major version bump to 2.0.0. yay! this in turn is a good point to change the Opcode enums from being tuple-like to struct-like, and i've done so in https://github.com/iximeow/yaxpeax-x86/commit/1b8019d5b39a05c109399b8628a1082bfec79755.

full notes in CHANGELOG ofc. this is notes for myself when i'm trying to remember any of this in two years :)

5225225 commented 2 months ago

Actually, I believe safer_unchecked is not needed anymore, this should be checked by default in debug builds nowadays.

fn main() {
    let slice = &[1];
    unsafe {
        let _ = slice.get_unchecked(2);
    }
}
   Compiling playground v0.0.1 (/playground)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.47s
     Running `target/debug/playground`
thread 'main' panicked at library/core/src/panicking.rs:220:5:
unsafe precondition(s) violated: slice::get_unchecked requires that the index is within the slice
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread caused non-unwinding panic. aborting.

see: https://github.com/rust-lang/rust/pull/120594 (before that, the checks existed, but were optimized out unless you rebuilt the stdlib since the std is built in release mode)

The checking in the std is also a lot more complete than just some bounds checks. Still no replacement for miri, but tells you when you're doing something blatantly wrong without a huge compatibility/perf hit.

iximeow commented 2 months ago

oh! well that's great. serves me right for using an old rust for testing