-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
[i128] C FFI mismatch on GNU/Linux with gcc #38762
Comments
|
The value is passed correctly to the C side, but after return memcpy value becomes wrong. These are the signatures at the LLVM level: ; C
define void @foo(%struct.Foo* noalias nocapture sret, %struct.Foo* byval align 16) local_unnamed_addr #0;
; rust
declare void @foo(%Foo* noalias nocapture sret dereferenceable(24), %Foo* byval noalias nocapture dereferenceable(24)) unnamed_addr #2; Concerns:
What happens here is that the memcpy on C side overwrites the padding that does not exist on the rust side. That’s how rust instructs LLVM to allocate stack:
Then C side does The fix here is to calculate alignment correctly, which I seem to have messed up. Ugh. #[repr(C)]
struct Foo {
a: i128,
b: i8,
c: u16,
}
fn main() {
assert_eq!(16, ::std::mem::align_of::<Foo>()); // fails currently
} |
clang has this code snippet which ignores the data-layout of the target (probably to match sysv64 ABI. Also potentially has buggy interactions with the LLVM expectations?). We can’t quite do that because of various assertions that the sizes and alignments match the data-layout. Any suggestions @eddyb? Is changing data-layout string an option here? EDIT: Oh |
We can add extra alignment attributes through |
#38870 fixes the by-value case, but struct Foo foo(struct Foo *a) {
return *a;
}
will still sigsegv(!) for the same reasons if both rustc and C sides are compiled with opts on my machine. |
LLVM patch has landed as llvm-mirror/llvm@d11a6cc, however only for x86_64 and PPC64, so we might eventually run into a similar issue on more obscure platforms later. Not sure if the patch is important enough to backport into our branch as opposed to just waiting for LLVM 4.0 to get released. EDIT: it got backed out; clang sucks. |
Given the following setup:
test.rs:
test.c:
gcc version
6.2.0
, rust of current master (ac5cd3b), and a 64 bit gnu/linux platform, main will panic:cc #35118 (tracking issue)
cc @nagisa , me
The text was updated successfully, but these errors were encountered: