Documentation for mem::forget implies that it may invalidate raw pointers to heap allocations

[fleabitdev]

The doc comment for mem::forget currently reads:

Any resources the value manages, such as heap memory or a file handle, will linger forever in an unreachable state. However, it does not guarantee that pointers to this memory will remain valid.

The second sentence was introduced in #53503, with this rationale:

it's not obvious mem::forget does not guarantee leaking of memory: memory of stack-allocated objects and values partially moved out of Box will still be freed

In other words, it's intended to be a warning about these two footguns:

let byte = 42_u8;
let raw_ptr = &byte as *const u8;
std::mem::forget(byte);
let forty_two = unsafe { *raw_ptr };

let raw_ptr;
{
    let boxed_byte = Box::new(42u8);
    raw_ptr = &*boxed_byte as *const u8;
    std::mem::forget(*boxed_byte);
}
let forty_two = unsafe { *raw_ptr };

To me, the current language is too broad. It implies that heap allocations owned by a value passed to mem::forget may be invalidated. For example, it implies that the following code is unsound:

let s = format!("example");
let s_ptr = s.as_ptr();
std::mem::forget(s);
let lowercase_e = unsafe { *s_ptr };

Opening an issue rather than a PR because I'm not sure how best to rephrase this.

[pierwill]

Would like to work on this! ✋

@rustbot claim

[pierwill]

So what can we say for sure? Maybe, based on @fleabitdev's comment: "Heap allocations owned by a value passed to mem::forget are not immediately invalidated." If it is true that mem::forget "does not guarantee that pointers to this memory will remain valid" (which seems right to me 🤷‍♀️), when might pointers to such forgotten memory in fact be invalidated? Is this unknown because undefined?

Perhaps simply adding the sentence above would help? ("Heap allocations owned by a value passed to mem::forget are not immediately invalidated.")

[pierwill]

Thoughts, @jyn514? Just trying to wrap my head around this.

[jyn514]

@pierwill sorry, I don't even understand why the original example is incorrect, I can't guess why the third example is sound.

[fleabitdev]

When a value is passed to mem::forget, safe Rust code will never run its destructor, and so any heap allocations owned by the value will live forever (unsafe code notwithstanding). This is the point which I feel is obscured by the current documentation.

It's an important property of mem::forget; I've written unsafe code which relies on it. You can insert checks in a type's destructor to prevent memory from being deallocated when it's unsafe to do so, but if mem::forget could implicitly perform deallocations, those checks would be bypassed.

[steveklabnik]

cc @rust-lang/lang @rust-lang/libs

[pierwill]

@rustbot release-assignment

[pnkfelix]

I think this is best handled by delegating to the unsafe-code-guidelines WG. (That is, I think any of the footguns and issues discussed arised solely in the context of unsafe code. All sound code should continue to be sound.)

Filed rust-lang/unsafe-code-guidelines#320 and closing this.