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

Test variadic fnptr uses incompatible definition and declaration #66690

Closed
bjorn3 opened this issue Nov 24, 2019 · 1 comment · Fixed by #95088
Closed

Test variadic fnptr uses incompatible definition and declaration #66690

bjorn3 opened this issue Nov 24, 2019 · 1 comment · Fixed by #95088
Labels
A-cranelift Things relevant to the [future] cranelift backend A-ffi Area: Foreign Function Interface (FFI) A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@bjorn3
Copy link
Member

bjorn3 commented Nov 24, 2019

#[test]
#[allow(warnings)]
// Have a symbol for the test below. It doesn’t need to be an actual variadic function, match the
// ABI, or even point to an actual executable code, because the function itself is never invoked.
#[no_mangle]
pub fn test_variadic_fnptr() {
use core::hash::{Hash, SipHasher};
extern {
fn test_variadic_fnptr(_: u64, ...) -> f64;
}
let p: unsafe extern fn(u64, ...) -> f64 = test_variadic_fnptr;
let q = p.clone();
assert_eq!(p, q);
assert!(!(p < q));
let mut s = SipHasher::new();
assert_eq!(p.hash(&mut s), q.hash(&mut s));
}

This causes a crash when trying to test libcore with cg_clif, as Cranelift doesn't allow incompatible definitions of the same function. While this currently works for cg_llvm, according to rust-lang/unsafe-code-guidelines#198 it is likely to become forbidden with cg_llvm too.

@jonas-schievink jonas-schievink added A-ffi Area: Foreign Function Interface (FFI) A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 24, 2019
@bjorn3
Copy link
Member Author

bjorn3 commented Mar 12, 2020

@rustbot modify labels:+A-cranelift

@rustbot rustbot added the A-cranelift Things relevant to the [future] cranelift backend label Mar 12, 2020
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Mar 19, 2022
…tolnay

Don't declare test_variadic_fnptr with two conflicting signatures

It is UB for LLVM and results in a compile error for Cranelift.

cc https://github.com/bjorn3/rustc_codegen_cranelift/issues/806
Fixes rust-lang#66690
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 19, 2022
…tolnay

Don't declare test_variadic_fnptr with two conflicting signatures

It is UB for LLVM and results in a compile error for Cranelift.

cc https://github.com/bjorn3/rustc_codegen_cranelift/issues/806
Fixes rust-lang#66690
@bors bors closed this as completed in 2b50739 Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cranelift Things relevant to the [future] cranelift backend A-ffi Area: Foreign Function Interface (FFI) A-testsuite Area: The testsuite used to check the correctness of rustc C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants