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.

Sign up for GitHub

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

Jump to bottom

transmute calculates incorrect size of some generic types involving pointers to T: ?Sized. #101084

Closed
zachs18 opened this issue Aug 27, 2022 · 0 comments · Fixed by #125740
Closed

transmute calculates incorrect size of some generic types involving pointers to T: ?Sized. #101084

zachs18 opened this issue Aug 27, 2022 · 0 comments · Fixed by #125740
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-opsem Relevant to the opsem team

Comments

@zachs18
Copy link
Contributor

zachs18 commented Aug 27, 2022 •
edited
Loading

I tried this code: playground link

#[repr(align(32))]
struct OverAlignZST;
// SizeSkeleton::compute returns Ok(Pointer { .. }) for this when T: ?Sized
pub struct PtrAndOverAlignZST<T: ?Sized> {
    _inner: *mut T,
    _other: OverAlignZST,
}
// This does not compile if you remove ?Sized
pub unsafe fn shouldnt_work<T: ?Sized>(from: *mut T) -> PtrAndOverAlignZST<T> {
    std::mem::transmute(from) // works, but shouldn't
}
fn main() {
    dbg!(std::mem::size_of::<*mut [i32]>());
    dbg!(std::mem::size_of::<PtrAndOverAlignZST<[i32]>>());
    let x = &mut [0][..];
    unsafe {
        let _x = shouldnt_work(x);
    }
    dbg!(std::mem::size_of::<*mut ()>());
    dbg!(std::mem::size_of::<PtrAndOverAlignZST<()>>());
    let y = &mut ();
    unsafe {
        let _y = shouldnt_work(y);
    }
}

I expected to see this happen: Compiler error because *mut T and PtrAndOverAlignZST<T> are not (known to be) the same size.

Instead, this happened: The above code compiles (and runs), but ICEs miri.

$ cargo run # or cargo +nightly run
[src/main.rs:13] std::mem::size_of::<*mut [i32]>() = 16
[src/main.rs:14] std::mem::size_of::<PtrAndOverAlignZST<[i32]>>() = 32
[src/main.rs:19] std::mem::size_of::<*mut ()>() = 8
[src/main.rs:20] std::mem::size_of::<PtrAndOverAlignZST<()>>() = 32

Meta

rustc --version --verbose:

rustc 1.63.0 (4b91a6ea7 2022-08-08)
binary: rustc
commit-hash: 4b91a6ea7258a947e59c6522cd5898e7c0a6a88f
commit-date: 2022-08-08
host: x86_64-unknown-linux-gnu
release: 1.63.0
LLVM version: 14.0.5

rustc +nightly --version --verbose:

rustc 1.65.0-nightly (c0941dfb5 2022-08-21)
binary: rustc
commit-hash: c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01
commit-date: 2022-08-21
host: x86_64-unknown-linux-gnu
release: 1.65.0-nightly
LLVM version: 15.0.0
Backtrace

No backtrace for `rustc`, since no error occurs. 

$ cargo +nightly miri run
Preparing a sysroot for Miri (target: x86_64-unknown-linux-gnu)... done
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `/home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner target/miri/x86_64-unknown-linux-gnu/debug/rustc-sized-2022-08-27`
[src/main.rs:13] std::mem::size_of::<*mut [i32]>() = 16
[src/main.rs:14] std::mem::size_of::<PtrAndOverAlignZST<[i32]>>() = 32
thread 'rustc' panicked at 'assertion failed: `(left == right)`
  left: `Size(16 bytes)`,
 right: `Size(32 bytes)`', /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/compiler/rustc_const_eval/src/interpret/place.rs:654:17
