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

portable_atomic_util::Arc::into_raw() returns unaligned reference to the inner type #137

Closed
notgull opened this issue Dec 16, 2023 · 3 comments · Fixed by #138
Closed

portable_atomic_util::Arc::into_raw() returns unaligned reference to the inner type #137

notgull opened this issue Dec 16, 2023 · 3 comments · Fixed by #138
Labels
A-portable-atomic-util Area: related to portable-atomic-util crate C-bug Category: related to a bug.

Comments

@notgull
Copy link
Contributor

notgull commented Dec 16, 2023

Repro:

#[cfg(feature = "has_bug")]
use portable_atomic_util::Arc;
#[cfg(not(feature = "has_bug"))]
use std::sync::Arc;

#[repr(align(128))]
struct Aligned(u32);

fn main() {
    let x = Arc::new(Aligned(128));
    println!("x: {:p}", &x);
    let ptr = Arc::into_raw(x);
    println!("ptr: {:p}", ptr);
    let x_ref = unsafe { &*ptr };
    println!("{:?}", &x_ref.0);
    drop(unsafe { Arc::from_raw(ptr) });
}
$ cargo miri run
Preparing a sysroot for Miri (target: x86_64-unknown-linux-musl)... done
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `/home/jtnunley/.rustup/toolchains/nightly-x86_64-unknown-linux-musl/bin/cargo-miri runner /hard_data/cargo_target/miri/x86_64-unknown-linux-musl/debug/pu-test`
x: 0x246d8
ptr: 0x24400
128
$ cargo miri run --features has_bug
Preparing a sysroot for Miri (target: x86_64-unknown-linux-musl)... done
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `/home/jtnunley/.rustup/toolchains/nightly-x86_64-unknown-linux-musl/bin/cargo-miri runner /hard_data/cargo_target/miri/x86_64-unknown-linux-musl/debug/pu-test`
x: 0x24860
ptr: 0x24490
error: Undefined Behavior: constructing invalid value: encountered an unaligned reference (required 128 byte alignment but found 16)
  --> src/main.rs:14:26
   |
14 |     let x_ref = unsafe { &*ptr };
   |                          ^^^^^ constructing invalid value: encountered an unaligned reference (required 128 byte alignment but found 16)
   |
   = 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: BACKTRACE:
   = note: inside `main` at src/main.rs:14:26: 14:31

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

error: aborting due to previous error
@taiki-e
Copy link
Owner

taiki-e commented Dec 16, 2023

The problem is that the offset calculation does not take into account the padding after Header.
Perhaps mem::size_of::<Header>() should be cmp::max(mem::size_of::<Header>(), mem::align_of::<T>()).

let new_ptr = strict::map_addr(raw_ptr, |addr| addr - mem::size_of::<Header>());

let new_ptr = strict::map_addr(ptr, |addr| addr + mem::size_of::<Header>());

@taiki-e taiki-e added A-portable-atomic-util Area: related to portable-atomic-util crate C-bug Category: related to a bug. labels Dec 16, 2023
@notgull
Copy link
Contributor Author

notgull commented Dec 16, 2023

@taiki-e I will write a PR to fix this shortly.

@taiki-e
Copy link
Owner

taiki-e commented Dec 16, 2023

Perhaps mem::size_of::<Header>() should be cmp::max(mem::size_of::<Header>(), mem::align_of::<T>()).

Ah, since align_of is incompatible with unsized type, it will actually be necessary to use align_of_val(&self.inner().value) (for as_ptr) and align_of_val(&*ptr) (for from_raw), instead of align_of::<T>().

notgull added a commit to forkgull/portable-atomic that referenced this issue Dec 16, 2023
This commit fixes taiki-e#137 by considering alignment when converting the raw
Arc pointer to and raw its raw representation.

Signed-off-by: John Nunley <[email protected]>
taiki-e pushed a commit that referenced this issue Dec 16, 2023
This commit fixes #137 by considering alignment when converting the raw
Arc pointer to and raw its raw representation.

Signed-off-by: John Nunley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-portable-atomic-util Area: related to portable-atomic-util crate C-bug Category: related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants