Skip to content
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 handling of from_str methods is broken #7588

Closed
tweber12 opened this issue Jul 4, 2013 · 1 comment
Closed

Overflow handling of from_str methods is broken #7588

tweber12 opened this issue Jul 4, 2013 · 1 comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@tweber12
Copy link
Contributor

tweber12 commented Jul 4, 2013

The from_str methods of all integer types, signed and unsigned, may return Some() values in the case of an overflow. For example, between 256 and 1000, u8::from_str() returns None for less than half of the values and Some(nonsense) for the rest.

Examples:

u8::from_str("1000"); // => Some(232)
u16::from_str("100000"); // => Some(34464)
u32::from_str("10000000000"); // => Some(1410065408)
u64::from_str("30000000000000000000"); // => Some(11553255926290448384)

i8::from_str("600"); // => Some(88)
i16::from_str("80000"); // => Some(14464)
i32::from_str("5000000000"); // => Some(705032704)
i64::from_str("21000000000000000000"); // => Some(2553255926290448384)

Floats don't seem to have this problem, but are inconsistent in their behavior on overflow:

f32::from_str("1000000000000000000000000000000000000000"); // => Some(inf)
f32::from_str("10000000000000000000000000000000000000000"); // => None
f32::from_str("1e40"); // same number as above => Some(inf)

Why this happens:
from_str_bytes_common (from strconv.rs), which is used by all these methods, does the following:
It converts the string byte by byte and keeps an accumulator (of the type it converts to) with the value of the bytes already converted. After every byte it tries to detect overflows by checking if the accumulator got smaller.
The problem is that this only works if the value added to the accumulator is less than the maximum value its type can hold. In cases where the added value is bigger, the accumulator can overflow and still end up larger than before.

Example:

u8::from_str("1000");
// byte 1: acc = 1,   old = 0
// byte 2: acc = 10,  old = 1
// byte 3: acc = 100, old = 10
// byte 4: acc = 232, old = 100
@emberian
Copy link
Member

This still reproduces.

bors added a commit that referenced this issue Sep 7, 2013
Here's a fix for issue #7588, "Overflow handling of from_str methods is broken". 

The integer overflow issues are taken care of by checking to see if the multiply-by-radix-and-add-next-digit process is reversible. If it overflowed, then some information is lost and the process is irreversible, in which case, None is returned. 

Floats now consistently return Some(Inf) of Some(-Inf) on overflow thanks to a call to NumStrConv::inf() and NumStrConv::neg_inf() respectively when the overflow is detected (which yields a value of None in the case of ints and uints anyway). 

This is my first contribution to Rust, and my first time using the language in general, so any and all feedback is appreciated.
flip1995 pushed a commit to flip1995/rust that referenced this issue Oct 7, 2021
Re-write shadow lints

changelog: Move shadow_unrelated to restriction
changelog: The shadow lints find a lot more shadows and are not limited to certain patterns

Drastically simplifies the implementation. Catches a lot more cases.

I removed the "initialization happens here" note. It is not helpful IMO.

Closes rust-lang#318
Fixes rust-lang#2890
Fixes rust-lang#6563
Fixes rust-lang#7588
Fixes rust-lang#7620
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

3 participants