Are Copy implementations semantically relevant (besides specialization)?

The question comes basically from https://users.rust-lang.org/t/is-this-undefined-behaviour/67886

I’m wondering whether the fact that a type A implements A: Copy or not can make a semantic difference, i.e.: can code using the struct that compiles either way, with the implementation or without it, behave differently depending on that impl alone? In particular, can it trigger UB in only one of the cases? (I do not consider any code that specializes on a T: Copy bound, because then the bound can trivially make a difference.)

The canonical code in mind is something like

#[derive(Clone)]
struct A(i32);

impl Copy for A {}

fn main() {
    let x = A(42);
    let p: *const A = &x;
    drop(x);
    println!("{}", unsafe { (*p).0 });
}

which is obviously sound; compared to

#[derive(Clone)]
struct A(i32);

// REMOVED
// impl Copy for A {}

fn main() {
    let x = A(42);
    let p: *const A = &x;
    drop(x);
    println!("{}", unsafe { (*p).0 });
}

Does the latter code example trigger UB? (Miri doesn’t think it does.)

As long as there are no destructors involved, then it's fine.

I think as soon as something in the struct has a destructor, even if it does nothing, then it's UB, because the drop glue is compiler generated, and there's no guarantee that it won't cause problems running it twice.

@Diggsey

@Diggsey Can you cite some official documentation that backs up that claim? According to the relevant parts of the Reference, it is not true at all, because:

Moving from a local variable of a non-Copy type renders its place uninitialized, as per the Reference:

If the type of that value implements Copy, then the value will be copied. In the remaining situations, if that type is Sized, then it may be possible to move the value.
[…]
After moving out of a place expression that evaluates to a local variable, the location is deinitialized and cannot be read from again until it is reinitialized.
Reading an uninitialized place is Undefined Behavior, as per the Reference:
Rust code is incorrect if it exhibits any of the behaviors in the following list:
[…]
- Producing an invalid value […]:
  - An integer (i*/u*), floating point value (f*), or raw pointer obtained from uninitialized memory
In other words, the only cases in which reading uninitialized memory is permitted are inside unions and in "padding" (the gaps between the fields/elements of a type).

In this situation, we clearly have a non-Copy local variable, which:

has been moved from;
has not been re-initialized;
is not inside a union;
is not (only) the padding of a struct type; and
transitively produces an uninitialized integer value.

Considering all of the above circumstances, how and why does this not cause UB?

Wouldn't saying this isn't UB prevent some potential optimisations?

My strong opinion is that it should be UB, but the doubt comes from the fact that Miri isn't able to detect it

Miri can't and officially isn't supposed to be able to detect every kind of UB. In other words:

If MIRI says it's UB, it definitely is UB.
If MIRI doesn't say it's UB, it might either be fine or it might be UB of which the detection is hard/impossible/impractical, intentionally unimplemented, or a missing feature/a bug in MIRI.

In particular, the README of MIRI explicitly mentions that uninitialized places aren't being checked for now:

Miri tries to avoid false positives here, so if your program runs fine in Miri right now that is by no means a guarantee that it is UB-free when these questions get answered.

In particular, Miri does currently not check that integers/floats are initialized or that references point to valid data.

@H2CO3

@H2CO3

Moving from a local variable of a non-Copy type renders its place uninitialized, as per the Reference:

It doesn't say that, it says "deinitialized". I believe the reference is referring to a different kind of initialization, not whether the memory is uninitialized. (Which would be very weird to call out in documentation about safe code, since safe code cannot observe uninitialized memory)

In the original example, there are a couple of things in play that make things more complicated though. Let's look at this simpler example first:

#[derive(Clone)]
struct A(i32);

fn main() {
    let x = A(42);
    let p: *const A = &x;
    let _ = std::ptr::read(p);
    let _ = std::ptr::read(p);
}

This is still moving out of the same address multiple times (3 technically), and is perfectly fine as long as A does not have a destructor or recursively contain any fields with destructors. When you read from a memory location, the original bit pattern is unchanged, and so the memory is still bit-pattern valid for the type A. It doesn't suddenly become uninitialized memory.

The difference between this and the first example is that the local variable is still "live" at the points where it is read. One thing I hadn't considered in my previous answer was that in the first example the local variable is no longer live when it is read via the pointer, which could be UB, but I'm not sure if the compiler is allowed to reason in that way when there is an escaped pointer to it?

@Diggsey

@Diggsey

It doesn't say that, it says "deinitialized"

