-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Overflow in libstd/io/cursor.rs "seek" returns incorrect error with large positions #39631
Comments
Thanks for the report! I'd definitely believe that the exact implementation here is slightly wrong and could use some tweaking. |
I wrote a test that should fail and an implementation that should fix, just
waiting for build & tests to finish on my machine before PR.
Edit: PR #39874
|
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this issue
Feb 16, 2017
Fixes overflow in libsdt/io/cursor.rs "seek" Fixes rust-lang#39631 Test which fails (with old implementation), then fix to implementation.
frewsxcv
added a commit
to frewsxcv/rust
that referenced
this issue
Feb 17, 2017
Fixes overflow in libsdt/io/cursor.rs "seek" Fixes rust-lang#39631 Test which fails (with old implementation), then fix to implementation.
frewsxcv
added a commit
to frewsxcv/rust
that referenced
this issue
Feb 17, 2017
Fixes overflow in libsdt/io/cursor.rs "seek" Fixes rust-lang#39631 Test which fails (with old implementation), then fix to implementation.
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this issue
Feb 17, 2017
Fixes overflow in libsdt/io/cursor.rs "seek" Fixes rust-lang#39631 Test which fails (with old implementation), then fix to implementation.
bors
added a commit
that referenced
this issue
Feb 21, 2017
Fixes overflow in libsdt/io/cursor.rs "seek" Fixes #39631 Test which fails (with old implementation), then fix to implementation.
amosonn
changed the title
Overflow in libsdt/io/cursor.rs "seek" returns incorrect error with large positions
Overflow in libstd/io/cursor.rs "seek" returns incorrect error with large positions
Feb 10, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Seems to be a reroll #20678. I have some idea how to fix it, will try to draft a pull request later, but it basically requires checking the sign of the
i64
,neg
ging if required and casting tou64
, and usingchecked_add
orchecked_sub
.I will divide into cases to demonstrate that this is required, to save repeating phrases, I will use the following notation:
S means
pos < i64::max_value()
, L meanspos > i64::max_value()
;P means
n >= 0
, N meansn < 0
;O+ means we will notice bads by checking OF on add (
u64::checked_add
), O- means we will notice them by checking OF on sub (u64::checked_sub
afteri64::wrapping_neg
), Z+ means we will notice by adding asi64
and making sure result is non-negative, Z- is making sure it is negative, D+ mean we will notice bads by asserting OF is set on add (u64::overflowing_add
), D- is likewise for sub (u64::overflowing_sub
). Note the we don't need to considern == i64::min_value()
, because afteri64::wrapping_neg
andas u64
we still get the right, large, positive number.So, we have the following cases and appropriate checks that spot trouble:
Overall, we see that there is no one test which works for all cases, and a minimum of two cases (and tests) yields the solution described above.
The text was updated successfully, but these errors were encountered: