Restrictions on use of mem::forget and ManuallyDrop are too strict

[HeroicKatora]

The rules currently specify:

Rule MEM-LEAK
In a secure Rust development, the forget function of std::mem (core::mem) must not be used.

Rule MEM-MANUALLYDROP
In a secure Rust development, any value wrapped in ManuallyDrop must be unwrapped to allow for automatic release (ManuallyDrop::into_inner) or manually released (unsafe ManuallyDrop::drop).

However, there are some legitimate uses that are not insecure, in particular some techniques that are used as part of the standard library to perform some essential operations, that are relevant to soundness of programs. Further details and justification below. I'm in favor of demoting it to a recommendation with peer-review, documentation and justification similar to Drop itself.

When to use it soundly without leaks

When a type has a Drop implementation, it is not possible to destructure it even if one usually has access to its fields by the usual privacy rules. Without being able to use mem::forget this usually means that a resource has exactly one way of cleaning it up. However, there are several instances where it is desirable to use multiple types or methods instead.

The simplest example is Box::leak/Box::into_raw and similar method on other smart pointers. These take a Box<_> by value and release the internal resource to the caller. This is of plainly incompatible with the Drop implementation of the box, that would perform its own cleanup of the resource. Thus it is critical for correctness and soundness that the drop is not executed for the value. This is only possible by wrapping it in a ManuallyDrop on method entry, or alternatively forgetting it before the scope is exited. Note in particular that rule Rule MEM-INTOFROMRAW allows these methods to be called. It seems inconsistent to allow this while forbidding the only possible mechanism that would enable a user-defined re-implementation of such a type.

In a secure Rust development, any pointer created with a call to into_raw [...] must eventually be transformed into a value with a call to the respective from_raw to allow for their reclamation.

Another similar example, but even less dangerious is {Rc,Arc}::into_inner. These essentially are a dynamically checked alternative to retrieving a resource from a Box. They first check whether the by-value argument is the only existing smart pointer to that particular resource and on success return the contained resource, otherwise maintain their pointer representation. Again, it is not possible to implement these without forget as the smart pointer's own Drop logic must not be executed. But here we never forget about any resource, no value is leaked.

Discussion and wording

As we've seen there are valid uses of ManuallyDrop and mem::forget that either are guaranteed to not leak resources or are already allowed with special review to ensure that no such leak occurs in practice. It's expected that most or all of these occur as part of specialized cleanup routines of smart resource owner implementations. This likens their use to Drop implementations even more, they concern the same or similar data structures.

Proposed wording:

Rule MEM-LEAK
In a secure Rust development, the forget function of std::mem (core::mem) must not be used.

Retain this. ManuallyDrop is more expressive and safer against double-drop bugs. In fact, it's implementation consists only of a call to ManuallyDrop::new.

Rule MEM-MANUALLYDROP
In a secure Rust development, any value wrapped in ManuallyDrop must

• be unwrapped to allow for automatic release (ManuallyDrop::into_inner) or
• manually released (unsafe ManuallyDrop::drop)
• or the resources owned by the value must be moved to another resource owner for their reclamation there. This usage should be justified, documented and peer-reviewed.