Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clarify what is UB #149

Merged
merged 21 commits into from
Aug 16, 2019
Merged

clarify what is UB #149

Changes from 14 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
dfb93b2
UB
RalfJung Jul 20, 2019
1dcafde
list more ptr offset computations
RalfJung Jul 20, 2019
280761a
NonNull, NonZero*
RalfJung Jul 20, 2019
86d9e2c
Define 'producing'
RalfJung Jul 20, 2019
3241c00
handle recursion in the heading
RalfJung Jul 20, 2019
909b14c
subsume the NonNull things as library types
RalfJung Jul 20, 2019
f59eca2
be more precise about dangling
RalfJung Jul 20, 2019
738a338
stick to broader UB for raw ptr offsets/derefs for now
RalfJung Jul 21, 2019
0f51082
avoid redundant UB
RalfJung Jul 21, 2019
efb5086
add more cases of UB
RalfJung Jul 24, 2019
df5ff63
mention !
RalfJung Jul 24, 2019
c41d492
explain when metadata is invalid
RalfJung Jul 24, 2019
1f613d8
Apply suggestions from code review
RalfJung Jul 24, 2019
c73730b
raw ptrs must be initialized like integers
RalfJung Jul 26, 2019
bd6215e
resolve some nits
RalfJung Jul 28, 2019
7386b5c
refactor null a bit
RalfJung Jul 28, 2019
864625f
fold uninit integer rule with reading uninit memory
RalfJung Jul 29, 2019
64bf0a5
fix def.n of dangling
RalfJung Jul 29, 2019
86a89ae
clarify dangling
RalfJung Jul 31, 2019
c5778a1
drop parenthetical
RalfJung Aug 16, 2019
7703c18
some edits
RalfJung Aug 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions src/what-unsafe-does.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,40 @@ to your program. You definitely *should not* invoke Undefined Behavior.
Unlike C, Undefined Behavior is pretty limited in scope in Rust. All the core
language cares about is preventing the following things:

* Dereferencing null, dangling, or unaligned pointers
* Dereferencing (using the `*` operator on) null, dangling, or unaligned
pointers, or fat pointers with invalid metadata (see below)
* Reading [uninitialized memory][]
* Breaking the [pointer aliasing rules][]
* Producing invalid primitive values:
* dangling/null references
* null `fn` pointers
* Producing invalid primitive values (either alone or as a field of a compound
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
type such as `enum`/`struct`/array/tuple):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of our bickering about unions does actually wrap back around to this point:

is union { a: bool; b: bool; } = 3 "producing an invalid value as a field of a compound type"?

(we can probably gloss over this, but it is something to make clearer when we have a better answer)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The answer is "we don't know yet, see rust-lang/unsafe-code-guidelines#73".

So yes, this is a good question, and one that I would prefer we could skip over for now.

* a `bool` that isn't 0 or 1
* an undefined `enum` discriminant
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
* null `fn` pointers
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
* a `char` outside the ranges [0x0, 0xD7FF] and [0xE000, 0x10FFFF]
* A non-utf8 `str`
* a `!` (all values are invalid for this type)
* dangling/null/unaligned references, references that do themselves point to
invalid values, or fat references (to a dynamically sized type) with
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
invalid metadata
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
* slice metadata is invalid if the slice has a total size larger than
`isize::MAX` bytes in memory
* `dyn Trait` metadata is invalid if it is not a pointer to a vtable for
`Trait` that matches the actual dynamic trait the reference points to
* a non-utf8 `str`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reword for consistency:

  • a str that isn't valid utf8

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe we want to skip this entirely because this is just a library invariant?

* an uninitialized integer (`i*`/`u*`), floating point value (`f*`), or raw
pointer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to think about this precise wording. I think you can break this out of here and fold it into "reading uninitialized memory". I want to have some wording along the lines of "you can't tell the compiler a value definitely exists where only uninitialized memory resides", but this needs to be reinforced by documentation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a type-based property though. The following is fine:

let x: MaybeUninit<u8> = mem::uninitialized();

but the following is not:

let x: u8 = mem::uninitialized();

