5 ms·

If changing the safe code causes UB, the actual bug is in the unsafe code.

tialaramex 1y ago
This is very true from outside an encapsulation, but within the private implementation of a type there may be safe Rust that's required to uphold some assumptions made by the unsafe Rust [if there is some] and while in theory the bug is in the incorrect assumption made by the unsafe Rust in practice we're going to fix the "safe" code which broke our assumption instead.
Capricorn2481 1y ago
Nope. I would encourage you to actually read what unsafe does, because nowhere in the Rust docs does it say "scopes bugs to the unsafe block" See the below code. The unsafe code is doing exactly what it's supposed to. The safe code frees a value while it's being used. This compiles. There's nothing to change here in the unsafe code. ```rust use std::slice; // Unsafe block that creates a slice from raw parts fn make_slice(ptr: *const i32, len: usize) -> &'static [i32] { unsafe { slice::from_raw_parts(ptr, len) } } fn main() { let vec = vec![1, 2, 3, 4, 5]; let ptr = vec.as_ptr(); let len = vec.len(); // Unsafe block creates the slice - this operation itself is fine let slice_ref = make_slice(ptr, len); // Safe code: drop the vec drop(vec); // Safe code: use the slice - CRASH/UB println!("{:?}", slice_ref); } ```
- aw1621107 1y ago
  I think one can argue make_slice is in fact at fault here since it's unsound (i.e., it doesn't enforce the requirements of from_raw_parts() and so allows safe code to invoke UB) and therefore either should be marked unsafe or should actually enforce the requirements of from_raw_parts(). That being said, I think in this context that's more of a nitpick since you're right in stating that bugs that result in UB need not be scoped strictly to unsafe blocks.
- Dylan16807 1y ago
  The unsafe code is not fulfilling the contract of "unsafe", which requires the coder to uphold the memory safety rules as seen from outside. make_slice is putting a very wrong lifetime on the slice it returns.
  - Capricorn2481 1y ago
    My whole comment here is that safe code has to care about how it uses data from unsafe code. That's pretty evident from my example and from reading from_raw_parts. How would you fix this by only editing the code inside the unsafe block? Because you said the problem is the lifetime. That's outside of the unsafe block. > The unsafe code is not fulfilling the contract of "unsafe", which requires the coder to uphold the memory safety rules as seen from outside It's the exact opposite. From the Rust docs. [1] > By calling an unsafe function within an unsafe block, we’re saying that we’ve read this function’s documentation and we take responsibility for upholding the function’s contracts The caller here is what is violating the contract. It's not the unsafe code's job to do that. How could it? It's just making a slice, it doesn't know what from. It can't guarantee a slice it makes is ANY kind of lifetime. If it could, it wouldn't require unsafe. We don't have to use a static lifetime to have this bug: // still broken // basically says it has SOME kind of lifetime fn make_slice<'a>(ptr: *const i32, len: usize) -> &'a [i32] { Lifetime annotations are just annotations for the compiler, but they mean very little when using unsafe. I wrote a bad lifetime to illustrate that Rust doesn't care. Normally, the compiler would make sure your lifetimes make sense, but not with unsafe. Because it's the calling code that has to care. Yes, if you change the function signature and require it to receive the vec so you can see the lifetime, you could technically fix it from this function. That's the only way to get the right lifetime. fn make_slice<'a>(ptr: *const i32, len: usize, _lifetime: &'a Vec<i32>) -> &'a [i32] { So yes, as long as you change your function so much you can basically remove unsafe, you can fix it from the function. But you can't do this in scenarios where you'd actually want unsafe, like FFI, custom data structures, etc. You don't have the same lifetime information available to you. [1] https://doc.rust-lang.org/book/ch20-01-unsafe-rust.html#calling-an-unsafe-function-or-method https://doc.rust-lang.org/book/ch20-01-unsafe-rust.html#call...
    - 1y ago

This is very true from outside an encapsulation, but within the private implementation of a type there may be safe Rust that's required to uphold some assumptions made by the unsafe Rust [if there is some] and while in theory the bug is in the incorrect assumption made by the unsafe Rust in practice we're going to fix the "safe" code which broke our assumption instead.

Capricorn2481 1y ago

Nope. I would encourage you to actually read what unsafe does, because nowhere in the Rust docs does it say "scopes bugs to the unsafe block" See the below code. The unsafe code is doing exactly what it's supposed to. The safe code frees a value while it's being used. This compiles. There's nothing to change here in the unsafe code. ```rust use std::slice; // Unsafe block that creates a slice from raw parts fn make_slice(ptr: *const i32, len: usize) -> &'static [i32] { unsafe { slice::from_raw_parts(ptr, len) } } fn main() { let vec = vec![1, 2, 3, 4, 5]; let ptr = vec.as_ptr(); let len = vec.len(); // Unsafe block creates the slice - this operation itself is fine let slice_ref = make_slice(ptr, len); // Safe code: drop the vec drop(vec); // Safe code: use the slice - CRASH/UB println!("{:?}", slice_ref); } ```

aw1621107 1y ago
I think one can argue make_slice is in fact at fault here since it's unsound (i.e., it doesn't enforce the requirements of from_raw_parts() and so allows safe code to invoke UB) and therefore either should be marked unsafe or should actually enforce the requirements of from_raw_parts(). That being said, I think in this context that's more of a nitpick since you're right in stating that bugs that result in UB need not be scoped strictly to unsafe blocks.
Dylan16807 1y ago
The unsafe code is not fulfilling the contract of "unsafe", which requires the coder to uphold the memory safety rules as seen from outside. make_slice is putting a very wrong lifetime on the slice it returns.
- Capricorn2481 1y ago
  My whole comment here is that safe code has to care about how it uses data from unsafe code. That's pretty evident from my example and from reading from_raw_parts. How would you fix this by only editing the code inside the unsafe block? Because you said the problem is the lifetime. That's outside of the unsafe block. > The unsafe code is not fulfilling the contract of "unsafe", which requires the coder to uphold the memory safety rules as seen from outside It's the exact opposite. From the Rust docs. [1] > By calling an unsafe function within an unsafe block, we’re saying that we’ve read this function’s documentation and we take responsibility for upholding the function’s contracts The caller here is what is violating the contract. It's not the unsafe code's job to do that. How could it? It's just making a slice, it doesn't know what from. It can't guarantee a slice it makes is ANY kind of lifetime. If it could, it wouldn't require unsafe. We don't have to use a static lifetime to have this bug: // still broken // basically says it has SOME kind of lifetime fn make_slice<'a>(ptr: *const i32, len: usize) -> &'a [i32] { Lifetime annotations are just annotations for the compiler, but they mean very little when using unsafe. I wrote a bad lifetime to illustrate that Rust doesn't care. Normally, the compiler would make sure your lifetimes make sense, but not with unsafe. Because it's the calling code that has to care. Yes, if you change the function signature and require it to receive the vec so you can see the lifetime, you could technically fix it from this function. That's the only way to get the right lifetime. fn make_slice<'a>(ptr: *const i32, len: usize, _lifetime: &'a Vec<i32>) -> &'a [i32] { So yes, as long as you change your function so much you can basically remove unsafe, you can fix it from the function. But you can't do this in scenarios where you'd actually want unsafe, like FFI, custom data structures, etc. You don't have the same lifetime information available to you. [1] https://doc.rust-lang.org/book/ch20-01-unsafe-rust.html#calling-an-unsafe-function-or-method https://doc.rust-lang.org/book/ch20-01-unsafe-rust.html#call...
  - 1y ago