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

ice: 'PassMode::Direct' for aggregates only allowed on wasm targets #115845

Closed
matthiaskrgr opened this issue Sep 14, 2023 · 6 comments · Fixed by #117500
Closed

ice: 'PassMode::Direct' for aggregates only allowed on wasm targets #115845

matthiaskrgr opened this issue Sep 14, 2023 · 6 comments · Fixed by #117500
Labels
C-bug Category: This is a bug. F-unsized_fn_params `#![feature(unsized_fn_params)]` I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@matthiaskrgr
Copy link
Member

Code

#![feature(unsized_tuple_coercion)]
#![feature(unsized_fn_params)]

fn bad() -> extern "C" fn(([u8],)) {
    todo!()
}

fn main() {
    let f = bad();
    let slice: Box<([u8],)> = Box::new(([1; 8],));
    f(*slice);
}

Meta

rustc --version --verbose:

rustc 1.74.0-nightly (8142a319e 2023-09-13)
binary: rustc
commit-hash: 8142a319ed5c1d1f96e5a1881a6546e463b77c8f
commit-date: 2023-09-13
host: x86_64-unknown-linux-gnu
release: 1.74.0-nightly
LLVM version: 17.0.0

Error output

<output>
Backtrace 

warning: `extern` fn uses type `([u8],)`, which is not FFI-safe
 --> b.rs:4:13
  |
