Non-zeroing dynamic drops

[pnkfelix]

Summary

Remove drop flags from values implementing Drop, and remove automatic memory zeroing associated with dropping values.

Keep dynamic drop semantics, by having each function maintain a (potentially empty) set of auto-injected boolean flags for the drop obligations for the function that need to be tracked dynamically (which we will call "dynamic drop obligations").

text/ (rendered)

[arcto]

This looks good. It removes the two worst costs (zeroing and injected flags). Having the flags on the stack shouldn't be so bad. Most of the time no flags would even need to be created, right?

Question: I might have missed this in the RFC but, will it be possible to query the state of the drop flags, in a standard way, from the function/scope encompassing the droppable values?

[pnkfelix]

@arcto no, the RFC does not provide a querying primitive.

Note that such a primitive is not terribly useful, at least not without more significant language changes, since if any branch moves a path like a.x, then all successor nodes in the control flow cannot access a.x. So such a primitive could only be used to try to muck with some state independent from a.x.

[bill-myers]

Dynamic drop is probably a bad idea, because it means that some code cannot be moved out to a function without altering behavior (since the hidden flag cannot be returned from the function), which is a sign of a misdesigned language feature.

One can just use Option to get the same effects, while also allowing to move code into functions (by passing an &mut Option, which the function can leave unchanged or set to None depending on control flow).

[sfackler]

@bill-myers could you give an example of a case where that problem would come up?

[thestinger]

It's required to provide scope-based RAII like D and C++. The lifetime of variables being tied to scopes of what nearly everyone will expect since that's also what the lifetime system assumes and is how it works in other languages. Extending variable lifetimes on borrows like the eager drop proposal would change them from a compile-time check to a feature with runtime impact along with making correct low-level code more difficult to write.

The need for a dynamic drop flag will only occur when there is a conditional early drop and it can be avoided by adding an explicit drop call in the branch to make it unconditional. It will hardly have any performance impact when it isn't optimized out usually will be, so that micro-optimization isn't important.

[arielb1]

The description seems slightly over-complicated: we could add drop flags for every (relevant) path, and then do value propagation and dead store elimination to remove the unneeded ones. Maybe we can even get LLVM to do these optimizations for us.

[zwarich]

@arielb1 It is probably better to treat the optimization separately, but it is probably important to avoid generating drop flags for every single move that occurs. That will only bloat the peak memory usage of rustc even more, even if LLVM could optimize them away.

[thestinger]

The RFC doesn't need to describe the optimization though, elision of the flag is an implementation detail.

[zwarich]

Feasibility of implementation is important when considering an RFC, and if @pnkfelix already did the work in another context he might as well describe it.

[Ericson2314]

It also also necessary for the dynamic drop lint to work.

While I still prefer eager drop, this does solve all the practical problems with the drop flag. Good work, pnkfelix.

[pnkfelix]

Its true that the RFC may not have needed to spec out so much detail. But the crucial thing is that the size of the set of drop obligations is bounded by the size of the fn definition in the program text. (As opposed to a more naive approach where you would first recursively descend the type structure all the way down until you hit an actual Drop impl). One might argue that the latter spec would be observationally equivalent and thus not a problem, but this is where the "feasibility of implementation" argument arises, IMO.

[rkjnsn]

@arcto, wouldn't that be a compiler error? The compiler won't yet you use an object that might already have been moved.

[thestinger]

@arcto: A destructor isn't the only reason for a type moving ownership on shallow copies. The complexity is unnecessary anyway because types that don't move are still usable after a shallow copy.

[arcto]

@rkjsns I don't know, is it? It's very dangerous at least!

@thestinger I just used the name for the macro from the flag name in the RFC.

Never mind. I don't know what I was thinking. It can't be allowed.

[bill-myers]

@sfackler

fn f() {
  let x = S { ... };
  if a() {
    b();
    consume(x);
  }
  c();
}

should be equivalent to:

fn new_function(x: S)
{
  if a() {
    b();
    consume(x);
  }
}

fn f() {
  let x = S { ... };
  new_function(x);
  c();
}

since this is the natural "introduce a function" transformation that should not alter behavior in a well-designed language.

However, with dynamic drop the first snippet will not drop x until c() is executed if a() returns false, while the second snippet always drops x before c() is executed, and thus language behavior is not preserved.

It's also not particularly intuitive that this problem even exists, and not trivial to check manually for complicated code.

The root of this problem is of course the use of "injected boolean flags for the drop obligations for the function" rather than a first class mechanism that can be passed across functions, which already exists (for non-partial moves) in the form of the "Option" type, which provides exactly such a boolean flag, but as a first-class mechanism.

[thestinger]