stack backtrace:
   0:     0x7fb8bd552820 - std::backtrace_rs::backtrace::libunwind::trace::h4cfc0f9f3e5a75d1
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   1:     0x7fb8bd552820 - std::backtrace_rs::backtrace::trace_unsynchronized::h65db0c7f3c27bd40
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x7fb8bd552820 - std::sys_common::backtrace::_print_fmt::h7089d9085447b800
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/std/src/sys_common/backtrace.rs:66:5
   3:     0x7fb8bd552820 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h476cb2d77b0da91f
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/std/src/sys_common/backtrace.rs:45:22
   4:     0x7fb8bd5ad6ae - core::fmt::write::h53819b6276006088
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/core/src/fmt/mod.rs:1202:17
   5:     0x7fb8bd5434a5 - std::io::Write::write_fmt::h5b1829515868707a
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/std/src/io/mod.rs:1672:15
   6:     0x7fb8bd5554d3 - std::sys_common::backtrace::_print::h6cb157a41b3c080b
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/std/src/sys_common/backtrace.rs:48:5
   7:     0x7fb8bd5554d3 - std::sys_common::backtrace::print::h117b78b6f131ece6
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/std/src/sys_common/backtrace.rs:35:9
   8:     0x7fb8bd5554d3 - std::panicking::default_hook::{{closure}}::hf55bb9d90900cb92
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/std/src/panicking.rs:295:22
   9:     0x7fb8bd5551bf - std::panicking::default_hook::h095947f029f67011
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/std/src/panicking.rs:314:9
  10:     0x7fb8bfd94a54 - <rustc_driver[1396542de658d9ef]::DEFAULT_HOOK::{closure#0}::{closure#0} as core[bafc4c672d35f3dc]::ops::function::FnOnce<(&core[bafc4c672d35f3dc]::panic::panic_info::PanicInfo,)>>::call_once::{shim:vtable#0}
  11:     0x7fb8bd555d0d - std::panicking::rust_panic_with_hook::h3df526ea3ef21ddb
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/std/src/panicking.rs:702:17
  12:     0x7fb8bd555b67 - std::panicking::begin_panic_handler::{{closure}}::h802dd6d61f5dd58f
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/std/src/panicking.rs:588:13
  13:     0x7fb8bd552d2c - std::sys_common::backtrace::__rust_end_short_backtrace::h4bc0ea0de74aba3e
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/std/src/sys_common/backtrace.rs:138:18
  14:     0x7fb8bd555882 - rust_begin_unwind
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/std/src/panicking.rs:584:5
  15:     0x7fb8bd5aa1c3 - core::panicking::panic_fmt::h7c2821ba6a6b7ecd
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/core/src/panicking.rs:142:14
  16:     0x7fb8bd5aa3ab - core::panicking::assert_failed_inner::hafdffc86af950093
  17:     0x558c9d9d8a3b - core[bafc4c672d35f3dc]::panicking::assert_failed::<rustc_target[9d1ff69abfb01353]::abi::Size, rustc_target[9d1ff69abfb01353]::abi::Size>
  18:     0x558c9d942edb - <rustc_const_eval[786c7168b6e05b59]::interpret::eval_context::InterpCx<miri[a311be1da908590b]::machine::Evaluator>>::copy_op_no_validate
  19:     0x558c9d91fd69 - <rustc_const_eval[786c7168b6e05b59]::interpret::eval_context::InterpCx<miri[a311be1da908590b]::machine::Evaluator>>::emulate_intrinsic
  20:     0x558c9d92617a - <rustc_const_eval[786c7168b6e05b59]::interpret::eval_context::InterpCx<miri[a311be1da908590b]::machine::Evaluator>>::eval_fn_call
  21:     0x558c9d92f7f1 - <rustc_const_eval[786c7168b6e05b59]::interpret::eval_context::InterpCx<miri[a311be1da908590b]::machine::Evaluator>>::terminator
  22:     0x558c9da02010 - <core[bafc4c672d35f3dc]::panic::unwind_safe::AssertUnwindSafe<miri[a311be1da908590b]::eval::eval_entry::{closure#0}> as core[bafc4c672d35f3dc]::ops::function::FnOnce<()>>::call_once
  23:     0x558c9d9e21cd - miri[a311be1da908590b]::eval::eval_entry
  24:     0x558c9d866233 - <rustc_interface[d358214f54393a52]::passes::QueryContext>::enter::<<miri[cc91c6e922e90fca]::MiriCompilerCalls as rustc_driver[1396542de658d9ef]::Callbacks>::after_analysis::{closure#0}, ()>
  25:     0x558c9d86aaff - <miri[cc91c6e922e90fca]::MiriCompilerCalls as rustc_driver[1396542de658d9ef]::Callbacks>::after_analysis
  26:     0x7fb8bf4a380b - <rustc_interface[d358214f54393a52]::interface::Compiler>::enter::<rustc_driver[1396542de658d9ef]::run_compiler::{closure#1}::{closure#2}, core[bafc4c672d35f3dc]::result::Result<core[bafc4c672d35f3dc]::option::Option<rustc_interface[d358214f54393a52]::queries::Linker>, rustc_errors[68a9913b99f4d6e1]::ErrorGuaranteed>>
  27:     0x7fb8bf49f2ec - rustc_span[5318bedef2ca73b1]::with_source_map::<core[bafc4c672d35f3dc]::result::Result<(), rustc_errors[68a9913b99f4d6e1]::ErrorGuaranteed>, rustc_interface[d358214f54393a52]::interface::create_compiler_and_run<core[bafc4c672d35f3dc]::result::Result<(), rustc_errors[68a9913b99f4d6e1]::ErrorGuaranteed>, rustc_driver[1396542de658d9ef]::run_compiler::{closure#1}>::{closure#1}>
  28:     0x7fb8bf49ecd2 - rustc_interface[d358214f54393a52]::interface::create_compiler_and_run::<core[bafc4c672d35f3dc]::result::Result<(), rustc_errors[68a9913b99f4d6e1]::ErrorGuaranteed>, rustc_driver[1396542de658d9ef]::run_compiler::{closure#1}>
  29:     0x7fb8bf49d871 - <scoped_tls[cc0aafcf1c0b5a7f]::ScopedKey<rustc_span[5318bedef2ca73b1]::SessionGlobals>>::set::<rustc_interface[d358214f54393a52]::interface::run_compiler<core[bafc4c672d35f3dc]::result::Result<(), rustc_errors[68a9913b99f4d6e1]::ErrorGuaranteed>, rustc_driver[1396542de658d9ef]::run_compiler::{closure#1}>::{closure#0}, core[bafc4c672d35f3dc]::result::Result<(), rustc_errors[68a9913b99f4d6e1]::ErrorGuaranteed>>
  30:     0x7fb8bf49d55f - std[e6e0e56adfd51cea]::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface[d358214f54393a52]::util::run_in_thread_pool_with_globals<rustc_interface[d358214f54393a52]::interface::run_compiler<core[bafc4c672d35f3dc]::result::Result<(), rustc_errors[68a9913b99f4d6e1]::ErrorGuaranteed>, rustc_driver[1396542de658d9ef]::run_compiler::{closure#1}>::{closure#0}, core[bafc4c672d35f3dc]::result::Result<(), rustc_errors[68a9913b99f4d6e1]::ErrorGuaranteed>>::{closure#0}, core[bafc4c672d35f3dc]::result::Result<(), rustc_errors[68a9913b99f4d6e1]::ErrorGuaranteed>>
  31:     0x7fb8bfbb52e9 - <<std[e6e0e56adfd51cea]::thread::Builder>::spawn_unchecked_<rustc_interface[d358214f54393a52]::util::run_in_thread_pool_with_globals<rustc_interface[d358214f54393a52]::interface::run_compiler<core[bafc4c672d35f3dc]::result::Result<(), rustc_errors[68a9913b99f4d6e1]::ErrorGuaranteed>, rustc_driver[1396542de658d9ef]::run_compiler::{closure#1}>::{closure#0}, core[bafc4c672d35f3dc]::result::Result<(), rustc_errors[68a9913b99f4d6e1]::ErrorGuaranteed>>::{closure#0}, core[bafc4c672d35f3dc]::result::Result<(), rustc_errors[68a9913b99f4d6e1]::ErrorGuaranteed>>::{closure#1} as core[bafc4c672d35f3dc]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
  32:     0x7fb8bd55f8b3 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::h153a5b9c09be8d12
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/alloc/src/boxed.rs:1935:9
  33:     0x7fb8bd55f8b3 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::h50171cd485c6ebe5
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/alloc/src/boxed.rs:1935:9
  34:     0x7fb8bd55f8b3 - std::sys::unix::thread::Thread::new::thread_start::h1d58b0ea990239e8
                               at /rustc/c0941dfb5a7d07ef2d70cc54d319669d9d6f6c01/library/std/src/sys/unix/thread.rs:108:17
  35:     0x7fb8bd1a5b43 - start_thread
                               at ./nptl/./nptl/pthread_create.c:442:8
  36:     0x7fb8bd237a00 - clone3
                               at ./misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
  37:                0x0 - <unknown>

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.65.0-nightly (c0941dfb5 2022-08-21) running on x86_64-unknown-linux-gnu

note: compiler flags: --crate-type bin -C embed-bitcode=no -C debuginfo=2 -C incremental

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
end of query stack

Miri caused an ICE during evaluation. Here's the interpreter backtrace at the time of the panic:
note: the place in the program where the ICE was triggered
  --> src/main.rs:10:5
   |
10 |     std::mem::transmute(from) // works, but shouldn't
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: inside `shouldnt_work::<[i32]>` at src/main.rs:10:5
note: inside `main` at src/main.rs:17:18
  --> src/main.rs:17:18
   |
17 |         let _x = shouldnt_work(x);
   |                  ^^^^^^^^^^^^^^^^
   = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:248:5
   = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:122:18
   = note: inside closure at /home/zachary/.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 /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:280:13
   = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:492:40
   = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:456:19
   = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:137:14
   = note: inside closure at /home/zachary/.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 /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:492:40
   = note: inside `std::panicking::r#try::<isize, [closure@std::rt::lang_start_internal::{closure#2}]>` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:456:19
   = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:137:14
   = note: inside `std::rt::lang_start_internal` at /home/zachary/.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 /home/zachary/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:144:17

AFICT, rustc currently only supports transmuting a subset of Sized types that it conservatively can guarantee are the same size. For generic struct types with ?Sized parameters, I think rustc can only guarantee the size if the whole struct's size does not depend on the parameters, or if the struct's only non-zst field is a (transitive transparent wrapper of a) pointer type depending on a generic parameter (rustc_middle::ty::layout::SizeSkeleton only has Known(usize) and Pointer { .. } variants, so can only express completely known and only-a-pointer sizes). I think this size-calculation ignoring all ZSTs (not just 1-ZSTs) when there is a pointer field can cause transmute to (incorrectly?) allow transmuting between differently-sized types.

rustc_middle::ty::layout::SizeSkeleton probably needs to be made aware of alignment, and then needs to change compute to account for alignment when checking fields.

# compiler/rustc_middle/src/ty/layout.rs:2028
- if size.bytes() > 0 {
+ if size.bytes() > 0 || align_of_field_somehow > 1 {
      return Err(err);
  }

See also: #101081
@zachs18 zachs18 added the C-bug Category: This is a bug. label Aug 27, 2022
@zachs18 zachs18 mentioned this issue Aug 27, 2022
Closed
@zachs18 zachs18 changed the title transmute does not correctly calculate size of some types. transmute does not correctly calculate size of some generic types involving pointers to T: ?Sized. Aug 27, 2022
@zachs18 zachs18 changed the title transmute does not correctly calculate size of some generic types involving pointers to T: ?Sized. transmute calculates incorrect size of some generic types involving pointers to T: ?Sized. Aug 28, 2022
@fmease fmease added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-opsem Relevant to the opsem team and removed needs-triage-legacy A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. labels Jan 26, 2024
@RalfJung RalfJung mentioned this issue May 29, 2024
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 24, 2024
@matthiaskrgr
Rollup merge of rust-lang#125740 - RalfJung:transmute-size-check, r=o…
621ab36 
…li-obk

transmute size check: properly account for alignment

Fixes another place where ZST alignment was ignored when checking whether something is a newtype. I wonder how many more of these there are...

Fixes rust-lang#101084
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 24, 2024
@matthiaskrgr
Rollup merge of rust-lang#125740 - RalfJung:transmute-size-check, r=o…
e78bfc3 
…li-obk

transmute size check: properly account for alignment

Fixes another place where ZST alignment was ignored when checking whether something is a newtype. I wonder how many more of these there are...

Fixes rust-lang#101084
@bors bors closed this as completed in bda221a Jun 25, 2024
tiif pushed a commit to tiif/miri that referenced this issue Jun 25, 2024
@bors
Auto merge of #125740 - RalfJung:transmute-size-check, r=oli-obk
24e16c6 
transmute size check: properly account for alignment

Fixes another place where ZST alignment was ignored when checking whether something is a newtype. I wonder how many more of these there are...

Fixes rust-lang/rust#101084
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Jun 28, 2024
@bors
Auto merge of #125740 - RalfJung:transmute-size-check, r=oli-obk
e7ae929 
transmute size check: properly account for alignment

Fixes another place where ZST alignment was ignored when checking whether something is a newtype. I wonder how many more of these there are...

Fixes rust-lang/rust#101084
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Jul 11, 2024
@bors
Auto merge of #125740 - RalfJung:transmute-size-check, r=oli-obk
17e057c 
transmute size check: properly account for alignment

Fixes another place where ZST alignment was ignored when checking whether something is a newtype. I wonder how many more of these there are...

Fixes rust-lang/rust#101084
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-opsem Relevant to the opsem team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

transmute size check: properly account for alignment RalfJung/rust
3 participants
@ChrisDenton @zachs18 @fmease