Integer handling is broken
Floating point can be tricky. You can’t really check for equality, and
with IEEE 754 you have a bunch of fun things like values of not a
number
, infinities, and positive and negative zero.
But integers are simple, right? Nope.
I’ll use “integers” to refer to all integer types. E.g. C’s int,
unsigned int, gid_t, size_t, ssize_t, unsigned long long
, and Java’s
int, Integer
, etc…
Let’s list some problems:
- You can’t parse unsigned integers in C/C++ with standard functions.
- Casting between types silently discards information (C, Java, Go, Rust, Haskell).
- Casting between some other types changes the semantic meaning of the number, even when technically no information is lost.
- The state of integers in Javascript is its own mess.
What’s wrong with casting?
Casting an integer from one type to another changes three things:
- The type in the language’s type system.
- Crops values that don’t fit.
- May change the semantic value, by changing sign.
The first is obvious, and is even safe for the language to do implicitly. Why even bother telling the human that a conversion was done?
But think about the other two for a minute. Is there any reason that you want your code to take one number, and because of a type system detail it’ll change it to another number, possibly even changing its sign?
Does your code fulfill its promise if it accepts one number but uses another?
Does this make any sense?
# strace -e setsockopt ping -Q 4294967298 -c 1 8.8.8.8
setsockopt(3, SOL_IP, IP_TOS, [2], 4) = 0 <-- 4294967298%2^32=2
setsockopt(4, SOL_IPV6, IPV6_TCLASS, [2], 4) = 0
setsockopt(5, SOL_IP, IP_TOS, [2], 4) = 0 <-- 4294967298%2^32=2
# strace -e setsockopt ping -F -4294945720 -c 1 ::1
[…]
setsockopt(4, SOL_IPV6, 0x20 /* IPV6_??? */, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\1\0\0TH\0\1\1\0\0\0\0\0\0\0\0\0", 32) = 0
^^
Flowlabel TH, eh? ------------/
These are negative numbers, and grossly out of range. They should not be accepted.
Basically most languages behave as if when the teacher says your answer is wrong, you reply that your answer was in a different number system, and declare victory.
Go tries to do it right, but kinda makes it worse
This is a compile time error:
var a int32
var b int8
a = b // cannot use b (variable of type int8) as type int32 in assignment
Sigh, ok. Why, though? It’s safe by definition. Especially since adding an explicit cast can cause errors in the future.
func foo() int8 {
return someNumber1
}
func bar() int32 {
return someNumber2
}
func baz(bool which) int16 {
if which {
// Bug? No, on closer inspection it's fine for now.
// But forcing us to use a cast means this will hide bugs
// if foo() ever changes.
//
//
// So basically this needless explicit cast is not so much a
// "cast" as it is "please suppress warnings".
return int16(foo())
}
return int16(bar()) // Bug: loss of data.
}
This could have been avoided if the cast were only needed when information risks being cropped or change semantic value. Then the programmer would know to be careful about casting integers, as any time they’d have to cast manually they’d likely be adding a latent bug.
Go forces the programmer to sweep the problem under the rug. Sprinkle some casts and pretend it’s all fine.
The value is still cropped. It’s still the wrong value.
An example of the wrong value
int count = 1;
while(…getopt()) {
switch (c) {
case 'c':
count = atoi(optarg);
break;
}
}
With 32bit int
on my amd64 Linux this is what I get:
Input | Output | What happened? |
---|---|---|
2^61 | 0 | Wrap :-( |
2^62 | 0 | Wrap :-( |
2^63-2 | -2 | Wrap :-( |
2^63-1 | -1 | Wrap :-( |
2^63 | -1 | Unspecified |
2^64 | -1 | Unspecified |
2^128 | -1 | Unspecified |
2^128+1 | -1 | Unspecified |
The manpage says the converted value or 0 on error
. Though it also
says atoi() does not detect errors
.
What actually seems to be happening is that it gets parsed as a 64bit signed integer, returning 0 on error (not counting trailing non-numbers as error), counting over/underflow as -1, and then cropping the result into a 32bit integer.
Like… what? How is that “convert a string to an integer”? Oh, let’s not get distracted into what I have already blogged about.
The problem here is actually that the implementation of atoi()
in
GNU libc is:
int
atoi (const char *nptr)
{
return (int) strtol (nptr, (char **) NULL, 10);
}
The problem isn’t actually the parsing. As I said strtol()
and strtoll()
are actually the only correct number parsers.
Sure, atoi()
doesn’t check for trailing characters. This is part of
its API, and a known gotcha.
But that harmless looking (int)
is the problem, here. It makes it so
that given what are clearly valid numbers as input, the returned value
will have three different semantic values.
Note here that atoi()
returns a 32bit integer.
Range | Semantics |
---|---|
-2^31 — +2^31-1 | Correct & obvious |
-2^63 — 2^31 or 2^31 — 2^63-1 | Truncated using modulus arithmetic |
<-2^63 or >2^63-1 | -1 |
(I left out cases where the input is not a valid number, since that’s a separate problem)
atoi()
has a hidden behavioral dependency on the size of long
!
The Linux manpage defines the implementation as being a direct call to
strtol()
, but POSIX only says:
The call atoi(str) shall be equivalent to:
(int) strtol(str, (char **)NULL, 10)
except that the handling of errors may differ. If the value cannot be
represented, the behavior is undefined.
RETURN VALUE
The atoi() function shall return the converted value if the value
can be represented.
Our friend “undefined behavior”. glibc could have chosen to
“error” (return 0
) when the number cannot fit into the return value
type. POSIX does allow for the error handling to be different.
To me the correct implementation of atoi()
must be:
int
atoi (const char *nptr)
{
const long l = strtol (nptr, (char **) NULL, 10);
if (l < INT_MIN) {
return 0;
}
if (l > INT_MAX) {
return 0;
}
return (int)l;
}
Of course this will give a warning if sizeof long == sizeof int
,
since the conditionals will in that case always be false.
atol()
is also broken, but both POSIX and the manpage recommend
against using it for these reasons. Annoyingly the manpage recommends
using strtoul()
, which is also broken.
Cast causes cropping
Seasoned programmers may simply dismiss this as “so what, that’s intended behavior”.
I’m arguing that it’s not.
257 is not 1. 128 is not -128. 2560384 is also not -128. They are different numbers. Changing the type should not change the semantic meaning.
If a user said “do this 2560384 times” then it’s not OK to do it -128 times.
Or if a database transaction says to add 2560384 tokens into a user’s credits, then it’s not OK to subtract 128 tokens.
If you did want cropping, and casting int to int8 for positive numbers, then you can always do:
return (int8_t)(some_int & 0x7f)
Se below for the general case.
Cast can cause sign change
package main
import (
"fmt"
"flag"
"log"
)
var (
num = flag.Int("num", -1, "Some number. Can't be negative")
)
func process(val int16) {
if val < 0 {
panic("Can't happen: We've already checked for negative")
}
fmt.Println(val)
}
func main() {
flag.Parse()
if *num < 0 {
log.Exitf("I said -num can't be negative")
}
process(int16(*num))
}
Let’s try it out
$ ./a -num=$(python3 -c 'print(2**32-1)')
panic: Can't happen: We've already checked for negative
goroutine 1 [running]:
main.process(0xe180?)
a.go:15 +0x74
main.main()
a.go:25 +0x8d
The parsing isn’t the problem. The cast is.
Even haskell is wrong
Prelude> read "9223372036854775808" :: Int
-9223372036854775808
Prelude> read "9223372036854775808" :: Integer
9223372036854775808
Prelude> let x = read "9223372036854775808" :: Integer
Prelude> fromIntegral x
9223372036854775808
Prelude> fromIntegral x :: Int
-9223372036854775808
So what should I do?
For GCC / Clang languages turn on -Wconversion -Wsign-conversion
, to
find the problems. Then, as with all warnings, fix all of them.
In C++
One example, that doesn’t log the reason the cast failed, is this:
template<typename To, typename From>
std::optional<To> cast_int(const From from)
{
// If casting from signed to unsigned then reject negative inputs.
if (std::is_signed_v<From> && std::is_unsigned_v<To> && from < 0) {
return {};
}
const To to = static_cast<To>(from);
// If casting from unsigned to signed then the result must be positive.
if (std::is_unsigned_v<From> && std::is_signed_v<To> && to < 0) {
return {};
}
// If the number fits then it'll be the same number when cast back.
if (from != static_cast<From>(to)) {
return {};
}
return to;
}
template<typename To, typename From>
To must_cast_int(const From from)
{
return cast_int<To>(from).value();
}
int main(int argc, char** argv)
{
// Ignore the parsing problems with atoi() for this example.
const int a = atoi(argv[1]);
const auto b = must_cast_int<int8_t>(a);
}
Or if you’re fine with Boost then numeric_cast
.
C
For C I guess you have to manually do that code generation, like this mkcast.py (URL subject to change when I merge with main branch).
But POSIX just says that gid_t
is an Integer type
. Signed or
unsigned? Unspecified. Great.
Although there are POSIX functions (e.g. setregid()
) that take a
gid_t
and are use -1
as a special value, implying that gid_t
is
signed.
Gah, but it also says that they must be positive arithmetic types
,
which presumably excludes signed!
On my Linux system it’s unsigned, FWIW.
Java
I’m still scarred from before, so leaving this as an exercise for the reader instead of going back into Java land.
Rust
Don’t use as
:
fn main() {
let mut foo:i32 = -123;
let mut bar:u32 = 0;
bar = foo as u32; // <-- don't do this
println!("Hello world! {}", bar);
}
$ ./r
Hello world! 4294967173
Instead do use try_from
, with .unwrap()
, .unwrap_or(defval)
, or
even expect("exception message")
:
use std::convert::TryFrom;
use std::env;
fn main() {
let args: Vec<String>= env::args().collect();
let foo:i32 = args[1].parse::<i32>().unwrap();
let bar:u32 = u32::try_from(foo).unwrap();
println!("Hello world! {}", bar);
}
A bit verbose, but ok.
Go
I don’t think the generics typesystem allows this, so you’ll probably have to write a code generator like with C.
Haskell
Disclaimer: I’m a haskell newbie. This may be terrible advice.
- Always parse as
Integer
- When casting to
Int
then do something like:
There are some other types, like in Numeric.Natural
, so maybe there
are better solutions.
import System.Environment
safe_cast_int from = safe_cast_int' from $ fromIntegral from :: Int
safe_cast_int' from to = do
case (fromIntegral to :: Integer) == from of
True -> to
False -> error $ "Did not return the same value: in=" ++ (show from) ++ " out=" ++ (show to)
main = do
args <- getArgs
let str = head args
putStrLn $ "Input string: " ++ str
let big = read str :: Integer
putStrLn $ "Integer: " ++ (show big)
putStrLn $ "Int: " ++ (show $ safe_cast_int big)
$ ghc cast.hs && ./cast 123
[1 of 1] Compiling Main ( cast.hs, cast.o )
Linking cast ...
Input string: 123
Integer: 123
Int: 123
$ ./cast $(python3 -c 'print(2**63)')
Input string: 9223372036854775808
Integer: 9223372036854775808
cast: Did not return the same value: in=9223372036854775808 out=-9223372036854775808
CallStack (from HasCallStack):
error, called at cast.hs:8:14 in main:Main
Python
Python is actually all good, thanks. int
will transparently expand
into a bigint, and you can’t cast to smaller integers, so no need for
special code.
Related
- https://blog.regehr.org/archives/1401 talks about overflows in computation. Not quite the same thing, but related and interesting.