@bill-myers: You're ignoring that functions introduce a new scope. If you do the same inside a function by moving a variable into the new nested scope it will be no different. Having scope-based RAII doesn't make a programming language poorly designed.

[bill-myers]

@thestinger Functions of course introduce a new scope, but with static drop, behavior is unchanged as long as you move exactly the variables that are consumed, by passing them by value as parameters, which is the natural thing to do; with dynamic drop you can't do that because some variables are "conditionally consumed" and you can't move them at all.

Also it's of course possible to have "scope-based RAII" with static drop: that's what C++ does for instance.

[thestinger]

Functions of course introduce a new scope, but with static drop, behavior is unchanged as long as you move exactly the variables that are consumed, by passing them by value as parameters, which is the natural thing to do; with dynamic drop you can't do that because some variables are "conditionally consumed" and you can't move them at all.

You haven't demonstrated any difference between moving variables into a function scope or a local scope in a function. There isn't a difference between the same code in a local function, but your example isn't using the equivalent code as you're missing a scope and a move into it for the non-split example.

Also it's of course possible to have "scope-based RAII" with static drop: that's what C++ does for instance.

C++ move semantics are implemented with dynamic drop flags. Using a flag on the stack instead of the space provided by the type isn't significantly different. The drop flags were always an implementation detail and code relying on the zeroing within data structures had undefined behaviour already. This proposal preserves the same semantics that exist today, which are also the same in C++.

[bill-myers]

@thestinger There is indeed no difference, but that's unrelated to the issue of whether code can be moved out into a new function without changing the rest of the function (or I guess one could say that the difference is that with a new function everything goes out of scope, including the implicit drop flags which can't be moved into the new scope because they are implicit).

Also, as far as I know, C++11 does not have "move" in the Rust sense at all: "moving out" of an variable just leaves it potentially logically empty (if a custom move constructor empties it), and the destructor is still run normally and there are no language-level drop flags; there is also in fact no concept of dropping anything early at all (you can manually call the destructor, but that will in general crash the program unless you undo it with placement new or equivalent, since the compiler will invoke the destructor again itself when the variable goes out of scope).

[arcto]

It freaks me out a little bit that an object becomes undead after a conditional block in which it might be consumed. You can't interact with it, but still its drop() function may have an effect.

In @bill-myers example: is it viable that x would not be dropped by new_function() but remain undead until the end of its original scope, f() so as to get the dropping-order correct?

[thestinger]

code can be moved out into a new function without changing the rest of the function

Code can be moved into a new function without changing behaviour. It does have to be scoped the same way to be equivalent, but that also applies to scope-based lifetimes.

Also, as far as I know, C++11 does not have "move" in the Rust sense at all: "moving out" of an variable just leaves it potentially logically empty (if a custom move constructor empties it), and the destructor is still run normally and there are no language-level drop flags; there is also in fact no concept of dropping anything early at all (you can manually call the destructor, but that will in general crash the program unless you undo it with placement new or equivalent, since the compiler will invoke the destructor again itself when the variable goes out of scope).

It does have a move in the same sense as Rust. In both languages, the variable may only be destroyed or re-initialized after it has been moved from, which is also true in Rust. Variables are destroyed in reverse order at the end of a block, but there is no effect if they have been moved from. It's incorrect to deviate from that behaviour in C++ and the compiler is explicitly permitted to make the assumption that move and copy constructors only do what they say on the tin.

C++ does use dynamic drop flags to implement that behaviour, although it's an implementation detail for the standard library just like it is in Rust. User code has no choice but to do it that way.

[thestinger]

It freaks me out a little bit that an object becomes undead after a conditional block in which it might be consumed. You can't interact with it, but still its drop() function may have an effect.

You're describing the semantics of scope-based resource management / RAII. The destructor runs at the end of the scope, unless ownership of the value has been moved out of the variable.

In @bill-myers example: is it viable that x would not be dropped by new_function() but remain undead until the end of its original block, f() so as to get the dropping-order correct?

I don't think you understand the proposal. It doesn't change the semantics from what they are today. The destructor runs at the end of the scope unless the variable has been moved from, in which case it does not run. There is nothing new_function can do to make it run in the parent scope. It could return the same value, but it will be immediately destroyed upon return. The drop flag only tracks whether a move has occurred. It doesn't track anything after that happens.

The proposal is a conservative improvement over what exists today. There is really no need for an RFC at all because the currently defined semantics are preserved. It was already undefined to depend on the drop flag within values, so this is just an implementation optimization. There is no language change here.

[pnkfelix]

@thestinger the argument for why an RFC is necessary is that libraries may be relying on the zero'ing behavior.

[thestinger]

