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

Fix &mut invalidation in ptr::swap doctest #95617

Merged
merged 1 commit into from
Apr 3, 2022

Conversation

saethlin
Copy link
Member

@saethlin saethlin commented Apr 3, 2022

Under Stacked Borrows with raw pointer tagging, the previous code was UB
because the code which creates the the second pointer borrows the array
through a tag in the borrow stacks below the Unique tag that our first
pointer is based on, thus invalidating the first pointer.

This is not definitely a bug and may never be real UB, but I desperately
want people to write code that conforms to SB with raw pointer tagging
so that I can write good diagnostics. The alternative aliasing models
aren't possible to diagnose well due to state space explosion.
Therefore, it would be super cool if the standard library nudged people
towards writing code that is valid with respect to SB with raw pointer
tagging.

The diagnostics that I want to write are implemented in a branch of Miri and the one for this case is below:

error: Undefined Behavior: attempting a read access using <2170> at alloc1068[0x0], but that tag does not exist in the borrow stack for this location
    --> /home/ben/rust/library/core/src/intrinsics.rs:2103:14
     |
2103 |     unsafe { copy_nonoverlapping(src, dst, count) }
     |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |              |
     |              attempting a read access using <2170> at alloc1068[0x0], but that tag does not exist in the borrow stack for this location
     |              this error occurs as part of an access at alloc1068[0x0..0x8]
     |
     = 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
help: <2170> was created due to a retag at offsets [0x0..0x10]
    --> ../libcore/src/ptr/mod.rs:640:9
     |
8    | let x = array[0..].as_mut_ptr() as *mut [u32; 2]; // this is `array[0..2]`
     |         ^^^^^^^^^^^^^^^^^^^^^^^
help: <2170> was later invalidated due to a retag at offsets [0x0..0x10]
    --> ../libcore/src/ptr/mod.rs:641:9
     |
9    | let y = array[2..].as_mut_ptr() as *mut [u32; 2]; // this is `array[2..4]`
     |         ^^^^^
     = note: inside `std::intrinsics::copy_nonoverlapping::<[u32; 2]>` at /home/ben/rust/library/core/src/intrinsics.rs:2103:14
     = note: inside `std::ptr::swap::<[u32; 2]>` at /home/ben/rust/library/core/src/ptr/mod.rs:685:9
note: inside `main::_doctest_main____libcore_src_ptr_mod_rs_635_0` at ../libcore/src/ptr/mod.rs:12:5
    --> ../libcore/src/ptr/mod.rs:644:5
     |
12   |     ptr::swap(x, y);
     |     ^^^^^^^^^^^^^^^
note: inside `main` at ../libcore/src/ptr/mod.rs:15:3
    --> ../libcore/src/ptr/mod.rs:647:3
     |
15   | } _doctest_main____libcore_src_ptr_mod_rs_635_0() }
     |   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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

error: aborting due to previous error

@rust-highfive
Copy link
Collaborator

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 3, 2022
Under Stacked Borrows with raw pointer tagging, the previous code was UB
because the code which creates the the second pointer borrows the array
through a tag in the borrow stacks below the Unique tag that our first
pointer is based on, thus invalidating the first pointer.

This is not definitely a bug and may never be real UB, but I desperately
want people to write code that conforms to SB with raw pointer tagging
so that I can write good diagnostics. The alternative aliasing models
aren't possible to diagnose well due to state space explosion.
Therefore, it would be super cool if the standard library nudged people
towards writing code that is valid with respect to SB with raw pointer
tagging.
@saethlin saethlin force-pushed the swap-test-invalidation branch from 7cb2711 to f4a7ed4 Compare April 3, 2022 20:17
@Dylan-DPC
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 3, 2022

📌 Commit f4a7ed4 has been approved by Dylan-DPC

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 3, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 3, 2022
Rollup of 5 pull requests

Successful merges:

 - rust-lang#95202 (Reduce the cost of loading all built-ins targets)
 - rust-lang#95553 (Don't emit non-asm contents error for naked function composed of errors)
 - rust-lang#95613 (Fix rustdoc attribute display)
 - rust-lang#95617 (Fix &mut invalidation in ptr::swap doctest)
 - rust-lang#95618 (core: document that the align_of* functions return the alignment in bytes)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f7f2d83 into rust-lang:master Apr 3, 2022
@rustbot rustbot added this to the 1.61.0 milestone Apr 3, 2022
@saethlin saethlin deleted the swap-test-invalidation branch May 16, 2022 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants