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

ICE when casting extern "C" fn when it has a non-FFI-safe argument. #52334

Closed
purpleposeidon opened this issue Jul 13, 2018 · 9 comments · Fixed by #122895
Closed

ICE when casting extern "C" fn when it has a non-FFI-safe argument. #52334

purpleposeidon opened this issue Jul 13, 2018 · 9 comments · Fixed by #122895
Labels
A-FFI Area: Foreign function interface (FFI) C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. glacier ICE tracked in rust-lang/glacier. 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

@purpleposeidon
Copy link

type Foo = extern "C" fn(::std::ffi::CStr);
extern "C" {
    fn meh(blah: Foo);
}

fn main() {
    meh as usize;
}

Playground: https://play.rust-lang.org/?gist=3fa8a28e06dc77fb478a33e872348f15&version=stable&mode=debug&edition=2015

@oli-obk oli-obk added the I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ label Jul 13, 2018
@csmoe
Copy link
Member

csmoe commented Jul 13, 2018

the ice can be suppressed with reference prefix: &::std::ffi::CStr

@jonas-schievink jonas-schievink added A-FFI Area: Foreign function interface (FFI) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 6, 2019
@rust-lang-glacier-bot rust-lang-glacier-bot added the glacier ICE tracked in rust-lang/glacier. label Oct 15, 2019
@Centril
Copy link
Contributor

Centril commented Mar 10, 2020

Still reproduces:

Standard Error

   Compiling playground v0.0.1 (/playground)
warning: `extern` block uses type `std::ffi::CStr`, which is not FFI-safe
 --> src/main.rs:3:18
  |
3 |     fn meh(blah: Foo);
  |                  ^^^ not FFI-safe
  |
  = note: `#[warn(improper_ctypes)]` on by default
  = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
  = note: this struct has unspecified layout

thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src/librustc_target/abi/call/x86_64.rs:158:14
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.44/src/backtrace/libunwind.rs:86
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.44/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:78
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1063
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1428
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:204
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:224
  10: rustc_driver::report_ice
  11: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:474
  12: rust_begin_unwind
             at src/libstd/panicking.rs:378
  13: core::panicking::panic_fmt
             at src/libcore/panicking.rs:85
  14: core::panicking::panic
             at src/libcore/panicking.rs:52
  15: rustc_target::abi::call::x86_64::cast_target
  16: rustc_target::abi::call::x86_64::compute_abi_info::{{closure}}
  17: <rustc_target::abi::call::FnAbi<&rustc::ty::TyS> as rustc::ty::layout::FnAbiExt<C>>::adjust_for_abi
  18: <rustc_target::abi::call::FnAbi<&rustc::ty::TyS> as rustc::ty::layout::FnAbiExt<C>>::of_fn_ptr
  19: <rustc_target::abi::TyLayout<&rustc::ty::TyS> as rustc_codegen_llvm::type_of::LayoutLlvmExt>::llvm_type
  20: <rustc_target::abi::call::FnAbi<&rustc::ty::TyS> as rustc_codegen_llvm::abi::FnAbiLlvmExt>::llvm_type
  21: rustc_codegen_llvm::callee::get_fn
  22: rustc_codegen_ssa::mir::rvalue::<impl rustc_codegen_ssa::mir::FunctionCx<Bx>>::codegen_rvalue_operand
  23: rustc_codegen_ssa::mir::block::<impl rustc_codegen_ssa::mir::FunctionCx<Bx>>::codegen_block
  24: rustc_codegen_ssa::mir::codegen_mir
  25: <rustc::mir::mono::MonoItem as rustc_codegen_ssa::mono_item::MonoItemExt>::define
  26: rustc_codegen_llvm::base::compile_codegen_unit::module_codegen
  27: rustc::dep_graph::graph::DepGraph::with_task
  28: rustc_codegen_llvm::base::compile_codegen_unit
  29: rustc_codegen_ssa::base::codegen_crate
  30: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_utils::codegen_backend::CodegenBackend>::codegen_crate
  31: rustc_interface::passes::start_codegen
  32: rustc::ty::context::tls::enter_global
  33: rustc_interface::queries::Queries::ongoing_codegen
  34: rustc_interface::interface::run_compiler_in_existing_thread_pool
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

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/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.43.0-nightly (3dbade652 2020-03-09) running on x86_64-unknown-linux-gnu

note: compiler flags: -C codegen-units=1 -C debuginfo=2 --crate-type bin

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

query stack during panic:
end of query stack
error: could not compile `playground`.

To learn more, run the command again with --verbose.

cc @eddyb @hanna-kruppe @nagisa

@hanna-kruppe
Copy link
Contributor

Some smaller examples, each of which seem to produce the same ICE (any by-value DST in place of str works, I just picked str for brevity):

pub fn blah(_: extern "C" fn(str)) {}
pub fn blah(_: extern "C" fn() -> str) {}
pub extern "C" fn blah(_: str) {} // requires #![feature(unsized_locals)]

There seem to be two related problems here:

  • function pointers and FFI imports are accepted even if their signature is something no actual function definition permits (all of the above without unsized_locals, and even with that feature you can't return DSTs), and the ABI code doesn't expect that / can't assign any sensible LLVM type to such function pointers
  • feature(unsized_locals) allows taking unsized arguments even in non-Rust ABIs, but there's no codegen support for that

The first could potentially be solved by making up some dummy ABI just to avoid crashing, but if we can get away with it, we could also try to reject such function pointers. We should also see if we can reject corresponding FFI imports (such as meh in the original example).

The interactions between unsized_locals and non-Rust ABIs is not obvious, so we should probably just limit the scope of that feature gate. We might want to support unsized arguments in more ABIs on principle, but as long as there's no use case and no codegen support, it just invites ICEs.

@oliviacrain
Copy link
Contributor

oliviacrain commented Oct 28, 2020

Glacier is reporting that this ICE will not reproduce as of 1.49.0-nightly 2020-10-27. I was able to bisect this fix and confirm that 2020-10-27 is the first version this ICE is fixed for, but I can't seem to bisect to the particular PR that fixed it.

@nagisa nagisa added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Oct 28, 2020
@Alexendoo
Copy link
Member

#77876 caused the original example to not ICE, however it is still there if you compile it with -Z mir-opt-level=0

@Alexendoo Alexendoo removed the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Oct 28, 2020
@fanninpm
Copy link

@JohnTitor This does not cause an ICE on an M1 Mac:

ices/52334.sh: fixed with no errors
=== stdout ===
=== stderr ===
warning: `extern` block uses type `CStr`, which is not FFI-safe
 --> <anon>:3:18
  |
3 |     fn meh(blah: Foo);
  |                  ^^^ not FFI-safe
  |
  = note: `#[warn(improper_ctypes)]` on by default
  = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct
  = note: this struct has unspecified layout

warning: 1 warning emitted

@JohnTitor
Copy link
Member

@fanninpm Still reproducible on my end (and on glacier as well), it's likely your env issue.

@fanninpm
Copy link

@JohnTitor it may have to do with architecture, because I can confirm this on an x64 architecture CPU running Manjaro.

@JohnTitor
Copy link
Member

Triage: The ICE has been fixed, marking as E-needs-test
@rustbot labels: +E-needs-test

@rustbot rustbot added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Jul 8, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 22, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 23, 2024
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Mar 24, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 24, 2024
@bors bors closed this as completed in 14d05c4 Mar 24, 2024
RenjiSann pushed a commit to RenjiSann/rust that referenced this issue Mar 25, 2024
RenjiSann pushed a commit to RenjiSann/rust that referenced this issue Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-FFI Area: Foreign function interface (FFI) C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. glacier ICE tracked in rust-lang/glacier. 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
Development

Successfully merging a pull request may close this issue.