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.

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

Detect instant UB caused by creating a reference to an invalid value #1638

Open
camelid opened this issue Dec 3, 2020 · 4 comments
Open
Labels
A-validation Area: This affects enforcing the validity invariant, and related UB checking C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement

Comments

@camelid
Copy link
Member

camelid commented Dec 3, 2020

According to rust-lang/rust#78123 (comment) and the Reference, producing an &-reference (not sure about raw pointers) to an invalid value is instant UB:

Unlike C, Undefined Behavior is pretty limited in scope in Rust. All the core language cares about is preventing the following things:
[...]
a reference/Box that is dangling, unaligned, or points to an invalid value.

For example, if I understand correctly, this program has UB, but Miri does not report any errors:

enum Void {}

fn main() {
    let _x: &Void = unsafe { std::mem::transmute(&()) };
}
@camelid camelid changed the title Detect instant UB through creating a reference to an invalid value Detect instant UB caused by creating a reference to an invalid value Dec 3, 2020
@camelid
Copy link
Member Author

camelid commented Dec 3, 2020

Note that Miri correctly detects the case of creating an owned invalid value (and rustc produces a warning as well):

enum Void {}

fn main() {
    let x: Void = unsafe { std::mem::transmute(()) };
}

stderr:

warning: the type `Void` does not permit zero-initialization
 --> src/main.rs:4:28
  |
4 |     let x: Void = unsafe { std::mem::transmute(()) };
  |                            ^^^^^^^^^^^^^^^^^^^^^^^
  |                            |
  |                            this code causes undefined behavior when executed
  |                            help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done
  |
  = note: `#[warn(invalid_value)]` on by default
  = note: enums with no variants have no valid value

error: Undefined Behavior: type validation failed: encountered a value of uninhabited type Void
 --> src/main.rs:4:28
  |
4 |     let x: Void = unsafe { std::mem::transmute(()) };
  |                            ^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of uninhabited type Void
  |
  = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
  = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
          
  = note: inside `main` at src/main.rs:4:28
  = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
  = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:125:18
  = note: inside closure at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:66:18
  = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:259:13
  = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:379:40
  = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:343:19
  = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:396:14
  = note: inside `std::rt::lang_start_internal` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:51:25
  = note: inside `std::rt::lang_start::<()>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:65:5

error: aborting due to previous error; 2 warnings emitted

@RalfJung
Copy link
Member

RalfJung commented Dec 3, 2020

The rules for validity of reference are not finally decided yet, but still being discussed at rust-lang/unsafe-code-guidelines#77. To not preempt the conclusion of this discussion, the docs declare as much UB as is possible -- we can always relax UB later, but introducing more UB is a breaking change.

That's one reason why Miri does not complain about this kind of UB. The other reason is that it is really expensive to recursively traverse references all the time.

However, one special case should be easy and cheap to check: references to uninhabited types (i.e., the case you used as an example). That one does not actually require looking at the value behind the reference.

@RalfJung RalfJung added A-validation Area: This affects enforcing the validity invariant, and related UB checking C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement labels Dec 29, 2020
@eddyb
Copy link
Member

eddyb commented Jan 30, 2022

Could this be done as an opt-in flag? I wanted to demonstrate where &mut explicitly (don't) exist in a program, through miri, and found that it's not available.

(I understand if it's not a priority, just thought opt-in wouldn't slow down everyone, and would allow demonstrating "what if" questions wrt that kind of model)

@RalfJung
Copy link
Member

I wanted to demonstrate where &mut explicitly (don't) exist in a program, through miri, and found that it's not available.

Not sure what you mean by this, or by "this" in your first sentence -- the uninhabited-pointee case, or the general case?

Both could be implemented, of course, but I am don't think we should do the general recursive case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-validation Area: This affects enforcing the validity invariant, and related UB checking C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement
Projects
None yet
Development

No branches or pull requests

3 participants