5 ms·

Sure. 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 ago
No. 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 ago
  The 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 ago
    Unsafe 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 ago
      There'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 ago
        There'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 ago
        Why is it better to add safe primitives directly to the compiler rather than implementing them in libraries?
        Animats 10y ago
        The 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 ago
        Right, 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.
        Animats 10y ago
        You did mention uninitialized arrays but uninitialized data is inherently unsafe. It's not an operation that can be made safe. Sure it can. You just need primitives which can be used in asserts such as is_initialized(tab,i,j) indicating that an array is initialized within those limits. Then you can write asserts such as assert(is_initialized(tab,i,j-1)); ... initialize tab[j] assert(is_initialized(tab,i,j)); Standard program verification technology. Verification of unsafe sections is a useful goal, and deserves language support. Hand-waving about "encoding the invariants specific to your use case in your code" is insufficient. You need to write them down and prove them. Then you can eliminate them from the run-time code.
      - wahern 10y ago
        I think the issue, if there is one, is that arguably the more appropriate thing to do would have been to implement the code without unsafe, using an intrinsically correct algorithm. The type-punning is premature optimization. _That_ was the misstep. That the byteorder crate exists is irrelevant in as much as this was an example of the urge for premature optimization leading the developer down the wrong path. The same amount of time pondering whether to even bother using a pre-existing library might have been better spent second-guessing the urge to type-pun at all. Also, looking at the byteorder crate, I wouldn't be surprised if it's even slower than the simpler and correct loop I posted elsethread. read_num_bytes in that create uses copy_nonoverlapping, which I assume is analogous to memcpy in C. That's a very round-a-bout and inefficient way to accomplish the task, and likely patterned after similarly bad C code. To even make it worthwhile, any byteorder library should provide some kind of iterator interface so that it can maintain alignment state while permitting the loop to be unrolled by the compiler. (And it might require a closure or someway of expanding a block of code inline.) That's probably the only way it could outperform the simple, hand-rolled, endianness- and alignment-neutral solution. But it doesn't provide that kind of interface AFAICT. It's all sort of ironic, which I suppose was the point upthread--this is an example of the irrational urge for premature optimization and of bad programming idioms being hauled into Rust land completely unhindered by Rust's type safety features. And the better, correct, and likely more performant way of accomplishing this task could have been done just as safely from C as it could from Rust.
        burntsushi 10y ago
        > Also, looking at the byteorder crate, I wouldn't be surprised if it's even slower than the simpler and correct loop I posted elsethread. read_num_bytes in that create uses copy_nonoverlapping, which I assume is analogous to memcpy in C. That's a very round-a-bout and inefficient way to accomplish the task, and likely patterned after similarly bad C code. It wasn't patterned after any C code. ptr::copy_nonoverlapping doesn't necessarily compile down to memcpy. Namely, concrete sizes are given, so the compiler backend can optimize this down to simple loads and stores on x86, which is probably going to do better than the bit-shifting approach. Namely, loading a little-endian encoded integer on a little-endian architecture should be as simple as a single word-sized load (because the byte swap is unnecessary). It would be interesting to consider whether the safer and more readable bit-shifting approach could be compiled down to the same code, but when I wrote the byteorder crate, this wasn't the case. This isn't the only place that ptr::copy_nonoverlapping is useful. I used it in my snappy[1] implementation as well, specifically to avoid the overhead of memcpy. To be clear, this wasn't my idea. This is what the C++ Snappy reference implementation does as well. Avoiding memcpys in favor of unaligned loads/stores is a dramatic win. I know this because I tried to write my Snappy implementation without specific unaligned loads/stores, and it performed quite a bit worse. The performance of the Rust implementation is now on par with the C++ implementation. Of course, this is always dealing with raw bytes---there's no type punning here. ptr::copy_nonoverlapping is a bit generic for this use case. That's why we recently accepted an RFC to add read_unaligned/write_unaligned to the standard library[2]. (Which are implemented via straight-forward calls to ptr::copy_nonoverlapping.) [1] - https://github.com/BurntSushi/rust-snappy/blob/master/src/decompress.rs https://github.com/BurntSushi/rust-snappy/blob/master/src/de... [2] - https://github.com/rust-lang/rfcs/blob/master/text/1725-unaligned-access.md https://github.com/rust-lang/rfcs/blob/master/text/1725-unal...
    - Manishearth 10y ago
      > We're probably going to see "unsafe" code that assumes good behavior on the part of the caller. I have yet to see any of this. I have noticed that it's harder to write correct unsafe code when it comes to parallelism and FFI, but parallelism has always been a hard problem and the FFI problems generally come from the fact that you need to know the invariants being upheld on the other end, which is trickier. But for this kind of unsafe code -- designing (non-parallel) abstractions -- upholding invariants is pretty straightforward.
      - Gankro 10y ago
        > I have yet to see any of this. mem::forget-pocalypse was this. (Rc/Arc, Vec::drain, thread::scoped) Any UB bug that results from an overflow is kind've implicitly this. BTreeMap::range still has an UB bug from trusting the caller! I literally asked you to fix it! https://github.com/rust-lang/rust/issues/33197 https://github.com/rust-lang/rust/issues/33197 Bugs happen man.
        Manishearth 10y ago
        > mem::forget-pocalypse was this. I would say that this is from a time when the invariants were not understood. In particular, the fact that leaking is safe to do in safe code was not known. (The invariants are still not completely understood, but there's work to specify that, and IMO they're understood enough to be able to avoid unsafe bugs) > BTreeMap::range still has an UB bug from trusting the caller! Fair :) I'd completely forgotten about that one.
        Gankro 10y ago
        > I would say that this is from a time when the invariants were not understood. Yeah, but it's not like "oh this is an obvious thing to consider trusting the caller about". It's an exceptionally niche problem that you'd only know about if someone told you about it. Especially since a Rust programmer shouldn't be expected to write unsafe code often, if ever! Similarly: not trusting traits to be implemented correctly. Not trusting closures to not-unwind.
