search

pascalkuthe / imara-diff

Reliably performant diffing
Apache License 2.0
106 stars 9 forks source link

myers: avoid a Miri-flagged pointer invalidation #10

Closed jonathantanmy closed 2 months ago

jonathantanmy commented 2 months ago

This was my attempt at fixing this - feel free to rewrite to fit your project's coding style if you like.

Commit message follows:

In Myers::new, the kforward and kbackward pointers (derived from kvec) are invalidated by a subsequent kvec.into() (into a pointer). This can be seen by copying the relevant code into the Rust playground and running it under Miri [1]. To fix this, make kvec a pointer from the start.

[1] https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=554abe93817c3d95a3412006e8efb022

pascalkuthe commented 2 months ago

Great catch! I did run miri kn the tests a whole ago, I am surprised this didn't flag back then. Looking at the code this is quite obviously UB I think back when I originally wrote this code it want always clear to me when &mut create a pointer vs a mutable reference...

There are some clippy lint failures (see CI run). I would appreciate if you could fix these. Otherwise LGTM!

pascalkuthe commented 2 months ago

Ah sorry I didn't see the first part of your message (I am tired). I will just push a commit to fix the clippy messages myself tomorrow as you don't seem to mind. Thank you!

jonathantanmy commented 2 months ago

Thanks! Sorry for forgetting to run clippy.