Skip to content

Remove miri hack #1

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

Merged
merged 1 commit into from
Feb 13, 2022
Merged

Remove miri hack #1

merged 1 commit into from
Feb 13, 2022

Conversation

taiki-e
Copy link
Owner

@taiki-e taiki-e commented Feb 13, 2022

We currently use a hack to avoid rust-lang/rust#69488 and to make sure that Miri errors for atomic load/store of integers containing uninitialized bytes (which is probably not a problem and uncharted territory at best 1 2 3, and can be detected by -Zmiri-check-number-validity 4), do not mask Miri errors for the use of uninitialized bytes (which is definitely a problem).

atomic-memcpy/src/lib.rs

Lines 426 to 450 in 3507fef

// HACK: Miri cannot track uninitialized bytes on a per byte basis for
// partially initialized scalars: https://github.com/rust-lang/rust/issues/69488
//
// This hack allows miri to properly track the use of uninitialized
// bytes. See also tests/uninit.rs that is a test to check if
// valgrind/sanitizer/miri can properly detect the use of uninitialized
// bytes.
//
// Note: With or without this hack, atomic load/store of integers
// containing uninitialized bytes is technically an undefined behavior.
// The only purpose of this hack is to make sure that Miri errors for
// atomic load/store of integers containing uninitialized bytes
// (which is probably not a problem and uncharted territory at best [1] [2] [3],
// and can be detected by `-Zmiri-check-number-validity` [4]),
// do not mask Miri errors for the use of uninitialized bytes (which is definitely a problem).
// See also tests/padding.rs.
//
// [1] https://github.com/crossbeam-rs/crossbeam/issues/315
// [2] https://github.com/rust-lang/unsafe-code-guidelines/issues/158
// [3] https://github.com/rust-lang/unsafe-code-guidelines/issues/71
// [4] https://github.com/rust-lang/miri/pull/1904
//
// rust-lang/rust#69488 affects only CTFE(compile-time function evaluation)/Miri
// and atomic operations cannot be called in const context, so our code
// is only affected in the case of cfg(miri).

However, this actually causes another "unsupported operation" Miri error.

error: unsupported operation: unable to turn pointer into raw bytes
   --> /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:701:9
    |
701 |         copy_nonoverlapping(src, tmp.as_mut_ptr(), 1);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unable to turn pointer into raw bytes
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support

This actually causes another "unsupported operation" Miri error.

```
error: unsupported operation: unable to turn pointer into raw bytes
   --> /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:701:9
    |
701 |         copy_nonoverlapping(src, tmp.as_mut_ptr(), 1);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unable to turn pointer into raw bytes
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
```
@taiki-e
Copy link
Owner Author

taiki-e commented Feb 13, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented Feb 13, 2022

Build succeeded:

