20 ms·
This is is not a knock against SeaHash, but I was looking at buffer.rs [0] and noticed pretty much all the code is wrapped in unsafe {} blocks. How much advanta
by aleyan 10y ago
This is is not a knock against SeaHash, but I was looking at buffer.rs [0] and noticed pretty much all the code is wrapped in unsafe {} blocks. How much advantage is there to rust implementation vs c++ if unsafe is used so liberally? I ask this in ernest.
[0] https://docs.rs/crate/seahash/2.0.0/source/src/buffer.rs https://docs.rs/crate/seahash/2.0.0/source/src/buffer.rs
- fnord123 10y agoNone. But once you're out of the buffer code, it's safe.
- michaelmior 10y agoIt's safe as long as the unsafe code didn't mess with pointers in unexpected ways. Your unsafe code still needs to behave in a sane way for any safety guarantees to hold.
- fnord123 10y agoSure. And if there's a problem, the search for the bug should be limited to the unsafe blocks. Which is the promised benefit over C++.
- Animats 10y agoNo. As soon as there's unsafe code, there's the possibility of safe code misusing the unsafe code to create unsafe behavior. One would like to have un-abusable unsafe code, but the language does nothing to guarantee that.
- pcwalton 10y agoThe parent post is correct. Unsafe code must uphold the invariants of safe Rust. If unsafe code is safe only if the caller upholds some invariants not enforced by the compiler, then by definition it's the unsafe code that is wrong.
- Animats 10y agoUnsafe code must uphold the invariants of safe Rust. Ideally, yes. In practice, maybe. We're probably going to see "unsafe" code that assumes good behavior on the part of the caller. That's a classic problem with APIs.
- pcwalton 10y agoThere's no way to solve that problem without just forbidding unsafe code entirely. Unsafe code can have bugs; that's why you should keep it to the minimum and keep it well-known and audited. In this case, the byteorder crate would have been more appropriate than handrolling unsafe code.
- Animats 10y agoThere's no way to solve that problem without just forbidding unsafe code entirely. That's not at all clear. It's worth looking at unsafe code and asking "why was this necessary"? What couldn't you do within the language? As patterns reoccur, it may become clear what new safe primitives are needed.
- pcwalton 10y agoWhy is it better to add safe primitives directly to the compiler rather than implementing them in libraries?
- Animats 10y agoThe compiler can look at more data to decide if something is valid, or can be optimized. C++ is trying to add move semantics via templates, but can't get all the way to Rust's borrow checker that way.
- Manishearth 10y agoRight, but in this case it doesn't need to. I have yet to see an example of an operation that: - should be safe in Rust but isn't - needs /compiler/ support to work well (can't be done cleanly as a library) - isn't already on the track for implementation (non-lexical lifetimes, SEME regions) You did mention uninitialized arrays but uninitialized data is inherently unsafe. It's not an operation that can be made safe. Instead, you make it safe by encoding the invariants specific to your use case in your code and creating a safe wrapper -- these invariants differ by use case, so it can't be made a generic operation.
- staticassertion 10y agoThe unsafety still originates in the unsafe code, thus you can audit the use of the unsafe code or the unsafe code itself just the same.
- Manishearth 10y ago> there's the possibility of safe code misusing the unsafe code to create unsafe behavior. This is a misconception. You can write un-abusable unsafe code pretty easily. There are a couple of invariants that the language requires you to uphold. As long as you uphold them you are fine. If your unsafe code can be broken by external safe code that is a bug in your unsafe code, on par with doing `let p = ptr::null(); print(*p);` in your unsafe code.
- foota 10y agoIt seems like a small kernel of unsafe code, while the rest of the library is likely written safe. Perhaps more of this could be written safe?
- idsout 10y agoBetter integration within the Rust library ecosystem. Instead of having to install a C/C++ library through my package manager, I can just add the seahash dependency to my Cargo.toml file and cargo will handle the rest.
- pcwalton 10y agoBecause the unsafe code is isolated into an easily-auditable portion that basically just exists to perform word-aligned reads of a byte buffer. It might be nice to factor this out into a separate library, but it's fairly harmless.
- pcwalton 10y agoAddendum: There is a separate library for this, of course: byteorder. Can't believe I forgot about it :)
- reklawnos 10y agoAs others have mentioned, Rust allows you to write a safe wrapper around unsafe code. In this case, all of the functions implemented in buffer.rs are safe, even if their contents are not, so they can be used without having to worry about unsafety. Another advantage over C++ is that it's much more clear what code is safe and what isn't, as unsafe code is wrapped in `unsafe` blocks, as you mentioned.
- killercup 10y ago> How much advantage is there to rust implementation vs c++ if unsafe is used so liberally? I'm no expert on the topic. IIRC, Rust does not have the same kind/amount of unspecified behavior as C/C++ do. FYI: This is a "highly optimized version of SeaHash" (see comment in first line) with "optimized" meaning fiddling with raw pointers. You can find a version without any unsafe code is in `reference.rs`. I have no idea how fast the reference implementation is.
- wyldfire 10y ago> How much advantage is there to rust implementation vs c++ if unsafe is used so liberally? I think this code could potentially be refactored with smaller unsafe blocks, if that were a goal. The benefit in general is present for many reasons, among which is that you still have to opt-in to unsafe{} and SeaHash consumers wouldn't need to in order to leverage these features. The benefit for SeaHash specifically is that Rust isn't merely a safer language, it's also one with arguably newer/better language features than C++. And it's one that has support for several targets today.
- dbaupp 10y agoThat code looks like it could (and should, IMO) be refactored to put the unsafe code in more contained locations, e.g. the &[u8] could be manipulated into a (&[u64], &[u8]) pair (with the main sequence of values, and the trailing ones). Rust unfortunately can't stop people writing code that makes life hard for themselves.
- Animats 10y agoThis code is C code written as unsafe Rust: let mut ptr = buf.as_ptr(); let end_ptr = buf.as_ptr().offset(buf.len() as isize & !0x1F) as usize; while end_ptr > ptr as usize { a = a ^ read_u64(ptr); ptr = ptr.offset(8); b = b ^ read_u64(ptr); ptr = ptr.offset(8); c = c ^ read_u64(ptr); ptr = ptr.offset(8); d = d ^ read_u64(ptr); ptr = ptr.offset(8); .... match excessive { 0 => {}, 1...7 => { a = a ^ read_int(slice::from_raw_parts(ptr as *const u8, excessive)); a = diffuse(a); }, 8 => { a = a ^ read_u64(ptr); a = diffuse(a); }, 9...15 => { a = a ^ read_u64(ptr); ptr = ptr.offset(8); excessive = excessive - 8; .... This bothers me about Rust. There's too much "unsafe" code in libraries. The language is unable to express some essential concepts. Known areas of trouble include partial initialization of an array, needed to implement growable collections, and single ownership doubly linked lists. Neither of those is expressible within Rust, which leads to unsafe code to implement them. Here, though, it's purely a performance issue. That's disturbing. If you can't do fast big-banging in safe Rust, there's a problem somewhere. If Rust let you access a slice of bytes as an slice of ints, alignment and length permitting, the code above could be much more straightforward. That's what I mean about expressive power. The hack to do that used here: let end_ptr = buf.as_ptr().offset(buf.len() as isize & !0x1F) as usize; is iffy. Why is there an "isize" (a signed quantity) in there? They want to align with a 32-bit cache line, yes, but why the signed quantity? The documentation for Rust's "std::ops::BitAnd" doesn't say what the semantics are for signed numbers. What would happen on a 32-bit machine if someone allocated a buffer bigger than 2GB? Exploitable?
- pcwalton 10y agoWhy would hard-coding the ability to access a slice of bytes as ints into the compiler be safer than a well-encapsulated unsafe code abstraction? We used to implement things like vectors directly in the compiler, but it was a big headache for no gain. Writing actual code is way easier than writing code to generate LLVM IR. Anyway, there is a commonly-used crate for this: byteorder. Had I written the library, I would have just used that crate. But it's not a big deal either way. > Why is there an "isize" (a signed quantity) in there? Because pointer arithmetic is signed. > The documentation for Rust's "std::ops::BitAnd" doesn't say what the semantics are for signed numbers Bitwise operations on signed integers are applied to their twos complement representations. > What would happen on a 32-bit machine if someone allocated a buffer bigger than 2GB? It would work. > Exploitable? No.