4 ms·

Note that this is only something you really need to think about when writing `unsafe` code, which basically unlocks a whole new set of rules that you need to wo

by Gankro 11y ago

Note that this is only something you really need to think about when writing `unsafe` code, which basically unlocks a whole new set of rules that you need to worry about (zero-sized types in offsets, manual allocations, safety boundaries, GEP/aliasing rules, uninit memory, double destructors, etc). This is a drop in the bucket that anyone venturing to use `unsafe` will have to face in some kind of "guide to unsafety".

mcguire 11y ago
The problem is that all of this is safe code. The problem, to my mind, is that you can write mem::forget (as in the code here's safe_forget)* without ever using the unsafe keyword*. Edit: I was wrong. The problem isn't with forget. This is hairy.
- steveklabnik 11y ago
  The 'mem::forget can be written in safe code' meme comes from https://github.com/rust-lang/rust/issues/24456 https://github.com/rust-lang/rust/issues/24456 Which, as you can see, uses Rc, which uses unsafe code in a way that leads to the soundness bug. So that's not strictly true, or rather, doesn't really change anything, as it relies on the same bug.
  - mcguire 11y ago
    nikomatsakis, in that thread: "1. There were other ways to forget even before Rc, at least in some cases. For example, if T:Send holds, you could send the value to a thread that runs an infinite loop, or which is deadlocked on a port. "2. It is true that one cannot assume that a destructor will run, and hence that forget is not itself unsafe (rather, it is unsafe to write a dtor that must run)...." And alexcrichton: "I commented on #24292, but the gist is that there are multiple ways to leak memory today (e.g. #14875 and #16135), so a targeted solution at Rc may not cover all use cases. Although as I mention in #24292 these other bugs can also be considered separate bugs on their own which need to be fixed regardless (but sometimes is quite difficult to do so)." I especially liked dgrunwald's comment: "A safe mem::forget has the advantage that it makes it easier to write the counterexample proving thread::JoinGuard unsafe. Safe mem::forget makes it more likely that people will know that destructors are not realiable, so they can avoid repeating this mistake." But then, "Put a big freaking spike in the middle of the steering wheel and get rid of the airbags and seat belts" has always appealed to me as an automotive safety approach.
    - steveklabnik 11y ago
      Yeah, I mean, I guess that leaks in general are possible, which would still let you write `forget`. You're right about that. But the unsoundness RC bug still relies on unsafe code which was written incorrectly.
      - sirclueless 11y ago
        I think you need to split the unsoundness bug, which happens because thread::scoped uses unsafe code, from the "you can write mem::forget in safe code" bug, which is arguably not a bug but is at least an enormous footgun waiting to happen. When the footgun goes off in unsafe code in the standard library, you get use-after-free and memory unsoundness. When the footgun goes off in safe code written by mere humans, you leak arbitrary resources.
- eridius 11y ago
  The fact that you can write mem::forget in safe code is nothing more than an argument that mem::forget itself should not be marked as unsafe. The fact remains that as long as you are not using unsafe code yourself, you don't really have to worry about anything. The category of destructors that leave the world in an unsafe state if not run is AFAIK a strict subset of the category of destructors that require unsafe code to be written.
  - mcguire 11y ago
    The problem here looks like it is with Rc, which does require unsafe code. I haven't checked, but I don't think the destructors in the thread::scoped issue are unsafe. Even if they are, I can easily imagine another situation where a programmer assuming a safe destructor is always called causes a bug, which is exactly the problem with thread::scoped. And Rc only looks like it is the problem. Rc is doing exactly what it says it does, as safely as it can. The fact that you can use it to avoid having a destructor called is a side-effect of it doing what it's supposed to do. If main is safe, mem::forget is safe (remember, Rc is just playing the part of mem::forget), and the destructor for JoinGuard is safe (which it is; I just checked), where is the unsafe code?
    - deleted 11y ago
      [deleted]
    - kibwen 11y ago
      `unsafe` is a stateful property, not a local one. The specific invariant here is that you cannot rely on destructors for memory safety, which the old thread::scoped API was doing. The actual unsafe code is actually here: https://github.com/rust-lang/rust/blob/master/src/libstd/thread/mod.rs#L313 https://github.com/rust-lang/rust/blob/master/src/libstd/thr... (I myself have been guilty of this same error, of assuming that `unsafe` regardless of its position cannot have far-reaching effects on the overall state of your code.) The important thing is this: using safe Rust, and using any safe stdlib API, memory unsafety is impossible. This includes stdlib APIs that use `unsafe` internally, such as `Rc`. Proving those interfaces safe is the burden of the Rust developers, and if you can show that there has been an error in these proofs then it is a drop-everything sky-is-falling defcon-1 situation that they must address, up to and including unreleasing the language if no suitable solution came to mind (fortunately, this is quite unlikely).

by Gankro 11y ago

mcguire 11y ago

The problem is that all of this is safe code. The problem, to my mind, is that you can write mem::forget (as in the code here's safe_forget)* without ever using the unsafe keyword*. Edit: I was wrong. The problem isn't with forget. This is hairy.

steveklabnik 11y ago
The 'mem::forget can be written in safe code' meme comes from https://github.com/rust-lang/rust/issues/24456 https://github.com/rust-lang/rust/issues/24456 Which, as you can see, uses Rc, which uses unsafe code in a way that leads to the soundness bug. So that's not strictly true, or rather, doesn't really change anything, as it relies on the same bug.
- mcguire 11y ago
  nikomatsakis, in that thread: "1. There were other ways to forget even before Rc, at least in some cases. For example, if T:Send holds, you could send the value to a thread that runs an infinite loop, or which is deadlocked on a port. "2. It is true that one cannot assume that a destructor will run, and hence that forget is not itself unsafe (rather, it is unsafe to write a dtor that must run)...." And alexcrichton: "I commented on #24292, but the gist is that there are multiple ways to leak memory today (e.g. #14875 and #16135), so a targeted solution at Rc may not cover all use cases. Although as I mention in #24292 these other bugs can also be considered separate bugs on their own which need to be fixed regardless (but sometimes is quite difficult to do so)." I especially liked dgrunwald's comment: "A safe mem::forget has the advantage that it makes it easier to write the counterexample proving thread::JoinGuard unsafe. Safe mem::forget makes it more likely that people will know that destructors are not realiable, so they can avoid repeating this mistake." But then, "Put a big freaking spike in the middle of the steering wheel and get rid of the airbags and seat belts" has always appealed to me as an automotive safety approach.
  - steveklabnik 11y ago
    Yeah, I mean, I guess that leaks in general are possible, which would still let you write `forget`. You're right about that. But the unsoundness RC bug still relies on unsafe code which was written incorrectly.
    - sirclueless 11y ago
      I think you need to split the unsoundness bug, which happens because thread::scoped uses unsafe code, from the "you can write mem::forget in safe code" bug, which is arguably not a bug but is at least an enormous footgun waiting to happen. When the footgun goes off in unsafe code in the standard library, you get use-after-free and memory unsoundness. When the footgun goes off in safe code written by mere humans, you leak arbitrary resources.
eridius 11y ago
The fact that you can write mem::forget in safe code is nothing more than an argument that mem::forget itself should not be marked as unsafe. The fact remains that as long as you are not using unsafe code yourself, you don't really have to worry about anything. The category of destructors that leave the world in an unsafe state if not run is AFAIK a strict subset of the category of destructors that require unsafe code to be written.
- mcguire 11y ago
  The problem here looks like it is with Rc, which does require unsafe code. I haven't checked, but I don't think the destructors in the thread::scoped issue are unsafe. Even if they are, I can easily imagine another situation where a programmer assuming a safe destructor is always called causes a bug, which is exactly the problem with thread::scoped. And Rc only looks like it is the problem. Rc is doing exactly what it says it does, as safely as it can. The fact that you can use it to avoid having a destructor called is a side-effect of it doing what it's supposed to do. If main is safe, mem::forget is safe (remember, Rc is just playing the part of mem::forget), and the destructor for JoinGuard is safe (which it is; I just checked), where is the unsafe code?
  - deleted 11y ago
    [deleted]
  - kibwen 11y ago
    `unsafe` is a stateful property, not a local one. The specific invariant here is that you cannot rely on destructors for memory safety, which the old thread::scoped API was doing. The actual unsafe code is actually here: https://github.com/rust-lang/rust/blob/master/src/libstd/thread/mod.rs#L313 https://github.com/rust-lang/rust/blob/master/src/libstd/thr... (I myself have been guilty of this same error, of assuming that `unsafe` regardless of its position cannot have far-reaching effects on the overall state of your code.) The important thing is this: using safe Rust, and using any safe stdlib API, memory unsafety is impossible. This includes stdlib APIs that use `unsafe` internally, such as `Rc`. Proving those interfaces safe is the burden of the Rust developers, and if you can show that there has been an error in these proofs then it is a drop-everything sky-is-falling defcon-1 situation that they must address, up to and including unreleasing the language if no suitable solution came to mind (fortunately, this is quite unlikely).