Skip to content

clarify what is UB #149

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

Merged
merged 21 commits into from
Aug 16, 2019
Merged

clarify what is UB #149

Changes from 19 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
40 changes: 32 additions & 8 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,41 @@ 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
* Reading [uninitialized memory][]
* Dereferencing (using the `*` operator on) dangling, or unaligned pointers, or
wide pointers with invalid metadata (see below)
* Breaking the [pointer aliasing rules][]
* Producing invalid primitive values:
* dangling/null references
* null `fn` pointers
* Unwinding into another language
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it really strictly defined to unwind between two rust libraries that are directly calling eachother via extern "C"? Otherwise we should have a more precise definition.

Specifically it might be that we just need to enumerate that you can only "exit a function via unwinding" if it has one of these 3 rust-specific calling conventions:

  • extern "rust" (the default of all fns)
  • extern "rust-call" (rust closures)
  • extern "rust-intrinsic" (things in core::intrinsics)

I think "rust-intrinsic" isn't supposed to unwind, but I might be wrong on that (perhaps the builtin operator impls have this ABI, somewhere?). But also "rust-intrinsic" kinda doesn't matter since those implementations are only provided by rustc. Only slightly matters if we care about helping devs assume raw re-exported intrinsics never unwind. But making an intrinsic a "normal" function would hardly be considered a breaking change!

Also I'm guessing incompatible compilation flags like mixing panic=abort is subsumed by the "target features" bullet below?

extern "platform-intrinsic" is also a thing but I think not relevant. I'm guessing it has something to do with libc functions that llvm is allowed to make implementation assumptions about, like malloc?)

Also I thought we had automatic guards against unwinding from an extern fn. Is that not the case? (I only work on panic=abort software so I never worry about this issue...)

Copy link

Choose a reason for hiding this comment

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

It is currently de facto undefined to unwind out of a Rust function defined as extern "C" no matter the caller, because our LLVM IR generation adds nounwind to such functions which makes it UB to unwind out of. IIUC lang team is saying this is deliberately and explicitly UB until explicitly stated to be supported in some cases, for which there's an ongoing RFC, rust-lang/rfcs#2699. The safeguard that aborts when you try to unwind out of an extern "C" Rust function did exist but was reverted twice so far because of fallout for software that relied on it despite it being UB, see rust-lang/rust#58794 if you want the messy history and part of the discussion that led to the aforementioned RFC.

* Causing a [data race][race]
* Executing code compiled with target features that the current thread of execution does
not support (see [`target_feature`][])
* Producing invalid primitive values (either alone or as a field of a compound
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
* null `fn` pointers
* a `char` outside the ranges [0x0, 0xD7FF] and [0xE000, 0x10FFFF]
* A non-utf8 `str`
* Unwinding into another language
* Causing a [data race][race]
* a `!` (all values are invalid for this type)
* dangling/unaligned references, references that do themselves point to
invalid values, or wide references (to a dynamically sized type) with
invalid metadata
* 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 integer (`i*`/`u*`), floating point value (`f*`), or raw pointer read from
[uninitialized memory][]
* an invalid library type with custom invalid values, such as a `NonNull` or
the `NonZero` family of types, that is 0
Copy link
Contributor

Choose a reason for hiding this comment

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

reword:

  • a type with custom invalid values that is one of those values, such as a NonNull that is null. (Requesting custom invalid values is an unstable feature, but some stable stdlib types, like NonNull, make use of it.)

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 have to say "a type with custom invalid values that is one of those values" sounds rather awkward. I don't have a better proposal either, though.


"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 it is null or not all of the bytes it
points to are part of the same allocation (so in particular they all have to be
part of *some* allocation). The span of bytes it points to is determined by the
pointer value and the size of the pointee type. As a consequence, if the span is
empty, "dangling" is the same as "non-null".

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 +81,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