I would actually remove the "Reading uninitialized memory" clause in favor of this type-based one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave this a shot, let me know what you think.

Copy link
Contributor

@Gankra Gankra Aug 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so i get what you're saying but two things:

A) Pedantic aside that probably doesn't matter: do we actually care about guaranteeing let x: MaybeUninit<u8> = mem::uninitialized(); is valid? I suppose that's important if you want to be able to cast a pointer to a fresh allocation to be &mut MaybeUninit and not do dummy writes of uninit() to be able to call its methods? Hmm... (sub-aside: i'm a bit uncomfortable with technical discussion appealing to mem::uninitialized since it's busted in a bunch of ways, so it doesn't actually have a coherent meaning. Fresh allocations, however, work fine as well-defined uninitialized memory the compiler knows about.)

B) I was using a subtler definition of "a value exists", which agrees with your statement that it's type-based. Basically empty types don't have values; they can't. So by having a union { (), T } you are, with types, telling the compiler that it may always be the case that a value doesn't exist there (but it's not opaque like asm or volatile, so the compiler can clearly observe reads and writes and reason about the memory's bytes).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, my definition of empty things being valueless fails to handle !

Copy link
Contributor

@Gankra Gankra Aug 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's some Proposed Terminology for the sake of this conversation. This is not necessarily useful for a formal model, but it is helping me make sense of the concepts in my head:

  • a value is a property a byte or collection of bytes has. Bytes may have the secret 257th value of "uninitialized".

  • An instance of a type is something you get from constructing that type. Moving or destroying an instance relinquishes a piece of code's access to that instance. Those instances do not necessarily have a unique, tracked, identity, but their ability to reach a point in the program serves as a kind of proof (more on that below). The Copy trait just gives permission to all code that has acquired an instance to mint more instances with the same value.

I am not 100% comfortable with the notion of "construction", as room must be made for transmutes, plain-old-data, and deserializers. I think this basically boils down to a similar situation as double-drops where any type is freely allowed to assume its instances aren't double-dropped with UB at stake, but a type may randomly decide it's fine to be double-dropped. There's no trait/property for this, it's just a thing a type can randomly be fine with. It's very contrived to come up with such a type, so we don't care about this much outside of the very specific niche of standards and trivia.

Similarly, types must be able to assume that there is meaning to the existence of an instance, also with UB at stake. But types may allow instances to be minted without any constructor. Which is to say, for some types it may be fine to have more instances of a type "live" in a program than the number of its constructors that have run. This is a relatively important fact about a type, but just like with doubledropabble it's not something with a standardized trait/property. (perhaps one can just appeal to access to a type's record initialization syntax as permission to mint as many instances as one pleases without even using that syntax?)

Anyway.

Certain types may have forbidden values, indicating that if a piece of code claims to have an instance of a type, the memory that instance occupies must not have certain values.

Zero-sized types do not have values, but they do still have instances. A logical consequence of this is that we cannot have the concept of a ZST with a forbidden value. Nor can we speak of an "uninitialized" ZST. It doesn't make sense.

From this, I would like to assert that ! does not in fact fall under the umbrella of forbidden values: it is simply forbidden to claim that you have an instance of !, because it does not have a constructor. If you claim to have an instance of !, you are lying, and we have made that lie unconditional UB (as is our right as type authors).

Some additional context, from the practical perspective:

In the stdlib we frequently appeal to the idea that, logically speaking, there is always an instance of a zero-sized type everywhere you look. So e.g. (73 as *const ()).read() is not UB ("you can pull instances from the aether").

This logically falls out from needing to be able to implement a type like Vec which doesn't like, reserve an address in memory for its copies of the ZSTs. It just dumps them into the aether and then pulls them from the aether when it needs to.

However it is dangerous to pull values from the aether like this. It is an important property of rust that privacy lets abstraction authors use the existence of a instance of a type they control serve as a proof that certain things have occurred, and this extends to ZSTs. So for instance one might use a ZST as a token giving permission to access a global, and so pulling that token from the aether could cause UB.

The obvious rule of thumb here is that you should only be able to pull as many instances from the aether as you dumped there. Hence Vec<ZST> is fine, because it is basically just a counter that tracks how many instances of the ZST that this Vec has been given and subsequently dumped into the aether. This counter prevents safe code from pulling extra instances from the aether (forging them).

This jives well with ! being illegal to claim to have an instance of: you "can't" pull an instance from the aether that you did not dump there unless you have access to the type's constructor, and no one has access to that constructor.

Copy link
Contributor

@Gankra Gankra Aug 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

postscript: claiming to have an instance of union MyUnion { a: T, b: U } is a claim that you have either an instance of T or an instance of U stored in that memory (this similarly follows for enums, but discriminants make it messier to talk about so let's ignore them).

If T is a ZST (as in MaybeUninit), it follows that it is always valid for a MyUnion instance to be uninitialized, as that is always conformant with the claim that "there exists an instance of T here". Similarly if T is fine to instantiate, it's fine for U to be illegal to instantiate (!), as "p or contradiction" is just "p".

An interesting claim that falls out of what I just wrote here is that if T and U have mutually invalid values, the compiler may conclude that those mutually-invalid values are invalid for a MyUnion to have. So a union (or enum) is not actually completely opaque to the compiler. It's just that if one arm is (), the compiler has to let you do whatever you want with the memory/instantiation. I am lightly worried this contradicts something important, but I'm not sure! I do think this is consistent with how we compute "niches" for enum packing optimizations, I'm just having trouble convincing myself that it's ok to peek through someone's bare union if niches happen to line up. If it's not, the union author can use the MaybeUninit trick to make the compiler back off (similar to how people add PhantomData fields all the time), so I don't think it's a big deal.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a value is a property a byte or collection of bytes has. Bytes may have the secret 257th value of "uninitialized".

Okay, so that sounds more like what I wold call a "(Rust) Abstract (Machine) Byte".

An instance of a type is something you get from constructing that type.

Is this a list of bytes, of values, or something more mathematically abstract?

Zero-sized types do not have values, but they do still have instances. A logical consequence of this is that we cannot have the concept of a ZST with a forbidden value. Nor can we speak of an "uninitialized" ZST. It doesn't make sense.

From this, I would like to assert that ! does not in fact fall under the umbrella of forbidden values: it is simply forbidden to claim that you have an instance of !, because it does not have a constructor. If you claim to have an instance of !, you are lying, and we have made that lie unconditional UB (as is our right as type authors).

To me this indicates your concepts are flawed. I feel the fact that ! has 0 "inhabitants" and () has 1 and bool has 2 should nicely fall out of the same concept. I have described such a concept in the UCG repo, but it is still work-in-progress.

In particular, "ZSTs have no values" sounds just so wrong to me. The type () obviously has 1 value, and that's ()! (We sadly write them the same in Rust, but you get what I mean.) You even said yourself that "value" has something to do with a "collection of bytes"; if I interpret that freely as "list of bytes" then the type () has one "value", and that's the empty list.

"No value" to me sounds like "empty type", and that raises some bad memories about how many people call void in C "empty". That's not accurate. void is exactly like (), it conveys no information, but it isn't "empty". The way to formally model "no information" is to say that "there is only one possible answer", AKA "the answer was already known so it doesn't even have to be given", AKA a type with exactly 1 possible value.

Information-theoretically this also works perfectly: you need log2(N) many bits to encode which of N answers is given, and log2(1) = 0. So you need 0 bits to encode the () type.

In the stdlib we frequently appeal to the idea that, logically speaking, there is always an instance of a zero-sized type everywhere you look. So e.g. (73 as *const ()).read() is not UB ("you can pull instances from the aether").

The way I would phrase it is that copying/moving a ZST is a NOP. There is nothing to copy, operationally. As you said, Vec won't pull "more instance of a ZST out of this air as exist in the vector", so logically speaking, this is not really out-of-thin air -- it's just that you cannot see the moves that are happening.

We've had cases where our handling of ZST did not follow this model, but I think those were bugs.

However it is dangerous to pull values from the aether like this. It is an important property of rust that privacy lets abstraction authors use the existence of a instance of a type they control serve as a proof that certain things have occurred, and this extends to ZSTs. So for instance one might use a ZST as a token giving permission to access a global, and so pulling that token from the aether could cause UB.

Agreed. However, in my terminology, those privacy-induced extra properties arise from the safety invariant, not the validity invariant.

All of that said, I think much of it is not relevant for a discussion about "what is UB". Even if Vec did "duplicate" a ZST, that would not be instantaneous UB! We cannot make the Abstract Machine understand the safety invariant, it can be arbitrarily complicated after all and is often user-defined.

In other words, I claim that for any slice of ZST, doubling its length does not cause undefined behavior. Doing this may lead to UB later, when some code that assumed its safety invariant is not broken runs, but the act of doubling the slice length itself is perfectly fine, from the perspective of the Abstract Machine.

claiming to have an instance of union MyUnion { a: T, b: U } is a claim that you have either an instance of T or an instance of U stored in that memory

We have some disagreements here. ;)

