Fix issue with field retagging in scopeguard

[Nilstrieb]

With field retagging in SB, this causes UB because the read out value is invalidated by the move into mem::forget. Using ManuallyDrop<T> avoids the problem. With that issue fixed, we can now enable field retagging in CI. Field retagging is required to justify some of the noalias optimizations that rustc
performs currently.

Miri error

Undefined Behavior: trying to retag from <565074> for Unique permission at alloc212719[0x20], but that tag does not exist in the borrow stack for this location
    --> src/scopeguard.rs:40:13
     |
40   |             value
     |             ^^^^^
     |             |
     |             trying to retag from <565074> for Unique permission at alloc212719[0x20], but that tag does not exist in the borrow stack for this location
     |             this error occurs as part of retag at alloc212719[0x20..0x40]
     |
     = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows 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
help: <565074> was created by a Unique retag at offsets [0x20..0x40]
    --> src/scopeguard.rs:33:25
     |
33   |             let value = ptr::read(&guard.value);
     |                         ^^^^^^^^^^^^^^^^^^^^^^^
help: <565074> was later invalidated at offsets [0x20..0x40] by a Unique retag
    --> src/scopeguard.rs:39:25
     |
39   |             mem::forget(guard);
     |                         ^^^^^
     = note: backtrace:
     = note: inside `scopeguard::ScopeGuard::<&mut raw::RawTable<(i32, i32)>, [closure@src/raw/mod.rs:1637:45: 1637:52]>::into_inner` at src/scopeguard.rs:40:13
note: inside `<raw::RawTable<(i32, i32)> as core::clone::Clone>::clone_from` at src/raw/mod.rs:1671:17
    --> src/raw/mod.rs:1671:17
     |
1671 |                 ScopeGuard::into_inner(self_);
     |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `<map::HashMap<i32, i32> as core::clone::Clone>::clone_from` at src/map.rs:202:9
    --> src/map.rs:202:9
     |
202  |         self.table.clone_from(&source.table);
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `map::test_map::test_clone_from` at src/map.rs:6692:9
    --> src/map.rs:6692:9
     |
6692 |         m2.clone_from(&m);
     |         ^^^^^^^^^^^^^^^^^
note: inside closure at src/map.rs:6684:5
    --> src/map.rs:6684:5
     |
6683 |       #[test]
     |       ------- in this procedural macro expansion
6684 | /     fn test_clone_from() {
6685 | |         let mut m = HashMap::new();
6686 | |         let mut m2 = HashMap::new();
6687 | |         assert_eq!(m.len(), 0);
...    |
6695 | |         assert_eq!(m2.len(), 2);
6696 | |     }
     | |_____^
     = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

[Nilstrieb]

Huh, what happened on x86_64-pc-windows-gnu (it had a page fault at a very small address)???

[Amanieu]

I think this is a recent (last few days) regression in rustc. I'm looking into it.

[Amanieu]

By the way, this code is taken straight from the scopeguard crate. It would make sense to submit a fix there too.

[Nilstrieb]

I've tried to reproduce the fault locally in a cursed Wine in WSL2 environment but the tests passed for me.

[Nilstrieb]

Yep, it even fails without my diff, so there's something wrong.

[Nilstrieb]

scopeguard has received quite some changes around adding ManuallyDrop<T>. I'll just add all of them back to here.
edit: i don't think it actually makes sense to do that, our scopeguard doesn't have the strategy which avoids problems

[Amanieu]

Bisecting points to rust-lang/rust@9208625, I suspect the root cause might be rust-lang/rust#101325.

[Amanieu]

rust-lang/rust#101474

[Amanieu]

@bors r+

[bors]

📌 Commit 1f751c6 has been approved by Amanieu

It is now in the queue for this repository.

[bors]

⌛ Testing commit 1f751c6 with merge 2a7c322...

[bors]

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing 2a7c322 to master...

[RalfJung]

@Amanieu any chance of a release that includes this patch? We're considering enabling field retagging by default (since it matches the LLVM IR that rustc generates, Cc rust-lang/miri#2528), but I'd like to avoid breaking hashbrown in Miri.

[Amanieu]

Published hashbrown 0.13.1.