- staticassertion 10y ago
  The 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.

Animats 10y ago

No. 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 ago
The 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 ago
  Unsafe 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 ago
    There'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 ago
      There'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 ago
        Why is it better to add safe primitives directly to the compiler rather than implementing them in libraries?
        Animats 10y ago
        The 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 ago
        Right, 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.
        Animats 10y ago
        You did mention uninitialized arrays but uninitialized data is inherently unsafe. It's not an operation that can be made safe. Sure it can. You just need primitives which can be used in asserts such as is_initialized(tab,i,j) indicating that an array is initialized within those limits. Then you can write asserts such as assert(is_initialized(tab,i,j-1)); ... initialize tab[j] assert(is_initialized(tab,i,j)); Standard program verification technology. Verification of unsafe sections is a useful goal, and deserves language support. Hand-waving about "encoding the invariants specific to your use case in your code" is insufficient. You need to write them down and prove them. Then you can eliminate them from the run-time code.
    - wahern 10y ago
      I think the issue, if there is one, is that arguably the more appropriate thing to do would have been to implement the code without unsafe, using an intrinsically correct algorithm. The type-punning is premature optimization. _That_ was the misstep. That the byteorder crate exists is irrelevant in as much as this was an example of the urge for premature optimization leading the developer down the wrong path. The same amount of time pondering whether to even bother using a pre-existing library might have been better spent second-guessing the urge to type-pun at all. Also, looking at the byteorder crate, I wouldn't be surprised if it's even slower than the simpler and correct loop I posted elsethread. read_num_bytes in that create uses copy_nonoverlapping, which I assume is analogous to memcpy in C. That's a very round-a-bout and inefficient way to accomplish the task, and likely patterned after similarly bad C code. To even make it worthwhile, any byteorder library should provide some kind of iterator interface so that it can maintain alignment state while permitting the loop to be unrolled by the compiler. (And it might require a closure or someway of expanding a block of code inline.) That's probably the only way it could outperform the simple, hand-rolled, endianness- and alignment-neutral solution. But it doesn't provide that kind of interface AFAICT. It's all sort of ironic, which I suppose was the point upthread--this is an example of the irrational urge for premature optimization and of bad programming idioms being hauled into Rust land completely unhindered by Rust's type safety features. And the better, correct, and likely more performant way of accomplishing this task could have been done just as safely from C as it could from Rust.
      - burntsushi 10y ago
        > Also, looking at the byteorder crate, I wouldn't be surprised if it's even slower than the simpler and correct loop I posted elsethread. read_num_bytes in that create uses copy_nonoverlapping, which I assume is analogous to memcpy in C. That's a very round-a-bout and inefficient way to accomplish the task, and likely patterned after similarly bad C code. It wasn't patterned after any C code. ptr::copy_nonoverlapping doesn't necessarily compile down to memcpy. Namely, concrete sizes are given, so the compiler backend can optimize this down to simple loads and stores on x86, which is probably going to do better than the bit-shifting approach. Namely, loading a little-endian encoded integer on a little-endian architecture should be as simple as a single word-sized load (because the byte swap is unnecessary). It would be interesting to consider whether the safer and more readable bit-shifting approach could be compiled down to the same code, but when I wrote the byteorder crate, this wasn't the case. This isn't the only place that ptr::copy_nonoverlapping is useful. I used it in my snappy[1] implementation as well, specifically to avoid the overhead of memcpy. To be clear, this wasn't my idea. This is what the C++ Snappy reference implementation does as well. Avoiding memcpys in favor of unaligned loads/stores is a dramatic win. I know this because I tried to write my Snappy implementation without specific unaligned loads/stores, and it performed quite a bit worse. The performance of the Rust implementation is now on par with the C++ implementation. Of course, this is always dealing with raw bytes---there's no type punning here. ptr::copy_nonoverlapping is a bit generic for this use case. That's why we recently accepted an RFC to add read_unaligned/write_unaligned to the standard library[2]. (Which are implemented via straight-forward calls to ptr::copy_nonoverlapping.) [1] - https://github.com/BurntSushi/rust-snappy/blob/master/src/decompress.rs https://github.com/BurntSushi/rust-snappy/blob/master/src/de... [2] - https://github.com/rust-lang/rfcs/blob/master/text/1725-unaligned-access.md https://github.com/rust-lang/rfcs/blob/master/text/1725-unal...
  - Manishearth 10y ago
    > We're probably going to see "unsafe" code that assumes good behavior on the part of the caller. I have yet to see any of this. I have noticed that it's harder to write correct unsafe code when it comes to parallelism and FFI, but parallelism has always been a hard problem and the FFI problems generally come from the fact that you need to know the invariants being upheld on the other end, which is trickier. But for this kind of unsafe code -- designing (non-parallel) abstractions -- upholding invariants is pretty straightforward.
    - Gankro 10y ago
      > I have yet to see any of this. mem::forget-pocalypse was this. (Rc/Arc, Vec::drain, thread::scoped) Any UB bug that results from an overflow is kind've implicitly this. BTreeMap::range still has an UB bug from trusting the caller! I literally asked you to fix it! https://github.com/rust-lang/rust/issues/33197 https://github.com/rust-lang/rust/issues/33197 Bugs happen man.
      - Manishearth 10y ago
        > mem::forget-pocalypse was this. I would say that this is from a time when the invariants were not understood. In particular, the fact that leaking is safe to do in safe code was not known. (The invariants are still not completely understood, but there's work to specify that, and IMO they're understood enough to be able to avoid unsafe bugs) > BTreeMap::range still has an UB bug from trusting the caller! Fair :) I'd completely forgotten about that one.
        Gankro 10y ago
        > I would say that this is from a time when the invariants were not understood. Yeah, but it's not like "oh this is an obvious thing to consider trusting the caller about". It's an exceptionally niche problem that you'd only know about if someone told you about it. Especially since a Rust programmer shouldn't be expected to write unsafe code often, if ever! Similarly: not trusting traits to be implemented correctly. Not trusting closures to not-unwind.
staticassertion 10y ago
The 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.