pretzelhammer / rust-blog

Educational blog posts for Rust beginners
Apache License 2.0
7.11k stars 380 forks source link

Remove static mut example #19

Closed troublescooter closed 3 years ago

troublescooter commented 3 years ago

In the blog post about lifetimes, the following code snippet is used:

static BYTES: [u8; 3] = [1, 2, 3];
static mut MUT_BYTES: [u8; 3] = [1, 2, 3];

fn main() {
   MUT_BYTES[0] = 99; // compile error, mutating static is unsafe

    unsafe {
        MUT_BYTES[0] = 99;
        assert_eq!(99, MUT_BYTES[0]);
    }
}

There has been some controversy surrounding this feature (internals, blog), some even calling this a "misfeature" in dire need of replacement. Because they appear deceptively easy to use correctly (I think it's unproblematic here, given that indexing an array is a built-in, and not a call to index_mut which AFAIK is UB), perhaps at least a warning should be added that this is non-trivial.

pretzelhammer commented 3 years ago

I re-read my article to remind myself the context within which this snippet is presented and I don't see how it's problematic. I don't endorse or suggest anyone to use static mut in their programs. I use the snippet to illustrate and reinforce the idea that directly mutating static variables is unsafe. Also, I think the fact that it's impossible to use static mut variables outside of unsafe blocks already clearly communicates to the reader just how wildly unsafe they are.

I think it's fine to leave as-is.

troublescooter commented 3 years ago

My titled suggestion might have been too radical, but I do think there is value added by extending the list directly below the snippet to

Sometimes brains infer things which are not there, so the detail that static mut can only be mutated by assigning to it and not by mutable reference might be (probably is) something that is lost on a reader. This fact is little known and subtle, so I think there's some didactic value in explicitly stating it in a tutorial.

troublescooter commented 3 years ago

With the introduction of const-generic indexing this example is now certainly UB, as I understand it.

pretzelhammer commented 3 years ago

@troublescooter could you create a minimal reproducible example of the UB on the Rust playground? I'd like to better understand this myself before I make any changes to the blog post. Thank you!

pretzelhammer commented 3 years ago

@troublescooter I read this comment and as I understand it the example as-is has no UB. I tried to my hardest to create UB but was unable to, even with multiple aliased mut references.

troublescooter commented 3 years ago

@troublescooter I read this comment and as I understand it the example as-is has no UB.

Yes, that appears to be so.

I tried to my hardest to create UB but was unable to, even with multiple aliased mut references.

UB cannot be disproven by example and may be present even if there's no observable misbehaviour. That example clearly is UB and future compiler improvements might lead to broken code.

Some optimisations are currently disabled, but the example still runs fine when they are activated. Using MIRI in the Tools menu demonstrates it exhibits UB.

Edit: Better wording

pretzelhammer commented 3 years ago

Since you've been very persistent I've added the following disclaimer header comment to the example:

// Note: This example is purely for illustrative purposes.
// Never use `static mut`. It's a footgun. There are
// safe patterns for global mutable singletons in Rust but
// those are outside the scope of this article.

static BYTES: [u8; 3] = [1, 2, 3];
static mut MUT_BYTES: [u8; 3] = [1, 2, 3];

fn main() {
   MUT_BYTES[0] = 99; // compile error, mutating static is unsafe

    unsafe {
        MUT_BYTES[0] = 99;
        assert_eq!(99, MUT_BYTES[0]);
    }
}

Closing as completed.

parasyte commented 2 years ago

I tried to my hardest to create UB but was unable to, even with multiple aliased mut references.

👋 Hi from the future.

This example is not only clearly UB, but Miri is even able to tell you why:

error: Undefined Behavior: no item granting read access to tag <2005> at alloc1 found in borrow stack.
  --> src/main.rs:11:9
   |
11 |         ref0[0] += 1;
   |         ^^^^^^^^^^^^ no item granting read access to tag <2005> at alloc1 found in borrow stack.
   |
   = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
   = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information

   = note: inside `main` at src/main.rs:11:9
   = 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:123:18
   = note: inside closure at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:145: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:406: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:370: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:133:14
   = note: inside closure at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:128:48
   = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:406:40
   = note: inside `std::panicking::r#try::<isize, [closure@std::rt::lang_start_internal::{closure#2}]>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:370:19
   = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:133: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:128:20
   = 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:144:17

You can't observe UB the same way you can observe the result of 1 + 1. UB is often invisible. To put it another way, it is true that UB sometimes has the effect of segfaulting or making demons fly out of your nose. But UB never guarantees that it will segfault or spawn a nasal demon.