@bors bors bot merged commit a9de7a6 into main Feb 13, 2022
@bors bors bot deleted the miri branch February 13, 2022 08:48
@@ -18,11 +18,12 @@ See [P1478R1][p1478r1] for more.
- If the alignment of the type being copied is the same as the pointer width, `atomic_load` is possible to produce an assembly roughly equivalent to the case of using volatile read + atomic fence on many platforms. (See [`tests/asm-test/asm`][asm-test] directory for more).
- If the alignment of the type being copied is smaller than the pointer width, there will be some performance degradation. However, it is implemented in such a way that it does not cause extreme performance degradation at least on x86_64. (See [the implementation comments of `atomic_load`][implementation] for more.) It is possible that there is still room for improvement, especially on non-x86_64 platforms.
- Optimization for the case where the alignment of the type being copied is larger than the pointer width has not yet been fully investigated. It is possible that there is still room for improvement, especially on 32-bit platforms where `AtomicU64` is available.
- If the type being copied contains uninitialized bytes (e.g., padding), it is incompatible with `-Zmiri-check-number-validity`. This will probably not be resolved until something like `AtomicMaybeUninit` is supported.
- If the type being copied contains uninitialized bytes (e.g., padding), it is incompatible with `-Zmiri-check-number-validity`. This will probably not be resolved until something like `AtomicMaybeUninit` is supported. **Note**: Due to [Miri cannot track uninitialized bytes on a per byte basis for partially initialized scalars][rust-lang/rust#69488], Miri may report this case as an access to an uninitialized byte, regardless of whether the uninitialized byte is actually accessed or not.

Choose a reason for hiding this comment

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

Just noticed this by coincidence, and I am trying to understand what happens here.

If I understand correctly, this code effectively performs a copy at a type like u64, and due to rust-lang/rust#69488, that "inflates" uninit which causes problems. However, if that is the case, it will cause problems even without -Zmiri-check-number-validity, the problems are just slightly delayed: if x is uninit, x = 1 will fail in Miri even without that flag.

Moreover, arguably, this is not just a Miri limitation, it actually accurately reflects the reference. I have ideas for fixing rust-lang/rust#69488; they will not fix the problem described here.

Finally, if we start emitting noundef flags for LLVM on our integer types (which the reference says we can), then the approach in this crate will be problematic even if we ignore Rust UB and consider only LLVM UB.

So, I agree that AtomicMaybeUninit is the solution -- but until we have that, the problem isn't just a limitation in Miri, it is a limitation in Rust generally.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks. I agree that the current readme is incorrect. (Filed #5 to fix readme)

bors bot added a commit that referenced this pull request Feb 26, 2022
5: Fix docs about uninitialized bytes r=taiki-e a=taiki-e

Based on [the feedback](#1 (comment)) from `@RalfJung.`

Co-authored-by: Taiki Endo <te316e89@gmail.com>
taiki-e added a commit that referenced this pull request Feb 2, 2025
```
AddressSanitizer:DEADLYSIGNAL
=================================================================
==3334==ERROR: AddressSanitizer: SEGV on unknown address 0x02000d2ef240 (pc 0x555e01b0afc7 bp 0x7ffc430de1b0 sp 0x7ffc430de100 T0)
==3334==The signal is caused by a READ memory access.
    #0 0x555e01b0afc7  (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-04c46d82177f025f+0x186fc7) (BuildId: 29923f56aa27c9d06e08f73052eb3efbd82fedaf)
    #1 0x7fdf4de2a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
    #2 0x7fdf4de2a28a  (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
    #3 0x555e01a56014  (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-04c46d82177f025f+0xd2014) (BuildId: 29923f56aa27c9d06e08f73052eb3efbd82fedaf)

==3334==Register values:
rax = 0x000001ff8d2f7240  rbx = 0x00007ffc430de100  rcx = 0x0000000000000000  rdx = 0x000000008fff6fff
rdi = 0x00007fdf4be09010  rsi = 0x0000000000000000  rbp = 0x00007ffc430de1b0  rsp = 0x00007ffc430de100
 r8 = 0x00000ffc697b9200   r9 = 0x0000000000000000  r10 = 0xafffff00000fff01  r11 = 0x4000000000000000
r12 = 0x00007ffc430de2d8  r13 = 0x0000000000000001  r14 = 0x00007fdf4be09000  r15 = 0x00000ffbe97c1200
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-04c46d82177f025f+0x186fc7) (BuildId: 29923f56aa27c9d06e08f73052eb3efbd82fedaf)
==3334==ABORTING
error: test failed, to rerun pass `-p atomic-memcpy --lib`

Caused by:
  process didn't exit successfully: `/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-04c46d82177f025f` (exit status: 1)
note: test exited abnormally; to see the full output pass --nocapture to the harness.
```

```
MemorySanitizer init done
MemorySanitizer:DEADLYSIGNAL
==3315==ERROR: MemorySanitizer: SEGV on unknown address 0x6fff3618a3d8 (pc 0x55a3c90d218a bp 0x000000000000 sp 0x7fff3618a1b0 T3315)
==3315==The signal is caused by a READ memory access.
    #0 0x55a3c90d218a  (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-0e0b4cc066b47158+0x15f18a) (BuildId: eb746f488bb9238cde720cedbf7039f80464d20d)
    #1 0x55a3c90d1b56  (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-0e0b4cc066b47158+0x15eb56) (BuildId: eb746f488bb9238cde720cedbf7039f80464d20d)
    #2 0x55a3c903f0ec  (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-0e0b4cc066b47158+0xcc0ec) (BuildId: eb746f488bb9238cde720cedbf7039f80464d20d)
    #3 0x7fd98962a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
    #4 0x7fd98962a28a  (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
    #5 0x55a3c8fcc214  (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-0e0b4cc066b47158+0x59214) (BuildId: eb746f488bb9238cde720cedbf7039f80464d20d)

==3315==Register values:
rax = 0x0000000080000003  rbx = 0x0000500000000000  rcx = 0x00006fff3618a3d8  rdx = 0x0000100000000000
rdi = 0x00007fff3618a3d0  rsi = 0x00002fff3618a3d0  rbp = 0x0000000000000000  rsp = 0x00007fff3618a1b0
 r8 = 0x00006fff3618a3d0   r9 = 0x0000000000000000  r10 = 0x000055a3c8f8f0bf  r11 = 0x0000000000000206
r12 = 0x0000500000000000  r13 = 0x00002fff3618a3a0  r14 = 0x00007fff3618a3d0  r15 = 0xffffffffffffef60
MemorySanitizer can not provide additional info.
SUMMARY: MemorySanitizer: SEGV (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-0e0b4cc066b47158+0x15f18a) (BuildId: eb746f488bb9238cde720cedbf7039f80464d20d)
==3315==ABORTING
error: test failed, to rerun pass `-p atomic-memcpy --lib`

Caused by:
  process didn't exit successfully: `/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-0e0b4cc066b47158` (exit status: 1)
note: test exited abnormally; to see the full output pass --nocapture to the harness.
```
taiki-e added a commit that referenced this pull request Feb 2, 2025
```
AddressSanitizer:DEADLYSIGNAL
=================================================================
==3334==ERROR: AddressSanitizer: SEGV on unknown address 0x02000d2ef240 (pc 0x555e01b0afc7 bp 0x7ffc430de1b0 sp 0x7ffc430de100 T0)
==3334==The signal is caused by a READ memory access.
    #0 0x555e01b0afc7  (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-04c46d82177f025f+0x186fc7) (BuildId: 29923f56aa27c9d06e08f73052eb3efbd82fedaf)
    #1 0x7fdf4de2a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
    #2 0x7fdf4de2a28a  (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
    #3 0x555e01a56014  (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-04c46d82177f025f+0xd2014) (BuildId: 29923f56aa27c9d06e08f73052eb3efbd82fedaf)

==3334==Register values:
rax = 0x000001ff8d2f7240  rbx = 0x00007ffc430de100  rcx = 0x0000000000000000  rdx = 0x000000008fff6fff
rdi = 0x00007fdf4be09010  rsi = 0x0000000000000000  rbp = 0x00007ffc430de1b0  rsp = 0x00007ffc430de100
 r8 = 0x00000ffc697b9200   r9 = 0x0000000000000000  r10 = 0xafffff00000fff01  r11 = 0x4000000000000000
r12 = 0x00007ffc430de2d8  r13 = 0x0000000000000001  r14 = 0x00007fdf4be09000  r15 = 0x00000ffbe97c1200
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-04c46d82177f025f+0x186fc7) (BuildId: 29923f56aa27c9d06e08f73052eb3efbd82fedaf)
==3334==ABORTING
error: test failed, to rerun pass `-p atomic-memcpy --lib`

Caused by:
  process didn't exit successfully: `/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-04c46d82177f025f` (exit status: 1)
note: test exited abnormally; to see the full output pass --nocapture to the harness.
```

```
MemorySanitizer init done
MemorySanitizer:DEADLYSIGNAL
==3315==ERROR: MemorySanitizer: SEGV on unknown address 0x6fff3618a3d8 (pc 0x55a3c90d218a bp 0x000000000000 sp 0x7fff3618a1b0 T3315)
==3315==The signal is caused by a READ memory access.
    #0 0x55a3c90d218a  (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-0e0b4cc066b47158+0x15f18a) (BuildId: eb746f488bb9238cde720cedbf7039f80464d20d)
    #1 0x55a3c90d1b56  (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-0e0b4cc066b47158+0x15eb56) (BuildId: eb746f488bb9238cde720cedbf7039f80464d20d)
    #2 0x55a3c903f0ec  (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-0e0b4cc066b47158+0xcc0ec) (BuildId: eb746f488bb9238cde720cedbf7039f80464d20d)
    #3 0x7fd98962a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
    #4 0x7fd98962a28a  (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 6d64b17fbac799e68da7ebd9985ddf9b5cb375e6)
    #5 0x55a3c8fcc214  (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-0e0b4cc066b47158+0x59214) (BuildId: eb746f488bb9238cde720cedbf7039f80464d20d)

==3315==Register values:
rax = 0x0000000080000003  rbx = 0x0000500000000000  rcx = 0x00006fff3618a3d8  rdx = 0x0000100000000000
rdi = 0x00007fff3618a3d0  rsi = 0x00002fff3618a3d0  rbp = 0x0000000000000000  rsp = 0x00007fff3618a1b0
 r8 = 0x00006fff3618a3d0   r9 = 0x0000000000000000  r10 = 0x000055a3c8f8f0bf  r11 = 0x0000000000000206
r12 = 0x0000500000000000  r13 = 0x00002fff3618a3a0  r14 = 0x00007fff3618a3d0  r15 = 0xffffffffffffef60
MemorySanitizer can not provide additional info.
SUMMARY: MemorySanitizer: SEGV (/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-0e0b4cc066b47158+0x15f18a) (BuildId: eb746f488bb9238cde720cedbf7039f80464d20d)
==3315==ABORTING
error: test failed, to rerun pass `-p atomic-memcpy --lib`

Caused by:
  process didn't exit successfully: `/home/runner/work/atomic-memcpy/atomic-memcpy/target/x86_64-unknown-linux-gnu/release/deps/atomic_memcpy-0e0b4cc066b47158` (exit status: 1)
note: test exited abnormally; to see the full output pass --nocapture to the harness.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants