Stream: t-lang/wg-unsafe-code-guidelines

Topic: invalidate through drop


gnzlbg (Dec 23 2019 at 13:15, on Zulip):

Is this code sound:

struct S(Vec<u8>);
let mut s: S;
ptr::drop_in_place(&mut s.0);
// s.0 contains a field that has been dropped
ptr::write(&mut s.0, Vec::new(); // &mut s.0 creates a ref to dropped field

?

In particular, creating a &mut s or &mut s.0 after s.0 has been dropped, might create an invalid reference ?

simulacrum (Dec 23 2019 at 13:23, on Zulip):

Is that meant to be let mut s: S = S(Vec::new()) or so?

simulacrum (Dec 23 2019 at 13:23, on Zulip):

as is that doesn't compile since s isn't initialized

gnzlbg (Dec 23 2019 at 13:29, on Zulip):

it doesn't matter how its initialized, only that such a binding exists

gnzlbg (Dec 23 2019 at 13:29, on Zulip):

it could be fn foo(mut x: S) { ...code goes here... }

simulacrum (Dec 23 2019 at 13:33, on Zulip):
struct S(Vec<u8>);
let mut s: S = ...;
ptr::drop_in_place(&mut s.0);
// s.0 contains a field that has been dropped
ptr::write(&mut s.0, Vec::new(); // &mut s.0 creates a ref to dropped field

is then more accurate perhaps?

simulacrum (Dec 23 2019 at 13:33, on Zulip):

i.e., actually initializing the field

gnzlbg (Dec 23 2019 at 13:34, on Zulip):

sure

Amanieu (Dec 23 2019 at 13:47, on Zulip):

Yes this is sound as long as you make sure that no panic is possible while the value is "dropped".

gnzlbg (Dec 23 2019 at 13:53, on Zulip):

So what's the "value" of the dropped Vec ?

gnzlbg (Dec 23 2019 at 13:54, on Zulip):

Like maybe an even simpler example:

struct S(NonNull<u8>);
let mut s: S;
ptr::drop_in_place(&mut s.0);
let x: &mut NonNull<u8> = &mut s.0;
gnzlbg (Dec 23 2019 at 13:55, on Zulip):

The NonNull<u8> has been dropped, yet the validity invariant of NonNull<u8> requires it to be nonnull

gnzlbg (Dec 23 2019 at 13:56, on Zulip):

AFAICT dropping a value uninitializes its storage

gnzlbg (Dec 23 2019 at 13:56, on Zulip):

So x would be a reference to uninitialized memory, and therefore invalid

gnzlbg (Dec 23 2019 at 14:01, on Zulip):

Maybe a different way to phrase it would be whether a valid implementation of ptr::drop_in_place can write mem::uninitialized after dropping the values through the pointer ?

gnzlbg (Dec 23 2019 at 14:02, on Zulip):

Or whether after dropping a value its storage becomes uninitialized

gnzlbg (Dec 23 2019 at 14:04, on Zulip):

(essentially, the &mut s.0 after the ptr::drop_in_place is a "use-after-drop")

Lokathor (Dec 23 2019 at 14:35, on Zulip):

Normally dropping a value can uninit the storage because the storage is on a stack frame which you might have just popped away because drops normally only fires when scopes close. However, that's just the stack popping, there's no reason to write uninit if it's dropped as part of drop_in_place.

Lokathor (Dec 23 2019 at 14:44, on Zulip):

drop_in_place is unsafe and can trivially lead to a double-free, but I don't see anything that would also allow it to just write uninit and break validity invariants that way. It might be under specified to the point where it's technically allowed to do that, but it should not and if it did start doing that in some new release of rust you'd get torches and pitchforks at the door.

Lokathor (Dec 23 2019 at 22:48, on Zulip):

(deleted)

rkruppe (Dec 24 2019 at 08:44, on Zulip):

As far as I can tell, the only potential issue with the first example is that the &mut created in the last line is referring to uninit or otherwise not-safe memory. But this can be trivially side-stepped by using &raw mut s.0 (once that syntax is stable) and that's cleaner and more explicit anyway.

RalfJ (Dec 26 2019 at 17:57, on Zulip):

So what's the "value" of the dropped Vec ?

there's no such thing. or do you mean "what are the contents of the memory where the Vec is stored"? I don't think there is any general way to tell. It's whatever drop_in_place left behind. It must satisfy the validity invariant, I guess, but that's about it.

gnzlbg (Dec 28 2019 at 13:09, on Zulip):

So drop_in_place must satisfy the validity invariant ?

gnzlbg (Dec 28 2019 at 13:09, on Zulip):

As far as I can tell, the only potential issue with the first example is that the &mut created in the last line is referring to uninit or otherwise not-safe memory.

That's the main issue I see as well.

gnzlbg (Dec 28 2019 at 13:11, on Zulip):

@RalfJ IIUC, for drop_in_place to satisfy the validity invariant, Drop::drop(&mut self) needs to satisfy the validity invariant as well.

gnzlbg (Dec 28 2019 at 13:11, on Zulip):

And by satisfy I mean that Drop::drop is not allowed to leave self in an invalid state.

gnzlbg (Dec 28 2019 at 13:12, on Zulip):

That's something that Drop::drop docs do not spell out though.

simulacrum (Dec 28 2019 at 13:12, on Zulip):

They shouldn't need to?

simulacrum (Dec 28 2019 at 13:13, on Zulip):

i.e., &mut self means that the Self must be valid throughout the method

simulacrum (Dec 28 2019 at 13:13, on Zulip):

notably, we encounter this in other places too, e.g., AtomicUsize::fetch_sub which ends up dropping the Atomic before the end of the method, while we still have &self in that method

gnzlbg (Dec 28 2019 at 13:19, on Zulip):

notice that the content of the Drop impl doesn't matter

gnzlbg (Dec 28 2019 at 13:19, on Zulip):

the fields will be dropped "after" the Drop::drop function finishes, as part of dropping the value

gnzlbg (Dec 28 2019 at 13:20, on Zulip):

i.e. a call to Drop::drop(s) does not only execute the Drop::drop method, but it then subsequently drops the fields of s

gnzlbg (Dec 28 2019 at 13:20, on Zulip):

and this subsequent dropping of the fields of s would need to leave s in a "valid" state for the example above to not have UB

gnzlbg (Dec 28 2019 at 13:21, on Zulip):

In particular, notice that S(Vec<T>) does not implement Drop, its fields do

gnzlbg (Dec 28 2019 at 13:22, on Zulip):

the behavior of dropping a field is unspecified

gnzlbg (Dec 28 2019 at 13:24, on Zulip):

If I have:

struct S(i32);
let mut x = S(42);
unsafe { ptr::drop_in_place(&mut x) };

we don't specify anywhere in which state the memory of x is left in after the S(i32) is dropped

gnzlbg (Dec 28 2019 at 13:25, on Zulip):

Does the memory of x is left with a S(42) value? Does it become S(uninit) ?

gnzlbg (Dec 28 2019 at 13:26, on Zulip):

If it becomes S(uninit), then that call to drop_in_place is already UB.

simulacrum (Dec 28 2019 at 14:12, on Zulip):

my understanding is that it does not become uninit, but becomes unknown -- i.e., you should not do anything that could cause access to it

simulacrum (Dec 28 2019 at 14:12, on Zulip):

(read) access

Lokathor (Dec 28 2019 at 16:18, on Zulip):

Drop isn't specially able to write uninit when other code couldn't. In other words, any place you couldn't write uninit to &mut self, Drop can't do it either.

Drop can safely put your safety invariants out of place (eg: free some memory, de-init an API, etc), but it has no special validity invariant breaking ability at all.

simulacrum (Dec 28 2019 at 16:25, on Zulip):

Yes, that's my understanding as well. Note that this is somewhat unique to drop -- no other safe function has that property. Arguably, Drop::drop maybe should be an unsafe fn because of this (but since you can't call it directly doesn't matter too much)

RalfJ (Dec 29 2019 at 11:54, on Zulip):

So drop_in_place must satisfy the validity invariant ?

I cannot parse this question

RalfJ (Dec 29 2019 at 11:56, on Zulip):

@Lokathor @simulacrum agreed. And Drop::drop de-facto is unsafe... or rather, it's a magic special snowflake. It doesn't actually have the type fn(&mut Self), clearly.

gnzlbg (Jan 02 2020 at 12:56, on Zulip):

@RalfJ you stated:

It's whatever drop_in_place left behind. It must satisfy the validity invariant, I guess, but that's about it.

So my question was related to that. What do you mean with the claim that after ptr::drop_in_place(ptr) the memory at ptr must satisfy the validity invariant?

gnzlbg (Jan 02 2020 at 13:00, on Zulip):

Do you literally just mean that? (from the other comments, it appears so)

gnzlbg (Jan 02 2020 at 13:04, on Zulip):

my understanding is that it does not become uninit, but becomes unknown -- i.e., you should not do anything that could cause access to it

@simulacrum We don't have an "unknown" state in the abstract machine

gnzlbg (Jan 02 2020 at 13:04, on Zulip):

AFAICT, from reading @Lokathor and @RalfJ comments, Drop::drop cannot violate the validity invariant, and that means it cannot write to the memory any value that isn't valid for the type.

gnzlbg (Jan 02 2020 at 13:05, on Zulip):

That includes "uninit" for some types (for MaybeUninit it would be ok), and probably any kind of "unknown"state.

gnzlbg (Jan 02 2020 at 13:06, on Zulip):

This means it must be ok to read from a memory location after droping its contents, i.e., a read-after-drop is not UB.

gnzlbg (Jan 02 2020 at 13:06, on Zulip):

And since the memory contains a valid value, why wouldn't such a read be ok ?

gnzlbg (Jan 02 2020 at 13:06, on Zulip):

However, like @Lokathor mentioned, Drop::drop can break the safety invariants of a type

gnzlbg (Jan 02 2020 at 13:07, on Zulip):

so if you have a type for which this is the case, then you need to be extra careful, but none of this is UB per se

simulacrum (Jan 02 2020 at 13:08, on Zulip):

Just because the memory is valid doesn't make using the value there safe; furthermore, e.g. Box or references would not (technically) be safe to read.

gnzlbg (Jan 02 2020 at 13:09, on Zulip):

A Box is only valid if it is safe to read

gnzlbg (Jan 02 2020 at 13:09, on Zulip):

e.g., because a Box is dereferenceable

simulacrum (Jan 02 2020 at 13:10, on Zulip):

It's also true I think that we consider dropping to end the lifetime of the memory (i.e. llvm can reuse the stack slot for example)

gnzlbg (Jan 02 2020 at 13:10, on Zulip):

For a type like i32, a double drop is not UB, according to the model being discussed here

gnzlbg (Jan 02 2020 at 13:10, on Zulip):

e.g.:

let mut x = 42_i32;
ptr::drop_in_place(&mut x);
ptr::drop_in_place(&mut x); // OK
gnzlbg (Jan 02 2020 at 13:11, on Zulip):

Just because the memory is valid doesn't make using the value there safe;

No, as mentioned, Drop::drop is allowed to leave the memory in a state that does not satisfy the safety invariant of a type. But there are many operations for which that is ok.

gnzlbg (Jan 02 2020 at 13:12, on Zulip):

For example, Vec::set_len can be used to leave the Vec in a state that does not satisfy the safety invariant, and that turns out to be a very useful thing

gnzlbg (Jan 02 2020 at 13:13, on Zulip):

It's also true I think that we consider dropping to end the lifetime of the memory (i.e. llvm can reuse the stack slot for example)

That's something that we would need to document somehow.

gnzlbg (Jan 02 2020 at 13:13, on Zulip):

AFAICT we can do that when Drop::drop is called implicitly because then the language makes it impossible for the user to try to access that memory location due to ownership, but we can't do that when the user calls ptr::drop_in_place manually.

simulacrum (Jan 02 2020 at 13:14, on Zulip):

We could just assert that any operation on dropped values is UB

gnzlbg (Jan 02 2020 at 13:15, on Zulip):

In general that would require us to add some sort of "dropped" state to the abstract machine

simulacrum (Jan 02 2020 at 13:15, on Zulip):

i.e., drop_in_place is not required to satisfy validity invariant, though Drop::drop is

simulacrum (Jan 02 2020 at 13:15, on Zulip):

I don't think so? The value just doesn't exist

gnzlbg (Jan 02 2020 at 13:16, on Zulip):

I think the consensus from above was that Drop::drop and drop_in_place are required to satisfy the validity invariant

gnzlbg (Jan 02 2020 at 13:16, on Zulip):

(basically as currently defined, violating it is instant UB)

simulacrum (Jan 02 2020 at 13:16, on Zulip):

I think we have consensus that Drop::drop is. I'm not sure about drop_in_place - it takes *mut T, right?

gnzlbg (Jan 02 2020 at 13:17, on Zulip):

Yes, but it drops the memory at the *mut T address

simulacrum (Jan 02 2020 at 13:17, on Zulip):

So there's no &mut to speak of, technically

gnzlbg (Jan 02 2020 at 13:17, on Zulip):

Since drop_in_place must call Drop::drop(&mut T)

gnzlbg (Jan 02 2020 at 13:18, on Zulip):

then that pointer gets dereferenced at some point

simulacrum (Jan 02 2020 at 13:18, on Zulip):

Well, yes, the Drop can't do anything

simulacrum (Jan 02 2020 at 13:18, on Zulip):

But after Drop is called, it could do anything with said memory (e.g. reuse it for some other type, with different validity invariants)

gnzlbg (Jan 02 2020 at 13:19, on Zulip):

The difference is that, when Drop::drop is implicitly called, the memory becomes unaccessible for the rest of the program

gnzlbg (Jan 02 2020 at 13:19, on Zulip):

when you manually call ptr::drop_in_place(ptr), the ptr is still accessible

gnzlbg (Jan 02 2020 at 13:19, on Zulip):

in general, if memory is unaccessible, LLVM can do whatever it wants with it

gnzlbg (Jan 02 2020 at 13:19, on Zulip):

the user can't tell

gnzlbg (Jan 02 2020 at 13:20, on Zulip):

However for drop_in_place, were LLVM to use the memory at ptr for something else, the user would be able to tell

gnzlbg (Jan 02 2020 at 13:21, on Zulip):

We could say that memory for which drop_in_place is called can only be written to, but not read from, or something like that

simulacrum (Jan 02 2020 at 13:21, on Zulip):

I think the ptr itself is still valid, that can't change (*mut T is Copy, I believe)

simulacrum (Jan 02 2020 at 13:22, on Zulip):

but the memory behind it is not valid to use

gnzlbg (Jan 02 2020 at 13:22, on Zulip):

Well, currently, AFAICT, the memory behind the pointer is valid to use

gnzlbg (Jan 02 2020 at 13:22, on Zulip):

because if ptr::drop_in_place makes it invalid, the behavior is undefined

simulacrum (Jan 02 2020 at 13:22, on Zulip):

how so?

gnzlbg (Jan 02 2020 at 13:22, on Zulip):

an invalid value is undefined behavior

simulacrum (Jan 02 2020 at 13:23, on Zulip):

er, well, I'm confused why ptr::drop_in_place making it invalid is UB

simulacrum (Jan 02 2020 at 13:23, on Zulip):

or, rather, why it would not be acceptable to say that ptr::drop_in_place makes the memory behind *mut T invalid.

gnzlbg (Jan 02 2020 at 13:23, on Zulip):

because for ptr::drop_in_place to make the memory at pointer invalid, it would need to do: *ptr = invalid;

gnzlbg (Jan 02 2020 at 13:23, on Zulip):

and that is UB

simulacrum (Jan 02 2020 at 13:24, on Zulip):

I would disagree

simulacrum (Jan 02 2020 at 13:24, on Zulip):

well, not the literal claim

gnzlbg (Jan 02 2020 at 13:24, on Zulip):

I mean, if ptr::drop_in_place(ptr) changes the state of the memory behind ptr, it needs to do so by "writting" to that ptr "somehow"

gnzlbg (Jan 02 2020 at 13:25, on Zulip):

at least, in the abstract machine (the generated code might do nothing, e.g., like writing undef or so)

simulacrum (Jan 02 2020 at 13:25, on Zulip):
unsafe fn drop_in_place(ptr: *mut T) {
    Drop::drop(&mut *ptr);
    ptr::write(ptr as *mut SomeOtherType, ...);
}
gnzlbg (Jan 02 2020 at 13:26, on Zulip):

ah, so your point is that it would be invalid for T, but not for SomeOtherType ?

simulacrum (Jan 02 2020 at 13:26, on Zulip):

Basically, yeah

gnzlbg (Jan 02 2020 at 13:26, on Zulip):

yeah that would work i think

gnzlbg (Jan 02 2020 at 13:27, on Zulip):

I'm not sure how many optimizations that allows

simulacrum (Jan 02 2020 at 13:27, on Zulip):

Well, I think the main optimization is that if we know that *mut T points to the stack, LLVM/compiler can reuse it

simulacrum (Jan 02 2020 at 13:27, on Zulip):

(for other types, same type, whatever)

gnzlbg (Jan 02 2020 at 13:27, on Zulip):

only if the user does not write to it afterwards

gnzlbg (Jan 02 2020 at 13:28, on Zulip):

or at least, in the "window" in which the user does not write to it afterwards

simulacrum (Jan 02 2020 at 13:28, on Zulip):

well, I would argue that the user writing to it is UB, which is why we can make that optimization

gnzlbg (Jan 02 2020 at 13:28, on Zulip):

hmm

gnzlbg (Jan 02 2020 at 13:28, on Zulip):

notice that Vec::pop uses drop in place to drop an element

gnzlbg (Jan 02 2020 at 13:28, on Zulip):

and then Vec::push writes to that memory again afterwards

simulacrum (Jan 02 2020 at 13:28, on Zulip):

i.e.,

let a: T = ...;
ptr::drop_in_place(&mut a);
let b: T = ...; // can be put in the same place as a
gnzlbg (Jan 02 2020 at 13:29, on Zulip):

For that optimization to happen we don't really need the extra guarantee on drop_in_place I think

simulacrum (Jan 02 2020 at 13:29, on Zulip):

hm, I'm confused

simulacrum (Jan 02 2020 at 13:29, on Zulip):

Vec::pop shouldn't be calling drop_in_place?

gnzlbg (Jan 02 2020 at 13:30, on Zulip):

Sorry, Vec::truncate(N) for example

gnzlbg (Jan 02 2020 at 13:31, on Zulip):

https://doc.rust-lang.org/src/alloc/vec.rs.html#729-750

simulacrum (Jan 02 2020 at 13:31, on Zulip):

yes, I think this is a bit different, because there the memory is on the heap

gnzlbg (Jan 02 2020 at 13:31, on Zulip):

yes, I was talking in general about how we would write the ptr::drop_in_place docs

simulacrum (Jan 02 2020 at 13:31, on Zulip):

maybe that's not a meaningful distinction (and I'm not sure that we want to make it in the abstract machine)

gnzlbg (Jan 02 2020 at 13:31, on Zulip):

I think that the optimization you want to perform is reasonable

gnzlbg (Jan 02 2020 at 13:32, on Zulip):

and doable on the stack, but not as simple in general on the heap (maybe?)

gnzlbg (Jan 02 2020 at 13:32, on Zulip):

I'm not sure we'd need to prevent users from writing to the ptr after a drop_in_place (or even reading) to allow it

gnzlbg (Jan 02 2020 at 13:32, on Zulip):

like, right now, we can already perform this optimization as long as the user cannot tell (we can spill values from registers to the stack and back)

simulacrum (Jan 02 2020 at 13:33, on Zulip):

well, the advantage to making this guarantee is that e.g. generators could use it for layout optimization I think

simulacrum (Jan 02 2020 at 13:33, on Zulip):

which I guess is already true under "as if" to an extent, as you say

gnzlbg (Jan 02 2020 at 13:34, on Zulip):

e.g.

fn foo() -> i32 {
let a = 42;
let b = 42;
b
}

here b could be put in the same place as a, and well, it doesn't to be put in the stack at all.

simulacrum (Jan 02 2020 at 13:34, on Zulip):

I guess -- maybe a more reasonable and universal (not dependent on where *mut T points to) definition is that drop_in_place is allowed to leave the value in an invalid state

simulacrum (Jan 02 2020 at 13:35, on Zulip):

but that you are allowed to write to that pointer (e.g., via ptr::write)

gnzlbg (Jan 02 2020 at 13:35, on Zulip):

I think it would be just simpler to say that ptr::drop_in_place calls Drop::drop(&mut *ptr) and that's it

simulacrum (Jan 02 2020 at 13:35, on Zulip):

Well, that's different, right?

gnzlbg (Jan 02 2020 at 13:35, on Zulip):

that means that the value behind the *mut T must be valid, and must be left in a valid state

simulacrum (Jan 02 2020 at 13:36, on Zulip):

Like, that's not "simpler", that's a different definition

gnzlbg (Jan 02 2020 at 13:36, on Zulip):

not necessarily in a safe state

gnzlbg (Jan 02 2020 at 13:36, on Zulip):

well, that's how I understand the current definition, given the feedback that @Lokathor gave above

simulacrum (Jan 02 2020 at 13:36, on Zulip):

I'm not talking about a current definition

gnzlbg (Jan 02 2020 at 13:36, on Zulip):

that still allows your optimization

gnzlbg (Jan 02 2020 at 13:37, on Zulip):

or maybe put differently, is there an example for which that definition does not allow your optimization ?

simulacrum (Jan 02 2020 at 13:37, on Zulip):

I'm saying that ptr::drop_in_place can leave value in an invalid state is different from ptr::drop_in_place calls Drop::drop(&mut *ptr)

gnzlbg (Jan 02 2020 at 13:37, on Zulip):

sure, those are different

gnzlbg (Jan 02 2020 at 13:38, on Zulip):

I wouldn't say that your definition leaves the value in an invalid state, as that is instant UB because it means an invalid value was created, and then written to some address

gnzlbg (Jan 02 2020 at 13:38, on Zulip):

i would say it writes a different value to the memory behind the pointer

gnzlbg (Jan 02 2020 at 13:38, on Zulip):

we could maybe change the definition to this one, but I fail to see what it buys us

simulacrum (Jan 02 2020 at 13:39, on Zulip):

I would say that the memory is left invalid, which is not UB, because it is the compiler/language doing it, i.e., conceptually there is no more T at that place

gnzlbg (Jan 02 2020 at 13:40, on Zulip):

If there is no more T, there is something else there.

simulacrum (Jan 02 2020 at 13:40, on Zulip):

I'm not sure that it buys us anything, that's true. I think unless we go further and start talking about the stack/heap distinction it probably doesn't

gnzlbg (Jan 02 2020 at 13:40, on Zulip):

Even going for the stack / heap distinction, is there an optimization that we can't do without that ?

gnzlbg (Jan 02 2020 at 13:41, on Zulip):

You mentioned generators before, but I can't think of one optimization that isn't possible without this.

simulacrum (Jan 02 2020 at 13:41, on Zulip):

reusing the stack slot when the pointer is passed into unknown code (so we can't know what it does with it)

simulacrum (Jan 02 2020 at 13:42, on Zulip):
let a: T = ...;
let ptr: *mut T = &mut a;
ptr::drop_in_place(ptr);
some_c_function(ptr);
let b: T = ...; // can be put in the same place as a
gnzlbg (Jan 02 2020 at 13:43, on Zulip):

But that code does not use a after some_c_function, so b can always be put in the same place as a

simulacrum (Jan 02 2020 at 13:43, on Zulip):

well, per your rules, we can't, no

simulacrum (Jan 02 2020 at 13:43, on Zulip):

because the slot has "leaked" through ptr, right?

gnzlbg (Jan 02 2020 at 13:43, on Zulip):

Ah, indeed, yes

gnzlbg (Jan 02 2020 at 13:44, on Zulip):

So that unknown code can't do anything with the ptr right?

gnzlbg (Jan 02 2020 at 13:45, on Zulip):

It can't read from it, it can't write to it

gnzlbg (Jan 02 2020 at 13:45, on Zulip):

It can just copy it around

gnzlbg (Jan 02 2020 at 13:45, on Zulip):

(deleted)

simulacrum (Jan 02 2020 at 13:45, on Zulip):

sure

simulacrum (Jan 02 2020 at 13:46, on Zulip):

it's basically just a usize for that code

gnzlbg (Jan 02 2020 at 13:46, on Zulip):

and this optimization is important?

simulacrum (Jan 02 2020 at 13:47, on Zulip):

perhaps for generators to avoid non-local analysis (i.e., without inlining)

simulacrum (Jan 02 2020 at 13:47, on Zulip):

but to be honest, probably no

gnzlbg (Jan 02 2020 at 13:47, on Zulip):

maybe it would be worth it to try to come up with a generator example

simulacrum (Jan 02 2020 at 13:47, on Zulip):

well, basically copy/pasting mine, right? With an .await after the C call

gnzlbg (Jan 02 2020 at 13:49, on Zulip):

I mean, even with the .await, the example is still not very useful, because the non-local code can't do anything with the ptr in your example

gnzlbg (Jan 02 2020 at 13:49, on Zulip):

except for passing it around

simulacrum (Jan 02 2020 at 13:49, on Zulip):

only under my proposal, right?

gnzlbg (Jan 02 2020 at 13:49, on Zulip):

right

gnzlbg (Jan 02 2020 at 13:49, on Zulip):

with my proposal, the non-local code can read/write to the value, so its stack cannot be reused

simulacrum (Jan 02 2020 at 13:49, on Zulip):

right, exactly

gnzlbg (Jan 02 2020 at 13:50, on Zulip):

but that's useful

gnzlbg (Jan 02 2020 at 13:50, on Zulip):

you can at least read or write to the value, and re-initialize it to something else if you want

simulacrum (Jan 02 2020 at 13:50, on Zulip):

I would personally want to say that we should have a model which says that reading from such a value is not defined

simulacrum (Jan 02 2020 at 13:51, on Zulip):

but I would be okay saying that writing to it is

gnzlbg (Jan 02 2020 at 13:51, on Zulip):

i'm not opposed to that, but i'd prefer to balance optimizations with specification simplicity

simulacrum (Jan 02 2020 at 13:51, on Zulip):

that basically means that we can reuse the stack slot for other values, without "cleaning up" after ourselves, so long as we can see that the code is not using it

gnzlbg (Jan 02 2020 at 13:51, on Zulip):

if there are optimizations that are worth performing, then we should try to do so

simulacrum (Jan 02 2020 at 13:52, on Zulip):

I think a model which says ptr::drop_in_place leaves the place in an invalid state, but does not invalidate the place itself (i.e., the pointer points to 'usable' memory) is pretty straightforward

gnzlbg (Jan 02 2020 at 13:53, on Zulip):

If ptr::drop_in_place(ptr) "desugars" to Drop::drop(&mut *ptr), then I think reasoning about it is quite simple, because everything else users have to know about Drop::drop and pointers and values just translates to it

gnzlbg (Jan 02 2020 at 13:53, on Zulip):

Like, that desugaring implies that the value behind ptr must be valid before the ptr::drop_in_place call, that it is left in a valid state, so reading / writing to it using ptr::{read,write} is ok, etc.

gnzlbg (Jan 02 2020 at 13:54, on Zulip):

The only reason drop_in_place is unsafe is because Drop::drop does not necessarily leave the value in a "safe" state

simulacrum (Jan 02 2020 at 13:55, on Zulip):

okay, but then I would argue that we should have changed (or should change, if we can) ptr::drop_in_place to take &mut T

gnzlbg (Jan 02 2020 at 13:55, on Zulip):

It means that we don't have to make drop_in_place a "special" primitive, and that we don't have to add a new state for a "dropped" value to the abstract machine

gnzlbg (Jan 02 2020 at 13:55, on Zulip):

okay, but then I would argue that we should have changed (or should change, if we can) ptr::drop_in_place to take &mut T

+1

gnzlbg (Jan 02 2020 at 13:55, on Zulip):

Like, I at least have no idea why this isn't the case

gnzlbg (Jan 02 2020 at 13:56, on Zulip):

I mean, I guess the only reason might be ergonomics, as in, one typically uses ptr::drop_in_place when working with raw pointers.

gnzlbg (Jan 02 2020 at 13:56, on Zulip):

and a reference coerces to one, so the current API works for both cases

gnzlbg (Jan 02 2020 at 13:57, on Zulip):

but yeah, I've had the same feeling while thinking about these issues, why doesn't ptr::drop_in_place take a &mut T ?

simulacrum (Jan 02 2020 at 14:06, on Zulip):

https://github.com/rust-lang/rust/issues/27908

simulacrum (Jan 02 2020 at 14:07, on Zulip):

Seems like basically we stabilized without answering the question

simulacrum (Jan 02 2020 at 14:09, on Zulip):

certainly there's some discussion about leaving the value in an invalid state

gnzlbg (Jan 02 2020 at 14:26, on Zulip):

Thanks

gnzlbg (Jan 02 2020 at 14:27, on Zulip):

When they talk about "invalidation", I'm not sure they are using the word with the same meaning that we use now

simulacrum (Jan 02 2020 at 14:27, on Zulip):

yeah, I don't think it was around then as a meaning, or at least certainly less formal

gnzlbg (Jan 02 2020 at 14:27, on Zulip):

Looks like they are talking more about violating the safety invariant, than the validity invariant

simulacrum (Jan 02 2020 at 14:28, on Zulip):

I think we don't guarantee that &mut T must satisfy the safety invariant, right?

simulacrum (Jan 02 2020 at 14:28, on Zulip):

(Obviously we can't do anything about safety optimization wise anyway)

gnzlbg (Jan 02 2020 at 14:29, on Zulip):

No, I don't think so.

// I woke up today and it is only safe to create an OnlyOne(1) value.
struct OnlyOne(i32);

let mut x: OnlyOne;
let y = &mut x.0;
simulacrum (Jan 02 2020 at 14:29, on Zulip):

with that in mind, we could presumably make fn drop_in_place(&mut T) safe, right?

simulacrum (Jan 02 2020 at 14:29, on Zulip):

that would sort of fully answer all the questions

simulacrum (Jan 02 2020 at 14:29, on Zulip):

although, actually, no

simulacrum (Jan 02 2020 at 14:29, on Zulip):

since we want to guarantee it's called just the once, right?

gnzlbg (Jan 02 2020 at 14:29, on Zulip):

nono

gnzlbg (Jan 02 2020 at 14:30, on Zulip):

Safe code cannot violate the safety invariant

gnzlbg (Jan 02 2020 at 14:30, on Zulip):

In unsafe code, you can violate the safety invariant, as long as you restore it at the safe code boundary

simulacrum (Jan 02 2020 at 14:30, on Zulip):

okay, sure, though that boundary is sort of murky I think

gnzlbg (Jan 02 2020 at 14:31, on Zulip):

So, e.g.,

impl OnlyOne {
    fn new() -> Self { Self(1) } // safe
    unsafe fn new_unchecked(x: i32) -> Self { Self(x) }
}
simulacrum (Jan 02 2020 at 14:31, on Zulip):

A concrete proposal for drop_in_place then is:

gnzlbg (Jan 02 2020 at 14:32, on Zulip):

There, it would be "ok" for new_unchecked to look like this:

unsafe fn new_unchecked(x) -> Self {
    let mut a = OnlyOne(42); // valid but unsafe
    a.0 = x;
    a  // Ok, returns a potentially unsafe OnlyOne
}
simulacrum (Jan 02 2020 at 14:32, on Zulip):

sure, yes

simulacrum (Jan 02 2020 at 14:33, on Zulip):

though that could be a safe function?

simulacrum (Jan 02 2020 at 14:33, on Zulip):

oh, you dropped the assert, okay

gnzlbg (Jan 02 2020 at 14:33, on Zulip):

yeah, sorry, the assert was making the unsafe pointless

gnzlbg (Jan 02 2020 at 14:34, on Zulip):

In the issue you posted they discuss about adding a mem::drop_in_place(&mut T) API

gnzlbg (Jan 02 2020 at 14:34, on Zulip):

We could do that, and maybe deprecate the ptr:: one, or at least warn, don't know

simulacrum (Jan 02 2020 at 14:35, on Zulip):

I think there's not much point

gnzlbg (Jan 02 2020 at 14:35, on Zulip):

I mean, it is not UB to call it twice

simulacrum (Jan 02 2020 at 14:35, on Zulip):

A concrete proposal for drop_in_place then is:

gnzlbg (Jan 02 2020 at 14:36, on Zulip):

Yes, we should expand the docs on why the last point is true

gnzlbg (Jan 02 2020 at 14:41, on Zulip):

ptr::drop_in_place calls Drop::drop which is a safe API that is allowed to leave the value in a potentially _unsafe_ state (depending on the particular Drop implementation). All safe Rust APIs, including Drop::drop require their arguments to be in a _safe_ state, and this is why for some Drop implementations, double drops are unsound.

gnzlbg (Jan 02 2020 at 14:42, on Zulip):

AFAICT, because Drop::drop is safe, then &mut self must point to a self that's in a safe state, and if it isn't, the program is unsound. Whether you can double drop those types would then depend on whether Drop::drop breaks the safety invariant or not.

simulacrum (Jan 02 2020 at 14:43, on Zulip):

Drop::drop being safe doesn't matter

simulacrum (Jan 02 2020 at 14:44, on Zulip):

oh, safe state, then yes

gnzlbg (Jan 02 2020 at 14:44, on Zulip):

It kind of does, because safe functions don't have a # Safety clause.

simulacrum (Jan 02 2020 at 14:44, on Zulip):

though Drop::drop is notable in being able to violate the safety invariant on exit

gnzlbg (Jan 02 2020 at 14:45, on Zulip):

Only if it takes ownership of the value

gnzlbg (Jan 02 2020 at 14:45, on Zulip):

If you want to drop a value without taking ownership, then you need to use ptr::drop_in_place, which is unsafe for this reason.

gnzlbg (Jan 02 2020 at 14:45, on Zulip):

Typically this would be expressed by making Drop::drop unsafe..

simulacrum (Jan 02 2020 at 14:46, on Zulip):

Drop::drop is special and basically unsafe

gnzlbg (Jan 02 2020 at 14:47, on Zulip):

Yeah

gnzlbg (Jan 02 2020 at 14:47, on Zulip):

I'm not sure why we need ptr::drop_in_place(ptr) at all

simulacrum (Jan 02 2020 at 14:47, on Zulip):

Where one of the safety constraints is "don't call me again" by default, though some Drop::drop are less strict

gnzlbg (Jan 02 2020 at 14:47, on Zulip):

can't we just use unsafe { Drop::drop(&mut v) } ?

simulacrum (Jan 02 2020 at 14:48, on Zulip):

hm, well, maybe, but you can't actually call Drop::drop, the compiler won't let you

simulacrum (Jan 02 2020 at 14:48, on Zulip):

(maybe it should)

gnzlbg (Jan 02 2020 at 14:49, on Zulip):

yes, maybe

gnzlbg (Jan 02 2020 at 14:49, on Zulip):

There are some other parts of the language were unsafe is too "general"

gnzlbg (Jan 02 2020 at 14:49, on Zulip):

And what we want to express is unsafe(to_call_explicitly) fn drop(&mut self) { ... }

simulacrum (Jan 02 2020 at 14:50, on Zulip):

hm, why'd we want that? It's just unsafe period

simulacrum (Jan 02 2020 at 14:51, on Zulip):

you can do unsafe things inside it

gnzlbg (Jan 02 2020 at 14:51, on Zulip):

Well because we don't want unsafe code inside them by default

gnzlbg (Jan 02 2020 at 14:51, on Zulip):

e.g.

unsafe fn drop(&mut self) { ...raw ptr deref is ok... }
simulacrum (Jan 02 2020 at 14:51, on Zulip):

sure

gnzlbg (Jan 02 2020 at 14:52, on Zulip):

I think there was an issue about requiring explicit unsafe blocks in unsafe fn, I guess for drop that would also solve it

rkruppe (Jan 02 2020 at 15:56, on Zulip):

Drop::drop is not (primarily) unsafe to call, it's never sensible to call (outside of the compiler-generated drop glue). Drop::drop only handles custom per-type drop logic, not the recursive dropping of fields. So it can't replace ptr::drop_in_place and there's also no other reason to allow calling it manually.

simulacrum (Jan 02 2020 at 15:57, on Zulip):

ah, that's a good point

simulacrum (Jan 02 2020 at 15:57, on Zulip):

I forgot about the recursive bit

RalfJ (Jan 02 2020 at 16:40, on Zulip):

For a type like i32, a double drop is not UB, according to the model being discussed here

I agree, if we assume that uninit memory is never allowed for i32. But as you like to note yourself, it is not legal for code to rely on something being UB -- we might allow uninit ints in the future.

RalfJ (Jan 02 2020 at 16:42, on Zulip):

I mean, if ptr::drop_in_place(ptr) changes the state of the memory behind ptr, it needs to do so by "writting" to that ptr "somehow"

well it could do *(ptr as *mut u8) = 3, for example, so I don't think there is anything "automatic" here

RalfJ (Jan 02 2020 at 16:43, on Zulip):

but IMO the safety contract of drop should prohibit that

RalfJ (Jan 02 2020 at 16:43, on Zulip):

(and that is the only user-defined part of drop_in_place, after all)

RalfJ (Jan 02 2020 at 16:44, on Zulip):

oh, sorry, discussion went on after these things but Zulip failed to load it... never mind then

simulacrum (Jan 02 2020 at 16:48, on Zulip):

@RalfJ I think @gnzlbg and I agreed on this:

A concrete proposal for drop_in_place then is:

simulacrum (Jan 02 2020 at 16:48, on Zulip):

which I believe is consistent with today's world

RalfJ (Jan 02 2020 at 16:55, on Zulip):

well, drop_in_place is compiler-implemented, so we can't really require anything for it as much as guarantee that it has certain properties

RalfJ (Jan 02 2020 at 16:56, on Zulip):

the only place we have for making requirements is Drop::drop

RalfJ (Jan 02 2020 at 16:57, on Zulip):

not sure how I feel about it taking &mut, I guess that could be correct but it seems weird.
this reminds me, I think I can remove the real_drop_in_place again that I added a while back as a hack for earlier versions of Stacked Borrows...

simulacrum (Jan 02 2020 at 16:59, on Zulip):

ah, yes, these would be the guarantees, sure

simulacrum (Jan 02 2020 at 16:59, on Zulip):

I think these all apply as requirements to Drop::drop too

simulacrum (Jan 02 2020 at 16:59, on Zulip):

(where we substitute call for "cause to be called" or so)

RalfJ (Jan 02 2020 at 17:00, on Zulip):

well the requirements are stronger there I think

RalfJ (Jan 02 2020 at 17:00, on Zulip):

like, it must leave the fields in a "drop-valid" state... that's weaker than "safety invariant" but stronger than "validity invariant", e.g. it is wrong for drop to leave a field of type Vec in a state that is valid but unsafe (and leads to the later Vec::drop failing)

simulacrum (Jan 02 2020 at 17:01, on Zulip):

ah, yes, that's true

RalfJ (Jan 02 2020 at 17:02, on Zulip):

we could approximate this with "safety invariant of each field" but maybe there's good use for violating that after dropping

simulacrum (Jan 02 2020 at 17:03, on Zulip):

well -- one could imagine -- e.g., resetting some sort of Once primitive on drop so it can be reused which is technically unsafe (code relies on it being once only), but since you know you're about to drop it it's fine

simulacrum (Jan 02 2020 at 17:03, on Zulip):

hard to come up with examples :)

gnzlbg (Jan 03 2020 at 13:42, on Zulip):

@RalfJ agreed with all you said.

I agree, if we assume that uninit memory is never allowed for i32. But as you like to note yourself, it is not legal for code to rely on something being UB -- we might allow uninit ints in the future.

I'm not sure I understand this claim. If we allow i32 to become uninit, why wouldn't it be ok to double-drop it?

RalfJ (Jan 03 2020 at 17:22, on Zulip):

ah I guess dropping uninit would also be fine then... never mind then

gnzlbg (Jan 03 2020 at 19:36, on Zulip):

What would be the best place to document this? Should I open an issue in rust-lang/rust to discuss how to add this to the Drop trait docs ?

RalfJ (Jan 07 2020 at 20:44, on Zulip):

I guess that would be a good place? is there anything drop-related in the reference or the nomicon?

RalfJ (Jan 15 2020 at 12:02, on Zulip):

@gnzlbg something very related just came up on reddit: https://www.reddit.com/r/rust/comments/eowihi/what_invariants_must_hold_to_access_an_objects/

RalfJ (Jan 15 2020 at 12:02, on Zulip):

is there anywhere that we can point them to, currently?

gnzlbg (Jan 15 2020 at 13:05, on Zulip):

I think that issue is simpler

gnzlbg (Jan 15 2020 at 13:06, on Zulip):

since Rc is essentially a (raw_ptr, usize, usize), and the implementation of Drop for it does a drop_in_place of the content in the memory behind the raw_ptr, and then modifies the counts

gnzlbg (Jan 15 2020 at 13:06, on Zulip):

Only the last Rc being dropped has an extra step to free that memory

gnzlbg (Jan 15 2020 at 13:07, on Zulip):

That is, I think that users is making the incorrect assumption that calling drop_in_place(raw_ptr) somehow drops the RC?

gnzlbg (Jan 15 2020 at 13:08, on Zulip):

But it does not, it drops the value of whatever object is in the memory behind raw_ptr, which is not the Rcobject itself

gnzlbg (Jan 15 2020 at 13:08, on Zulip):

That's why accessing the counts afterwards is sound

gnzlbg (Jan 15 2020 at 13:09, on Zulip):

The thorny issue would be whether accessing those counts through the &mut Rc<T> inside the drop function is sound

gnzlbg (Jan 15 2020 at 13:10, on Zulip):

(deleted)

gnzlbg (Jan 15 2020 at 13:10, on Zulip):

(deleted)

gnzlbg (Jan 15 2020 at 13:14, on Zulip):

@RalfJ I think we should just document this in the API docs of Drop

gnzlbg (Jan 15 2020 at 13:16, on Zulip):

Document that ptr::drop_in_place(ptr: *mut T):

* requires exclusive access to ptr (as if it were an &mut T)
* must probably leave the memory behind pointer in a valid state

gnzlbg (Jan 15 2020 at 13:16, on Zulip):

and I say probably because I'm not sure, the validity is only required if you somehow use the value afterwards:

gnzlbg (Jan 15 2020 at 13:18, on Zulip):

e.g.

let ptr = GlobalAlloc::alloc(..big enough for T..);
ptr.write(...some T value...);
ptr::drop_in_place(&*ptr.field_of_T);
forget(ptr); // leaked
gnzlbg (Jan 15 2020 at 13:20, on Zulip):

There AFAICT it wouldn't matter if drop_in_place leaves a field of T in an invalid state, because there is no reference to that value anywhere after the call returns, only a raw pointer, which gets leaked, meaning nobody can access the memory anymore

Lokathor (Jan 15 2020 at 16:15, on Zulip):

you don't need to forget the pointer

RalfJ (Jan 15 2020 at 16:17, on Zulip):

must probably leave the memory behind pointer in a valid state

"will leave", I assume? this is documenting the guarantees libcore provides to the user. any "must" clause should be added to Drop::drop.

gnzlbg (Jan 15 2020 at 16:18, on Zulip):

ptr::drop_in_place calls user-defined Drops, so I don't see how libcore could make that guarantee, unless we require that all Drop implementations provide it

gnzlbg (Jan 15 2020 at 16:20, on Zulip):

I think the best place to document all this is the Drop trait, the only thing ptr::drop_in_place might want to document is that the raw pointer will be used as a &mut T, and maybe mention potential pitfalls like the one being discussed here, but all of that should also be explained in the Drop trait

RalfJ (Jan 15 2020 at 18:38, on Zulip):

ptr::drop_in_place calls user-defined Drops, so I don't see how libcore could make that guarantee, unless we require that all Drop implementations provide it

indeed, Drop is the place where we will have to require whatever we need to guarantee what we document at drop_in_place

RalfJ (Jan 15 2020 at 18:39, on Zulip):

but having requirements at drop_in_place makes no sense -- as little sense as it would for any other stdlib function

RalfJ (Jan 15 2020 at 18:39, on Zulip):

I think the best place to document all this is the Drop trait, the only thing ptr::drop_in_place might want to document is that the raw pointer will be used as a &mut T, and maybe mention potential pitfalls like the one being discussed here, but all of that should also be explained in the Drop trait

I agree it makes sense to centralize the docs

Last update: Jan 21 2020 at 08:15UTC