I think treating unions this way is very, very complicated because when we go about defining the Abstract Machine in a concrete, operational way (like an interpreter), we cannot know which variant the union is in. It is because of that that I am strongly pushing for "unions have no active variant". https://github.com/rust-lang/rfcs/blob/master/text/2514-union-initialization-and-drop.md deliberately did not have such a notion, and these days the Reference states:

Unions have no notion of an "active field".

An interesting claim that falls out of what I just wrote here is that if T and U have mutually invalid values, the compiler may conclude that those mutually-invalid values are invalid for a MyUnion to have. So a union (or enum) is not actually completely opaque to the compiler.

I think you are re-tracing a lot of rust-lang/unsafe-code-guidelines#73 right now. ;) The OP of that thread already contains some code that I (and many in the UCG) think is legal, but that you just ruled out.

Copy link
Contributor

@Gankra Gankra Aug 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't properly consider the claim of unions "having an instance" in the context of field-by-field re-initialization into another type (which is of course extremely important). But my intended claim here is exactly the one gnzlbg is championing in that thread (and I have replied there to that effect) -- only if niches align is a niche actually available. Hence "mutually invalid values" (the intersection of their value domains, in your terminology). This is consistent with the way we make you create a union by providing an entire instance of one of the union's cases. Like, by default to create a union you have to very materially have an instance.

But yeah once you start using the union you can enter a super-position of instances, which certainly makes it awkward to talk about. I am absolutely not intending to suggest there is an "active member" as that is a horribly cursed concept.

Certainly though, if you ever do &mut my_union.a, in my understanding of the current reference semantics, you are making a very material claim of having an instance of that type. And it would also be very difficult for us to make unions robustly opaque, in the way asm!() blocks and volatile accesses are.

Something still really bothers about the rules of ! falling out the rules for value domains. Like yes I understand the notion of 0 vs 1 inhabitants (although thank you for reminding me of that term), but inhabitants don't feel like the right abstraction for talking about packing bits? Like you also appealed to the notion of "a list of bytes", which I agree can reasonably include the empty list, but if I ask for a list, you're not allowed to come back with "well, here's no list at all, that's a list right".

Gonna keep reading the relevant things over and mulling it over. I am under the impression I'm just in a weird headspace and I'll eventually come around to your position on just having ! slot in here naturally.

Question for you: is union { a: !, b: ! } uninstantiable? i.e. is it equivalent to !? In my mind it should be equivalent to !, but it seems like perhaps you would like it to be instantiable?

More broadly: to what extent is union { a: T, b: T} not just T? Your definition of unions seems to want it to be equivalent to MaybeUninit<T> (ignoring Drop as uninteresting here), which is strange, because that isn't the definition it uses!

(assume repr(rust) here, if you find my proposal for unions in rust-lang/unsafe-code-guidelines#73 compelling)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly though, if you ever do &mut my_union.a, in my understanding of the current reference semantics, you are making a very material claim of having an instance of that type.

Fair. At this point we entered rust-lang/unsafe-code-guidelines#77: does a reference &mut T have to point to a "valid" T even when the T is not actually loaded from memory? The document here for now, conservatively, says "yes", but I actually think we should delay this until the pointer is actually "followed" and a T is materialized. But that is a separate discussion.

inhabitants don't feel like the right abstraction for talking about packing bits? Like you also appealed to the notion of "a list of bytes", which I agree can reasonably include the empty list, but if I ask for a list, you're not allowed to come back with "well, here's no list at all, that's a list right".

That's why I also have a relation in the mathematical sense that relates "abstract values" (the view of the world where bool has 2 inhabitants, () has 1 and ! has 0) with the way they are represented on the machine (as lists of bytes). If we assume canonical representaiton for a second (which not all types have, but the three we are considering here do), then that means there are exactly 2 byte lists that represent a valid bool (both lists have length 1: [0x00] and [0x01]), there is exactly 1 byte list that represents a valid () (the empty list []), and there is no byte list that represents a valid !.

So, I think this translates fine even if you switch between the abstract view of a type (the "value" view) and its concrete view (the "byte list" view).

Question for you: is union { a: !, b: ! } uninstantiable? i.e. is it equivalent to !? In my mind it should be equivalent to !, but it seems like perhaps you would like it to be instantiable?

I personally would prefer for a union to be just a "bag of bytes": its abstract values are all byte lists of appropriate lengths, and the types in the fields don't matter at all (except for their size and alignment) until some field is actually used.

My view of a union of size N is basically that is is the same as [MaybeUninit<u8>; N], and that we also offer field access as a convenient way to transmute that type to the type of the field (truncating the data if the field does not cover the entire union).

There are problems with that view, unfortunately. But they are not related to !.

So, with that view, the union you are asking about is indeed "instantiable"; it has size 0 and the empty byte list is valid for that type. But accessing any of its fields is a transmute-to-! and therefore UB.

which is strange, because that isn't the definition it uses!

What do you mean by "the definition it uses"?

* an invalid library type with custom invalid values, such as a `NonNull` or
`NonZero*` that is 0
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
* Unwinding into another language
* Causing a [data race][race]
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
* Executing code compiled with target features that the current thread of execution does
not support (see [`target_feature`])
RalfJung marked this conversation as resolved.
Show resolved Hide resolved

"Producing" a value happens any time a value is assigned, passed to a
function/primitive operation or returned from a function/primitive operation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggested reword and massive clarification:

Many have trouble accepting the consequences of invalid values, so they merit some extra discussion. The claim being made here is a very strong one, so read carefully.

A value is produced whenever it is assigned, passed to something, or returned from something. Keep in mind references get to assume their referents are valid, so you can't even create a reference to an invalid value. Additionally, uninitialized memory is always invalid, so you can't assign it to anything, pass it to anything, return it from anything, or take a reference to it. (Padding bytes are not technically part of a value's memory, and so may be left uninitialized.)

In simple and blunt terms: you cannot ever even suggest the existence of an invalid value. No, it's not ok if you "don't use" or "don't read" the value. Invalid values are instant Undefined Behaviour. The only correct way to manipulate memory that could be invalid is with raw pointers using methods like write and copy. If you want to leave a local variable or struct field uninitialized (or otherwise invalid), you must use a union or enum which clearly indicates at the type level that this memory may contain no values (see MaybeUninit for details).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I applied most of your suggestions but this one is big enough that it is probably easier to hand the PR off to you. ;) I'd love to do a pass over what you got when you are done, if you don't mind.

I like this new text, as usual in you very pointed style! One comment though:

Additionally, uninitialized memory is always invalid, so you can't assign it to anything

That's not true for MaybeUninit.


A reference/pointer is "dangling" if not all of the bytes it points to are part
of the same allocation. The span of bytes it points to is determined by the
pointer value and the size of the pointee type.

That's it. That's all the causes of Undefined Behavior baked into Rust. Of
course, unsafe functions and traits are free to declare arbitrary other
Expand Down Expand Up @@ -58,3 +80,4 @@ these problems are considered impractical to categorically prevent.
[pointer aliasing rules]: references.html
[uninitialized memory]: uninitialized.html
[race]: races.html
[`target_feature`]: ../reference/attributes/codegen.html#the-target_feature-attribute