4 | fn bad() -> extern "C" fn(([u8],)) {
  |             ^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
  |
  = help: consider using a struct instead
  = note: tuples have unspecified layout
  = note: `#[warn(improper_ctypes_definitions)]` on by default

thread 'rustc' panicked at compiler/rustc_codegen_llvm/src/abi.rs:360:25:
`PassMode::Direct` for aggregates only allowed on wasm targets
Problematic type: TyAndLayout {
    ty: ([u8],),
    layout: Layout {
        size: Size(0 bytes),
        align: AbiAndPrefAlign {
            abi: Align(1 bytes),
            pref: Align(8 bytes),
        },
        abi: Aggregate {
            sized: false,
        },
        fields: Arbitrary {
            offsets: [
                Size(0 bytes),
            ],
            memory_index: [
                0,
            ],
        },
        largest_niche: None,
        variants: Single {
            index: 0,
        },
        max_repr_align: None,
        unadjusted_abi_align: Align(1 bytes),
    },
}
stack backtrace:
   0:     0x7fbb7af62efc - std::backtrace_rs::backtrace::libunwind::trace::h7d696a3fe83931ea
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:     0x7fbb7af62efc - std::backtrace_rs::backtrace::trace_unsynchronized::h32ed3b706756d880
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x7fbb7af62efc - std::sys_common::backtrace::_print_fmt::h234b1d6e2050b1c7
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/std/src/sys_common/backtrace.rs:67:5
   3:     0x7fbb7af62efc - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hdba83e95158d24b7
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x7fbb7afc915c - core::fmt::rt::Argument::fmt::h56eddd6c4c48048d
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/core/src/fmt/rt.rs:138:9
   5:     0x7fbb7afc915c - core::fmt::write::heb62e4111c27e516
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/core/src/fmt/mod.rs:1094:21
   6:     0x7fbb7af5593e - std::io::Write::write_fmt::hbe56a2148663bb0e
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/std/src/io/mod.rs:1714:15
   7:     0x7fbb7af62ce4 - std::sys_common::backtrace::_print::h17c88990d1e7ffc8
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/std/src/sys_common/backtrace.rs:47:5
   8:     0x7fbb7af62ce4 - std::sys_common::backtrace::print::h18bfa712e4237a54
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/std/src/sys_common/backtrace.rs:34:9
   9:     0x7fbb7af65dda - std::panicking::panic_hook_with_disk_dump::{{closure}}::h29435e023d2fef53
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/std/src/panicking.rs:280:22
  10:     0x7fbb7af65ad5 - std::panicking::panic_hook_with_disk_dump::h747314e0eb3083d0
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/std/src/panicking.rs:314:9
  11:     0x7fbb7e144229 - <rustc_driver_impl[6872457824a876c8]::install_ice_hook::{closure#0} as core[468698755226bf15]::ops::function::FnOnce<(&core[468698755226bf15]::panic::panic_info::PanicInfo,)>>::call_once::{shim:vtable#0}
  12:     0x7fbb7af66693 - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h28a1f6f4d204892e
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/alloc/src/boxed.rs:2021:9
  13:     0x7fbb7af66693 - std::panicking::rust_panic_with_hook::hcbc867ec0eddd0c3
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/std/src/panicking.rs:757:13
  14:     0x7fbb7af66411 - std::panicking::begin_panic_handler::{{closure}}::h3c9eb8c1fbf3ea52
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/std/src/panicking.rs:631:13
  15:     0x7fbb7af63426 - std::sys_common::backtrace::__rust_end_short_backtrace::hd2324a4c2985e4f3
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/std/src/sys_common/backtrace.rs:170:18
  16:     0x7fbb7af66152 - rust_begin_unwind
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/std/src/panicking.rs:619:5
  17:     0x7fbb7afc5505 - core::panicking::panic_fmt::h583d94a3776bd1d2
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/core/src/panicking.rs:72:14
  18:     0x7fbb7cd895bd - <rustc_codegen_ssa[33c9f48d12f8e4aa]::mir::block::TerminatorCodegenHelper>::do_call::<rustc_codegen_llvm[863ddb282f34b5ae]::builder::Builder>
  19:     0x7fbb7cd2d4a4 - rustc_codegen_ssa[33c9f48d12f8e4aa]::mir::codegen_mir::<rustc_codegen_llvm[863ddb282f34b5ae]::builder::Builder>
  20:     0x7fbb7d7985a1 - rustc_codegen_llvm[863ddb282f34b5ae]::base::compile_codegen_unit::module_codegen
  21:     0x7fbb7d7963f5 - rustc_codegen_llvm[863ddb282f34b5ae]::base::compile_codegen_unit
  22:     0x7fbb7d831567 - rustc_codegen_ssa[33c9f48d12f8e4aa]::base::codegen_crate::<rustc_codegen_llvm[863ddb282f34b5ae]::LlvmCodegenBackend>
  23:     0x7fbb7d830f9f - <rustc_codegen_llvm[863ddb282f34b5ae]::LlvmCodegenBackend as rustc_codegen_ssa[33c9f48d12f8e4aa]::traits::backend::CodegenBackend>::codegen_crate
  24:     0x7fbb7d67f702 - <rustc_session[bf85c9c3ca58fcd0]::session::Session>::time::<alloc[5760c3642cffe391]::boxed::Box<dyn core[468698755226bf15]::any::Any>, rustc_interface[89ffa3a3953ed9b2]::passes::start_codegen::{closure#0}>
  25:     0x7fbb7d67f25b - rustc_interface[89ffa3a3953ed9b2]::passes::start_codegen
  26:     0x7fbb7d679baa - <rustc_middle[a5aa27c2964d564a]::ty::context::GlobalCtxt>::enter::<<rustc_interface[89ffa3a3953ed9b2]::queries::Queries>::ongoing_codegen::{closure#0}, core[468698755226bf15]::result::Result<alloc[5760c3642cffe391]::boxed::Box<dyn core[468698755226bf15]::any::Any>, rustc_span[3e6de59a27d5529]::ErrorGuaranteed>>
  27:     0x7fbb7d678fb2 - <rustc_interface[89ffa3a3953ed9b2]::interface::Compiler>::enter::<rustc_driver_impl[6872457824a876c8]::run_compiler::{closure#1}::{closure#2}, core[468698755226bf15]::result::Result<core[468698755226bf15]::option::Option<rustc_interface[89ffa3a3953ed9b2]::queries::Linker>, rustc_span[3e6de59a27d5529]::ErrorGuaranteed>>
  28:     0x7fbb7d6721a4 - std[1d99e526d3fccd95]::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface[89ffa3a3953ed9b2]::util::run_in_thread_with_globals<rustc_interface[89ffa3a3953ed9b2]::interface::run_compiler<core[468698755226bf15]::result::Result<(), rustc_span[3e6de59a27d5529]::ErrorGuaranteed>, rustc_driver_impl[6872457824a876c8]::run_compiler::{closure#1}>::{closure#0}, core[468698755226bf15]::result::Result<(), rustc_span[3e6de59a27d5529]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[468698755226bf15]::result::Result<(), rustc_span[3e6de59a27d5529]::ErrorGuaranteed>>
  29:     0x7fbb7d6718fe - <<std[1d99e526d3fccd95]::thread::Builder>::spawn_unchecked_<rustc_interface[89ffa3a3953ed9b2]::util::run_in_thread_with_globals<rustc_interface[89ffa3a3953ed9b2]::interface::run_compiler<core[468698755226bf15]::result::Result<(), rustc_span[3e6de59a27d5529]::ErrorGuaranteed>, rustc_driver_impl[6872457824a876c8]::run_compiler::{closure#1}>::{closure#0}, core[468698755226bf15]::result::Result<(), rustc_span[3e6de59a27d5529]::ErrorGuaranteed>>::{closure#0}::{closure#0}, core[468698755226bf15]::result::Result<(), rustc_span[3e6de59a27d5529]::ErrorGuaranteed>>::{closure#1} as core[468698755226bf15]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
  30:     0x7fbb7af71075 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::ha4104662ef3c5bbd
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/alloc/src/boxed.rs:2007:9
  31:     0x7fbb7af71075 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::hb8d3ac4a3d377b23
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/alloc/src/boxed.rs:2007:9
  32:     0x7fbb7af71075 - std::sys::unix::thread::Thread::new::thread_start::h1cee3a3217d3e553
                               at /rustc/8142a319ed5c1d1f96e5a1881a6546e463b77c8f/library/std/src/sys/unix/thread.rs:108:17
  33:     0x7fbb7aa8c9eb - <unknown>
  34:     0x7fbb7ab10dfc - <unknown>
  35:                0x0 - <unknown>

error: 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: please attach the file at `/tmp/lol/icemaker_reduced/rustc-ice-2023-09-14T15:15:58.855984188Z-3701302.txt` to your bug report

query stack during panic:
end of query stack
warning: 1 warning emitted

@matthiaskrgr matthiaskrgr added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. F-unsized_fn_params `#![feature(unsized_fn_params)]` labels Sep 14, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 14, 2023
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 14, 2023
@bjorn3
Copy link
Member

bjorn3 commented Sep 15, 2023

cc @RalfJung

@RalfJung
Copy link
Member

RalfJung commented Sep 15, 2023 via email

@matthiaskrgr
Copy link
Member Author

fuzzing :D https://github.com/matthiaskrgr/icemaker/

@RalfJung
Copy link
Member

For the Rust ABI we have logic that makes all unsized arguments indirect:

rust/compiler/rustc_ty_utils/src/abi.rs

Lines 527 to 528 in ae9465f

if arg.layout.is_unsized() || size > Pointer(AddressSpace::DATA).size(cx) {
arg.make_indirect();

For other ABIs however, currently every single target ABI adjustment has to re-implement that.

Honestly I think we should just forbid unsized argument on non-Rust ABIs.

@RalfJung
Copy link
Member

RalfJung commented Sep 15, 2023 •
edited
Loading

Honestly I think we should just forbid unsized argument on non-Rust ABIs.

Oh unfortunately this will be more annoying than I thought. I thought without the feature gate we would reject fn types with unsized arguments, but we don't, this builds on stable:

type T = extern "C" fn(str);
fn foo(_x: T) {}

So we can't just adjust fn WF-checking to reject unsized arguments on non-Rust ABIs, that would be a breaking change. Instead we'll need two checks, one for the caller side (that fixes the reproducer above) and one for the callee side that is required to fix this one

#![feature(unsized_tuple_coercion)]
#![feature(unsized_fn_params)]

pub extern "C" fn foo(_x: str) {}

@RalfJung RalfJung mentioned this issue Oct 29, 2023
Closed
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 29, 2023
@bors
Auto merge of rust-lang#117351 - RalfJung:fn-ptr-sized-wf, r=<try>
ac89e27 
wf: ensure that non-Rust-functions have sized arguments, and everyone has sized return types

I do *not* know what I am doing, I have not touched this code before at all. We do get a bunch of extra diagnostics since we now check every occurrence of a fn ptr type for whether it makes some basic sense, whereas before we only complained when the fn ptr was being called. I think there's probably a way to now remove some of the sizedness check on the HIR side and rely on WF instead, but I couldn't figure it out. This fixes an ICE so maybe it's okay if the diagnostics aren't great from the start.

This is a breaking change if anyone uses the type `fn() -> str` or `extern "C" fn(str)`, or similar bogus function types, anywhere. We should probably crater this.

This does *not* reject the use of the type `fn(str)` on stable, (a) to reduce the amount of breakage, (b) since I don't know if WF-checking can depend on feature flags without causing havoc since a crate with less features might see types from a crate with more features that are suddenly not WF any more, and (c) since it's not required to ensure basic sanity of the ABI. This PR introduces an assertion in our ABI computation logic which checks that the computed ABI makes sense, and for `extern "C" fn(str)` there is no sensible ABI, and we *do* compute the ABI even for function pointers we never call, so we need to start rejecting that code.  `fn(str)`  has a sensible ABI, we just don't make it available on stable.

Fixes rust-lang#115845
@RalfJung RalfJung mentioned this issue Nov 2, 2023
Merged
@matthiaskrgr
Copy link
Member Author

another one: (already fixed with the pr above)

#![crate_type = "lib"]
#![feature(c_unwind, unsized_fn_params)]

#[no_mangle]
pub extern "C" fn rust_item_that_cannot_unwind() {
}

#[no_mangle]
pub extern "system" fn rust_item_that_can_unwind(f: dyn FnOnce()) {
}

bors added a commit to rust-lang-ci/rust that referenced this issue Nov 19, 2023
@bors
Auto merge of rust-lang#117500 - RalfJung:aggregate-abi, r=davidtwco
2ad7da3 
Ensure sanity of all computed ABIs

This moves the ABI sanity assertions from the codegen backend to the ABI computation logic. Sadly, due to past mistakes, we [have to](rust-lang#117351 (comment)) be able to compute a sane ABI for nonsensical function types like `extern "C" fn(str) -> str`.  So to make the sanity check pass we first need to make all ABI adjustment deal with unsized types... and we have no shared infrastructure for those adjustments, so that's a bunch of copy-paste. At least we have assertions failing loudly when one accidentally sets a different mode for an unsized argument.

To achieve this, this re-lands the parts of rust-lang#80594 that got reverted in rust-lang#81388.  To avoid breaking wasm ABI again, that ABI now explicitly opts-in to the (wrong, broken) ABI that we currently keep for backwards compatibility. That's still better than having *every* ABI use the wrong broken default!

Cc `@bjorn3`
Fixes rust-lang#115845
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 19, 2023
@bors
Auto merge of rust-lang#117500 - RalfJung:aggregate-abi, r=davidtwco
7f172a1 
Ensure sanity of all computed ABIs

This moves the ABI sanity assertions from the codegen backend to the ABI computation logic. Sadly, due to past mistakes, we [have to](rust-lang#117351 (comment)) be able to compute a sane ABI for nonsensical function types like `extern "C" fn(str) -> str`.  So to make the sanity check pass we first need to make all ABI adjustment deal with unsized types... and we have no shared infrastructure for those adjustments, so that's a bunch of copy-paste. At least we have assertions failing loudly when one accidentally sets a different mode for an unsized argument.

To achieve this, this re-lands the parts of rust-lang#80594 that got reverted in rust-lang#81388.  To avoid breaking wasm ABI again, that ABI now explicitly opts-in to the (wrong, broken) ABI that we currently keep for backwards compatibility. That's still better than having *every* ABI use the wrong broken default!

Cc `@bjorn3`
Fixes rust-lang#115845
@bors bors closed this as completed in d19980e Nov 19, 2023
bors added a commit to rust-lang/miri that referenced this issue Nov 21, 2023
@bors
Auto merge of #117500 - RalfJung:aggregate-abi, r=davidtwco
9d541f6 
Ensure sanity of all computed ABIs

This moves the ABI sanity assertions from the codegen backend to the ABI computation logic. Sadly, due to past mistakes, we [have to](rust-lang/rust#117351 (comment)) be able to compute a sane ABI for nonsensical function types like `extern "C" fn(str) -> str`.  So to make the sanity check pass we first need to make all ABI adjustment deal with unsized types... and we have no shared infrastructure for those adjustments, so that's a bunch of copy-paste. At least we have assertions failing loudly when one accidentally sets a different mode for an unsized argument.

To achieve this, this re-lands the parts of rust-lang/rust#80594 that got reverted in rust-lang/rust#81388.  To avoid breaking wasm ABI again, that ABI now explicitly opts-in to the (wrong, broken) ABI that we currently keep for backwards compatibility. That's still better than having *every* ABI use the wrong broken default!

Cc `@bjorn3`
Fixes rust-lang/rust#115845
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
@bors
Auto merge of #117500 - RalfJung:aggregate-abi, r=davidtwco
6e5b090 
Ensure sanity of all computed ABIs

This moves the ABI sanity assertions from the codegen backend to the ABI computation logic. Sadly, due to past mistakes, we [have to](rust-lang/rust#117351 (comment)) be able to compute a sane ABI for nonsensical function types like `extern "C" fn(str) -> str`.  So to make the sanity check pass we first need to make all ABI adjustment deal with unsized types... and we have no shared infrastructure for those adjustments, so that's a bunch of copy-paste. At least we have assertions failing loudly when one accidentally sets a different mode for an unsized argument.

To achieve this, this re-lands the parts of rust-lang/rust#80594 that got reverted in rust-lang/rust#81388.  To avoid breaking wasm ABI again, that ABI now explicitly opts-in to the (wrong, broken) ABI that we currently keep for backwards compatibility. That's still better than having *every* ABI use the wrong broken default!

Cc `@bjorn3`
Fixes rust-lang/rust#115845
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
@bors
Auto merge of #117500 - RalfJung:aggregate-abi, r=davidtwco
6a10bed 
Ensure sanity of all computed ABIs

This moves the ABI sanity assertions from the codegen backend to the ABI computation logic. Sadly, due to past mistakes, we [have to](rust-lang/rust#117351 (comment)) be able to compute a sane ABI for nonsensical function types like `extern "C" fn(str) -> str`.  So to make the sanity check pass we first need to make all ABI adjustment deal with unsized types... and we have no shared infrastructure for those adjustments, so that's a bunch of copy-paste. At least we have assertions failing loudly when one accidentally sets a different mode for an unsized argument.

To achieve this, this re-lands the parts of rust-lang/rust#80594 that got reverted in rust-lang/rust#81388.  To avoid breaking wasm ABI again, that ABI now explicitly opts-in to the (wrong, broken) ABI that we currently keep for backwards compatibility. That's still better than having *every* ABI use the wrong broken default!

Cc `@bjorn3`
Fixes rust-lang/rust#115845
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. F-unsized_fn_params `#![feature(unsized_fn_params)]` I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
5 participants
@RalfJung @matthiaskrgr @saethlin @bjorn3 @rustbot