News for hackers. Stuff that matters.
Re:The Git source code audit, viewed as a Rust programmer by throwaway81523 on Tuesday January 24, 2023 @11:13AM (#34505506)
This says again that the right behaviour for int overflow should be to trap, rather than wrap around or go into the weeds with UB.
Re:The Git source code audit, viewed as a Rust programmer by nextaccountic on Tuesday January 24, 2023 @11:41AM (#34506043)
I think the Rust approach of having different methods for wrapping and checked addition is excellent. If you have two i32, x.wrapping_add(y) [0] is a sum where you have wrapping semantics. x.saturating_add(y) [1] is a saturating sum (when it reaches the max it stays there instead of overflowing), nd x.checked_add(y) [2] is a sum that catches overflow.

If you are doing wrapping arithmetic a lot, there's a convenience wrapper (heh), Wrapping [3], so that if x and y have type Wrapping<i32>, then x + y always wraps (like calling wrapping_add on two i32). There's a Saturating<i32> too but it's nightly-only for now.

The only downside of Rust's approach is that if you have x + y on two plain integers, the language itself doesn't specify whether it will panic (and interrupt the execution of the program) or not. By default, debug builds will panic and release builds wraps (on the understanding that unintended overflows are always a bug, but catching this bug on release builds has some performance penalty, so it isn't caught by default on release), but you can control this by setting overflow-checks = true on the release profile on Cargo.toml [3]

I think future versions of Rust should set overflow-checks = true by default even on release builds, but right now it's only a matter of adding a single line on Cargo.toml to enable it, and almost every program should do it. With this on, x + y will panic on overflow if they are i32, will wrap if they are Wrapping<i32> and will saturate if they are Saturating<i32>

[0] https://doc.rust-lang.org/std/primitive.i32.html#method.wrap...

[1] https://doc.rust-lang.org/std/primitive.i32.html#method.satu...

[2] https://doc.rust-lang.org/std/primitive.i32.html#method.chec...

[3] https://doc.rust-lang.org/stable/std/num/struct.Wrapping.htm

[4] https://doc.rust-lang.org/cargo/reference/profiles.html#over...

Re:The Git source code audit, viewed as a Rust programmer by brundolf on Tuesday January 24, 2023 @12:25PM (#34506809)
It's off by default in production because all those overflow checks add up (hah). Rust's first priority has always been low-level/systems code, where these things matter. It (semi-controversially) makes runtime bounds checks despite that priority, because they're so important to memory safety, but with that guard-rail numeric overflows become less important- they can only cause logic bugs, not safety issues, so it makes sense to me that performance would then be prioritized by default
Re:The Git source code audit, viewed as a Rust programmer by stouset on Tuesday January 24, 2023 @12:48PM (#34507163)
Also, thanks to the language, most bounds checks can be elided in practice with idiomatic code. This isn’t as often the case with arithmetic operations.

I do wish there was a(n optional) clippy lint to warn against non-explicit integer arithmetic operations though.

Re:The Git source code audit, viewed as a Rust programmer by twic on Tuesday January 24, 2023 @01:12PM (#34507515)
What do they add up to? You can turn overflow checks on in release mode with -C overflow-checks=on, so it feels like it should be really easy to benchmark the current impact. I couldn't find any benchmarks doing this, though.
Re:The Git source code audit, viewed as a Rust programmer by brundolf on Tuesday January 24, 2023 @01:23PM (#34507699)
I'm sure it varies wildly by the type of program, and I'm sure much smarter people than me on the Rust team did their benchmarking homework when they originally made this choice. All I'm saying is that to me, it seems like a reasonable default
Re:The Git source code audit, viewed as a Rust programmer by throwaway81523 on Wednesday January 25, 2023 @12:11AM (#34514675)
It seems like a seriously wrong default for a language whose story is supposed to be safety. People wanting unsafe languages already have C and C++ for that :/.
Re:The Git source code audit, viewed as a Rust programmer by brundolf on Wednesday January 25, 2023 @02:29AM (#34515444)
Re:The Git source code audit, viewed as a Rust programmer by twic on Wednesday January 25, 2023 @09:44AM (#34518788)
To improve safety at large, Rust has to displace C++. People use C++ in part because of its speed (rightly or wrongly - but it is undeniably part of C++ culture). A language which is consistently slower than C++ will not displace C++. So, this was a point where the designers decided to hold their nose for now.
Re:The Git source code audit, viewed as a Rust programmer by dahfizz on Tuesday January 24, 2023 @03:52PM (#34510164)
I just did a very un-scientific benchmark to test this.

I chose ripgrep as a conservative choice, since most of the time should be I/O anyway.

I grepped for the word "Jesus" in a file that contained 100 copies of the king james bible (426MB of text). I ran it 100 times and averaged the results. Timed with /usr/bin/time.

--release without overflow checking was 0.0702 seconds on average.

--release with overflow checking was 0.0727 seconds, or about 3.5% slower

That's more than I expected for a program that's I/O bound.

Re:The Git source code audit, viewed as a Rust programmer by burntsushi on Tuesday January 24, 2023 @04:44PM (#34510915)
ripgrep author here.

> That's more than I expected for a program that's I/O bound.

Based on your description of your methodology and a reasonable assumption that you have more than ~426MB of available RAM, your benchmark is absolutely not I/O bound. Or at least, not bound by the speed of your disk. In your benchmark, your haystack is almost certainly entirely within memory and served from your operating system's page cache. It's unlikely you're hitting disk at all.

And even if you constructed your benchmark such that every search flushed your cache first and forced re-reading the file from disk, it may or may not be I/O bound. If your disk gives you 6 GB/s read speeds, you gotta do better than a traditional finite state machine to match that speed. (That is, probably SIMD.)

Re:The Git source code audit, viewed as a Rust programmer by dahfizz on Tuesday January 24, 2023 @09:28PM (#34513637)
Very true! I wouldn't expect any blocking IO here (except for the first run). I would still expect the majority of the time be spent reading from memory vs CPU bound "number crunching".

I should have been clearer. In my day job we are latency sensitive and we'll refer to excessively hitting main memory as being IO bound.

Re:The Git source code audit, viewed as a Rust programmer by burntsushi on Wednesday January 25, 2023 @05:14AM (#34516579)
Your query might be memory bound, but might not be. You've got to attain very high speeds to be memory bound. In your case, you chose a query that likely matches quite frequently, no? If so, it's certainly not going to be memory bound because of the overhead of dealing with all those matches.

Try a literal that doesn't match or matches very rarely and you'll have a better shot at being limited by memory bandwidth. (But still maybe not.)

Re:The Git source code audit, viewed as a Rust programmer by burntsushi on Tuesday January 24, 2023 @05:14PM (#34511326)
You might consider trying a harder regex for a benchmark like this. The problem with 'Jesus' is that it's a simple literal and is basically the best case for a tool like ripgrep. It won't even touch the regex engine and will instead stay in highly optimized SIMD code that is unlikely to be impacted by whether overflow checks are enabled or not. (Because it's already optimized to produce good codegen.)

Try something like '\w+\s+\w+' instead. That will force the regex engine to run and you might see more impact from overflow checks. (But I'm just guessing.)

Re:The Git source code audit, viewed as a Rust programmer by throwaway81523 on Wednesday January 25, 2023 @12:10AM (#34514670)
Can you say the difference in the binary program size?
Re:The Git source code audit, viewed as a Rust programmer by throwaway81523 on Wednesday January 25, 2023 @12:09AM (#34514664)
Of course the int overflows can cause safety issues. We're talking about Git CVE's regarding exactly that, after all. It's true that the int overflow checks incur overhead (has anyone measured it?). RISC-V missed the boat in not giving a hardware trap for it.
Re:The Git source code audit, viewed as a Rust programmer by brundolf on Wednesday January 25, 2023 @01:06AM (#34514947)
Indexing into an array or vec will always be checked in safe Rust, so if an overflow gives you a bad index you'll just get a panic (or an Option::None if you decided to access it that way); never a safety issue
Re:The Git source code audit, viewed as a Rust programmer by throwaway81523 on Wednesday January 25, 2023 @03:59AM (#34516058)
Indexes are only checked for being outside the array, not for being wrong. Getting a[3] when you wanted a[5] is a bug, potentially a serious or dangerous one. SPARK Ada goes to great lengths to statically verify that there are no int overflows in the code that it checks, and Ada itself specifies trapping overflow, though as an "optimization", the overflow check is often disabled. OCaml also checks overflow for its machine int type. GHC doesn't, and that seems like a mistake.
Re:The Git source code audit, viewed as a Rust programmer by brundolf on Wednesday January 25, 2023 @01:43PM (#34522393)
"Safety" means a specific thing here, and whatever bugs might happen with this, they do not make the code "unsafe"

It also helps that, in practice, when writing idiomatic Rust you'll almost always be accessing array/vec members using an iterator. Direct indexing is uncommon, which means indexing bugs specifically are very uncommon. Of course that's not the only bug that overflows can cause, but it's a big one.

Not every bug can be prevented by the language (at compile time or runtime). You have to make some choices about which things are just in the programmer's hands, and I think Rust has struck a reasonable balance. Especially since the user can always choose differently in this case, and just compile their code with `overflow-checks = true`

Re:The Git source code audit, viewed as a Rust programmer by jeroenhd on Tuesday January 24, 2023 @12:29PM (#34506886)
I agree from a language standpoint but trapping overflows by default would be quite costly. If you get the branch predictor involved with every addition or multiplication then you'll very quickly slow down your program, especially in the hot code paths. I think most general purpose programs don't run into overflow anymore these days, and the performance benefit probably outweighs the safety for most people.
Re:The Git source code audit, viewed as a Rust programmer by cerved on Tuesday January 24, 2023 @04:35PM (#34510794)
I think the opposite is true, most software projects benefit from safety and not performance
Re:The Git source code audit, viewed as a Rust programmer by throwaway81523 on Wednesday January 25, 2023 @04:06AM (#34516116)
Most programs should be written in HLL's with unbounded integers and garbage collection. There are a few programs that need machine arithmetic and manual memory management, for performance or whatever reasons, and Rust is intended to allow writing those programs safely. So it seems like a mistake to not treat int overflow as a serious error.

Idk if branch prediction or whatever would have to be involved. Most architectures will already trap on divide by zero or things like that, and will set a condition code for int overflow. Making the condition code sticky like it is for IEEE floating point arithmetic would allow doing a software trap once a basic block finished. It is a shame that the current trendy architecture (Risc-V) goes the opposite way and has no condition codes at all.

Re:The Git source code audit, viewed as a Rust programmer by mayoff on Tuesday January 24, 2023 @12:52PM (#34507238)
Swift does this. Bare operators like + and * trap (crash) on overflow. You can use &+ and &* for 2s complement wrapping.