Those libraries have undefined behaviour. Low-level compiler implementation details aren't something that unsafe code can rely on unless stated otherwise and AFAIK this was never even documented. It's similar to the representation of virtual function tables or the internals of the memory allocator. Library code can depend on the current virtual function table representation to retrieve the destructor, but there's no guarantee of it continuing to work.

[Gankra]

@bill-myers Your proposed behaviour would make it very difficult to reason locally about ownership. If I see something passed by-value into a function, I don't want to have to read that function (and any function called by it) to try to reason whether the value has actually been moved out of this function. That would be super impractical and annoying. Functions aren't macros that just paste code blocks for you, they're behaviour boundaries.

[nikomatsakis]

cc me

[bill-myers]

@thestinger

The destructor runs at the end of the scope unless the variable has been moved from, in which case it does not run. There is nothing new_function can do to make it run in the parent scope. It could return the same value, but it will be immediately destroyed upon return

Yeah, that's the problem: if you introduce a function, there is no way to do so without changing semantics by making the destructor always run earlier.

Let me try to restate this: in most languages, ANY scoped code block (i.e. code between "{" and "}" within a function) can be moved to a newly introduced function with simple changes, replacing the code block with a function call and possibly a match statement that performs any non-local jumps (e.g. "return", break, goto outside the block, etc.) in the parent function.

Obviously this also holds for any sequence of statements that can be surrounded by "{" and "}" without changing semantics (i.e. those that either don't introduce variables or other bindings or are immediately followed by a "}").

With dynamic drop, this is only true if, for all variables, either all execution paths through the code block leave the variable in a moved out state or if none of them leaves the variable in a moved out state.

Otherwise, you need to change the variable to a "let mut", move it into the function, return it back wrapped into an Option, and add a match statement in the parent that sets back the variable if the Option is Some.

I.e. something like this:

fn new_function(x: S) -> Option<S>
{
  if a() {
    b();
    consume(x);
    None
  } else {
    Some(x)
  }
}

fn f() {
  let mut x = S { ... }; // here we have to add the mut, or introduce a new variable in this outer scope
  ...
  match new_function(x) {
    Some(v) => {x = v},
    None => {}
  }
  c();
}

If you introduce a function in a place where behavior is changed without making that transformation you get no compiler warning or error at all, but rather just a silent and likely unexpected change of semantics (and there's no way to produce such a warning or error unless you pass both versions of the source code to the compiler).

Of course, it's possible that any potential convenience of dynamic drop outweighs the loss of the ability to easily move any block to a new function, but it seems pretty obvious that the issue exists.

[EDIT: I realized it's actually possible to move the code to a function, but it requires to return it back as an Option and conditionally reset it]

[Gankra]

As long as it's backwards compatible with allowing it in the future (even if we never do), I think it's a great compromise to make for the time being.

[aidancully]

Not sure if it affects this RFC, but regarding @bill-myers point, is it technically possible for a tool to implement an extract-method automated refactoring with any kind of implicit drop flag? The extract-method refactoring would need to be able to analyze and manipulate the drop-flag maintenance in order to be sure that it does not change behavior, but since these drop flags are not code-visible or manipulable, it seems like it may be technically impossible to write a tool that would support such an automated refactoring.

[pnkfelix]

It is an open question whether this can actually be added post 1.0 without breaking clients, even with the changes to the language I outlined above. @arielb1 has made an internals thread here:

http://internals.rust-lang.org/t/backwards-compatability-issue-with-non-zeroing-drop/1525

[nikomatsakis]

@pnkfelix I think it's clear that this change will break some unsafe code. Then again, everything breaks some unsafe code. I'm being a bit flippant, but I think if we advertise our intentions here and make it clear that it is wrong to rely on zeroing, then we are within our rights to break unsafe code that relies on it. It'd be great to get it implemented for 1.0 but I don't consider it a strict requirement.

[nikomatsakis]

(Also, this underscores the need for a document laying out what kinds of unsafe code are considered "stable" and what is off bounds, but that's a separate problem from this RFC in particular.)

[nikomatsakis]

We've decided to accept this RFC. Dynamic drop has long been recognized as the implementation strategy of choice. The reasoning for this is documented clearly in the RFC itself, but the primary points in favor of this strategy as compared to static drop semantics were:

1. A closer match to C++ semantics.
2. The correctness of unsafe code often relies on a precise knowledge of when drops occur, and the general opinion was that dynamic drop caused drops to occur in more predictable places. In particular, drops occur either on a move (a clear dynamic event) or the exit of a block, and don't require reasoning about the control-flow graph.

[nikomatsakis]

Tracking issue: rust-lang/rust#5016

[arthurprs]

What's the decision for #[unsafe_no_drop_flag] ?

[pnkfelix]

@arthurprs my suspicion is that the #[unsafe_no_drop_flag] attribute will become a no-op.

(The other option I have considered is to continue respecting it, but instead of controlling whether the type has an extra flag attached to it directly, it would instead control whether you put a flag onto the stack for that type.)

Either approach can be implemented after we have gotten direct experience with non-zeroing drop.

---

Update:

• See also this comment on my filling-drop internals thread, which points out a problem with attempting to continue supporting #[unsafe_no_drop_flag] with the semantics that it would remove even the stack-local flag -- that would then require we add some sort of forget method (that defaults to a no-op) that such structs can override.
• But that kind of machinery definitely pushes me towards saying that we should just remove the #[unsafe_no_drop_flag] attribute when we move to stack-local drop flags.

[reem]

What about intrinsics for manipulating these flags? Useful patterns like https://github.com/reem/rust-replace-map/blob/master/src/lib.rs will need to be made unsafe unless there are intrinsics for manipulating this system.

[bluss]

what replace_map is doing is nonlocal so I don't think it can be ported? By nonlocal I mean that zeroing is used to inhibit the drop in some arbitrary parent frame where src: &mut T is rooted.

[nikomatsakis]

On Mon, Feb 09, 2015 at 05:44:00PM -0800, Jonathan Reem wrote:

What about intrinsics for manipulating these flags? Useful patterns
like https://github.com/reem/rust-replace-map/blob/master/src/lib.rs
will need to be made unsafe unless there are intrinsics for
manipulating this system.

It would certainly be helpful if you could point at a specific line,
but I'm going to assume that what ReplaceMap is doing is zeroing out
data to prevent it from being dropped later (similar to what
BinaryHeap does in the stdlib, I think). This pattern simply does
not translate well into this brave new world, because there will be
types for which zeroing is just not a valid way to disable the
destructor.

The obvious replacement to this pattern is use an Option. This has
some downsides: usability suffers a bit (could be addressed via a
newtype) but also it requires a (sometimes) redundant zero flag. There
might be the kernel of a feature request here: some way to manually
request a drop flag, or otherwise write an (unsafe?) Option-like
wrapper that can be zero'd.

Of course, ReplaceMap could probably handle this another way. I
haven't looked in detail, but it may be possible (for example) to
carry a parallel set of bits to determine what needs to be dropped
etc.

[burdges]

I've a couple questions about the current state of memory zeroing:

Rust currently still zeros any non-Drop traits, yes? Yet, this zeroing is not stabilized, so crates that need it like cryptography should ensure zeroing happens. I suppose the easy way would be a test that fails if this behavior changes, like maybe:

struct SecretKey(pub [u8; 32])

#[test]
fn cryptography() {
    let p : *const SecretKey;
    let s = SecretKey([1u8; 32]);
    p = &s; 
    ::std::mem::drop(s);
    unsafe { assert_eq!(*p,SecretKey([0u8; 32])); }
}

Although actually this test fails, even if we replace the ::std::mem::drop(s); with a scope.

How best should we zero memory in a manual Drop::drop method? Maybe

impl Drop for SecretKey {
    fn drop (&mut self) {
        unsafe { ::std::intrinsics::volatile_set_memory(self,0,1); }
    }
}

Are there any situations where either the zeroing of non-drop traits or the drop might fail to run? How do trait objects handle zeroing on drop? I suppose there must always a drop pointer in the vtable or something?

[eddyb]

@burdges Rust 1.13 and newer never writes to a memory location without user actions (initialization/assignment, etc.). This includes padding between struct fields and complex dataflow.
Even before, there wasn't any zeroing/filling for values with no Drop fields (siblings may have gotten overriden because we would memset a whole rvalue/variable at a time, not every specific Drop field).

[burdges]

I see, so anything that needs zeroing needs a drop method like the one I gave above. Thanks!

[burdges]

As I understand, a drop should always be called unless std::mem::forget() gets called, via https://github.com/isislovecruft/curve25519-dalek/issues/11 Is that really the only way?

If so, is there any interest in making it hard to call std::mem::forget()? Say either a #[never_forget] attribute or more likely a ?Forget bound?

[bluss]

forget is the easy way to do it, but reference count cycles (Rc, Arc) are not hard to construct either. You can spawn a thread that owns the values and spins until the process exits too.

There was a very long discussion in RFC 1066 about this that effectively lead to making forget a safe function, which was kind of a token change marking the conclusion of the discussion/new understanding of memory leaks in relations to Rust's type system. They are not easy to prevent. The discussion visited the possibility of having a ?Leakable bound.