The prefix "de-" is for expressing the act of undoing something. What state is a place in, after it has been "deinitialized"? Surely it isn't initialized. Furthermore, the quoted excerpt from the Reference explicitly states that it "cannot be read from again until it is reinitialized".

a different kind of initialization, not whether the memory is uninitialized

What other kinds of initialization are there? What else can be initialized or uninitialized, if not places?

Which would be very weird to call out in documentation about safe code, since safe code cannot observe uninitialized memory

The code in question uses unsafe to observe uninitialized memory. Anyway, it's not weird to point that out at all – it explains why the compiler rejects safe code that tries to access a moved-from variable anyway.

When you read from a memory location, the original bit pattern is unchanged.

But initialization is not about bit patterns. See e.g. [1]. Uninitialized memory is not allowed to be read, no matter what bit pattern happens to physically inhabit it.

I'm not sure if the compiler is allowed to reason in that way when there is an escaped pointer to it?

I don't see why the compiler would have such an obligation. By destroying the value, any pointers pointing to it become dangling and must not be used for reading the referent. The "Safety" section for the documentation of the std::ptr module is very explicit about this constraint:

Even for operations of size zero, the pointer must not be pointing to deallocated memory, i.e., deallocation makes pointers invalid even for zero-sized operations.

and later,

The result of casting a reference to a pointer is valid for as long as the underlying object is live

Accordingly, the equivalent code with borrow-checked references does not compile either.

Let's look at this simpler example first:
[…]
This is still moving out of the same address multiple times (3 technically), and is perfectly fine

It doesn't move out of the place, though: the documentation for ptr::read() says that it:

Reads the value from src without moving it.

(Emphasis mine.) Furthermore, since ptr::read() is implemented via an intrinsic, it is entirely plausible that it is not equivalent with directly moving out of the pointed place. In fact my interpretation is that they must not be equivalent, in the sense that a place which was moved from is uninitialized and invalidated (because that's what the spec says), meanwhile a place which wasptr::read() from produces the same value as if it was moved from, but it leaves the place initialized, because that's what its documentation says (and, of course, it requires the caller to dynamically uphold the invariant that at most one such copy of the value will ever be destroyed).

If ptr::read() is supposed to be equivalent to a move, then both its documentation and the reference are at error, because in that case, neither "reading an uninitialized place is UB" nor "ptr::read() does not move the referent" is true. I find that to be highly unlikely.

Furthermore, the quoted excerpt from the Reference explicitly states that it "cannot be read from again until it is reinitialized".

Which doesn’t say “must not be read from again”, as it’s merely describing what’s possible in safe code. Obviously, rust has some form of initialization-check for variables; for the purpose of describing this check, uninitialized variables are the same as initialized-then-deinitialized ones, hence the terminology makes sense for describing how safe code behaves.

What other kinds of initialization are there? What else can be initialized or uninitialized, if not places?

Well, you can e.g. move out of a ManuallyDrop<Foo> (with the take() method); this doesn’t mean that it leaves behind uninitialized memory. You can still soundly leave the – logically deinitialized (but this is not checked by the compiler because ManuallyDrop’s API is unsafe) – ManuallyDrop<Foo> in, say, a niche-optimized Option (i.e. have Option<ManuallyDrop<Foo>>) and there’s no need to fear the deinitialized ManuallyDrop<Foo> to entail uninitialized memory, arbitrary bit-patterns, breaking the surrounding Option.

And AFAICT, when you partially deinitialize some enum, the compiler will still insert code to drop the remaining fields, and that code first reads the discriminant of the enum again; so the moved-out fields are definitely not uninitialized memory, otherwise you couldn’t trust the discriminant anymore if there’s niche optimization. (At least that’s the impression I get reading the MIR generated by this code.)

The remainder of what you’re writing seems to assume that the “deinitialized” wording refers to “uninitialized memory”, so much of it doesn’t add too much to the discussion – I think we all agree that if moving out of a variable really leaves behind uninitialized memory, then accessing the variable after it has been moved out of is UB. I personally think it’s highly likely that the cited portion of the reference using the “deinitiaized” wording does not have the intention to say anything about unsafe code or uninitialized memory, but simply explains the initialization-checker logic (or whatever it’s officially called) that applies to safe code; it specifies neither that the code in question in this thread is UB nor that it isn’t UB.

Also there’s of course reasons why valid code cannot duplicate values that don’t implement Copy. Even if it’s not instant UB in and by itself, it’s unsound. You can break safety-invariants; you can create aliasing Boxes, etc... everything without assuming that moving a variable leaves uninitialized memory behind. Hence e.g. the observation that

the equivalent code with borrow-checked references does not compile either

doesn’t really demonstrate anything.

I do find the wording on the ptr::read method interesting. I don’t know whether “without moving it” is a technical term and what exactly “moving” means in this context. This might also just be an old, informal, description of what ptr::read does. (Looking up the relevant commit...) Ah, it’s super old, so maybe the phrasing came from (or at least evolved from phrasing from a time) when moving a value still modified some drop-flags contained within the value itself.

Edit: Reading the relevant discussion again, it seems like the wording might in-fact not refer to such drop-flags. Really, I’m not sure of the history of those and when exactly they were removed. Maybe it’s just come about from lots of other functions specifying that destructors aren’t run, too, here, and the fix to “without moving” it was simply suboptimal and never improved. Even in the PR at the time, there’s a suggestion to instead say “Copies the value behind the pointer regardless of whether it is Copy”.

you can e.g. move out of a ManuallyDrop (with the take() method); this doesn’t mean that it leaves behind uninitialized memory.

I don't see how this counts as a different kind of initialization. ManuallyDrop is for managing memory, isn't it? The documentation of its take() method even says that:

This function semantically moves out the contained value

which means that it is intended to be equivalent with a regular move. Furthermore, it also asserts that

It is your responsibility to ensure that this ManuallyDrop is not used again.

Now, if a take()n instance of ManuallyDrop must not be "used" again, then I must assume it's about not using the wrapped value. Obviously, not being able to touch the ManuallyDrop instance itself ever again would render it useless. Moreover, it is a lang_item, and its very purpose is exactly to prevent some behavior that would be automatic otherwise (namely, dropping), so it looks very much like the intention here is that once you take it, then the wrapper itself remains valid to own, but the inner value must not be accessed. (This behavior would not be possible for a regular newtype wrapper.)

But this in turn means that take()ing the inner value leaves it in a state in which it must not be used, while still allowing sound ownership of the wrapper, exactly the purpose of this type. So I cannot fathom how moving out of the inner value differs from moving out of a variable. This seems to be exactly the same kind of initialized/uninitialized distinction.

This seems to be exactly the same kind of initialized/uninitialized distinction.

The “uninitialized” concept here does not mean “uninitialized memory”. You can’t put uninitialized memory into the Some-variant of an Option<bool>, because its size is 1, and is uses some of the bit-patterns not used by the bool to indicate that the Option is not None; hence something like testing which enum variant an Option<ManuallyDrop<bool>> is reads the whole value, no matter whether the ManuallyDrop was already deinitialized or not. Not without reason does the MaybeUninit wrapper close niches.

(I know a bool implements Copy, but the same things are true for non-Copy types with niches.)

I see, that is a fair point. In this case, then, it should be clarified which of the two notions of "uninitialized" is involved with moving from a place. (And ideally, they should start having distinct names, too.)

(Regardless, I would still find it inconsistent with everything else about the memory model if the original piece of code were valid.)

Furthermore, it also asserts that

It is your responsibility to ensure that this ManuallyDrop is not used again.

Now, if a take()n instance of ManuallyDrop must not be "used" again, then I must assume it's about not using the wrapped value.

Note that the standard library docs sometimes document invariants that are necessary to uphold to remain sound when working with arbitrary types. Not everything that Safety comments say you should not do automatically invokes instant UB when it’s done anyways.

E.g. the documentation of Pin goes at great lengths explaining things you “must not” do to pinned values. But all it really does is clearly state some common “Interface” of library-safety-guarantees. It doesn’t specify any language-UB that you would always automatically and immediately run into whenever you violate any of those rules. When working generically or with not further documented external types, that’s the conditions you need to ensure for sound code. But if you’re pinning your own types, you might as well still move them around as much as you like after pinning them, even if you decide not to implement Unpin for whatever reason.

I see, that is a fair point. In this case, then, it should be clarified which of the two notions of "uninitialized" is involved with moving from a place. (And ideally, they should start having distinct names, too.)

(Regardless, I would still find it inconsistent with everything else about the memory model if the original piece of code were valid.)

I fully agree with the notion that the original piece of code feels off, and that it might be UB. But not because of any reason involving actual “uninitialized memory” 😃

The canonical code in mind is something like

This is basically the same question as #188, I think (since drop(x) in your example incurs a move).

Also, another aspect of this program that is not properly specified is the exact lifetime of the local variable x: if the StorageDead for x is put right after drop, your program has a use-after-free.

But initialization is not about bit patterns.

Well it is, in the sense that this distinct notion of "uninitialized memory" was created because it's convenient for compiler optimizations to talk about memory whose bit pattern is ill-defined for some reason. The bit pattern left behind after a move is very well defined, so there's no reason to define the "move" operation as leaving behind uninitialized memory.

It doesn't move out of the place, though: the documentation for ptr::read() says that it:

Reads the value from src without moving it.

IMO that documentation is just wrong. I believe it's trying to say that the original value is left unchanged in memory, but ptr::read is by all definitions a move in Rust semantics, because an owned value is returned, not a reference, and it's not creating a new value.

Also, another aspect of this program that is not properly specified is the exact lifetime of the local variable x: if the StorageDead for x is put right after drop, your program has a use-after-free.

Yeah, this what I was trying (poorly) to explain earlier, and why I brought up the ptr::read example.

This is basically the same question as #188, I think (since drop(x) in your example incurs a move).

Ah, there we go! I tried searching for existing threads mentioning Copy, but this one doesn't mention it in the title, and I spent almost no time looking at issues without "copy" in the title.

Overall, this still being an open question means that while miri doesn't complain, one should avoid code like this because it's not yet certain that it won't be UB, right?

ptr::read is by all definitions a move in Rust semantics, because an owned value is returned, not a reference, and it's not creating a new value.

Define "move".
Certainly, as far as the operational semantics are concerned, ptr::read is just a copy. The only time a "move" happens in the operational semantics is when a MIR move operand is evaluated.

So while I would agree that "conceptually", when verifying Rust code, ptr::read could be considered a 'move' -- that is irrelevant here when we are discussing UB, i.e., a question of the operational semantics. And there, ptr::read is a copy.

Overall, this still being an open question means that while miri doesn't complain, one should avoid code like this because it's not yet certain that it won't be UB, right?

Yes.

Define "move".
Certainly, as far as the operational semantics are concerned, ptr::read is just a copy

AIUI: in the operational semantics, there is no difference between a copy and a move, they are the same.

The distinction only exists "conceptually" where copies are the subset of moves that leave the source in an initialized state.

So it's wrong whether it's talking about operational semantics, or conceptually, since in both cases it's still a move (but in the former is also a copy).

(And I would argue that it would make more sense for the docs for ptr::read to be talking about the "conceptual" move, since to use the function, you need to know whether the source can still be used, and with ptr::read it cannot unless the type is Copy)

@RalfJung

Is there anything describing the operational semantics of move in MIR somewhere?
There's KRust, but it's a bit obscure, I don't think lambda-rust models it in the operational semantics (@RalfJung ?), and neither does MIRI.

AIUI: in the operational semantics, there is no difference between a copy and a move, they are the same.

This is not at all clear, so please don't make such definite statements (EDIT: I was told that AIUI weakens the statement, I didn't understand the acronym). Discussing this question is exactly the point of #188 which I linked above.

One possible option is that a move replaces the source operand with uninit memory.

A mutable reference &mut T is specifically not Copy despite not having any drop glue, since it's not allowed to be aliasing; how does that factor into this discussion?

Navigation Menu

Repository navigation

Are `Copy` implementations semantically relevant (besides specialization)? #307

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Are Copy implementations semantically relevant (besides specialization)? #307

Description

Activity

Diggsey commented on Nov 25, 2021

H2CO3 commented on Nov 25, 2021

giltho commented on Nov 25, 2021

H2CO3 commented on Nov 25, 2021

Diggsey commented on Nov 25, 2021

H2CO3 commented on Nov 25, 2021

steffahn commented on Nov 25, 2021

H2CO3 commented on Nov 25, 2021

steffahn commented on Nov 25, 2021

H2CO3 commented on Nov 25, 2021

steffahn commented on Nov 25, 2021

steffahn commented on Nov 25, 2021

RalfJung commented on Nov 26, 2021

Diggsey commented on Nov 26, 2021

steffahn commented on Nov 26, 2021

RalfJung commented on Nov 26, 2021

Diggsey commented on Nov 26, 2021

giltho commented on Nov 26, 2021

RalfJung commented on Nov 26, 2021

sollyucko commented on Jan 13, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Are `Copy` implementations semantically relevant (besides specialization)? #307