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

Unalign ABI promises are wrong #164

Closed
Noratrieb opened this issue Mar 17, 2023 · 3 comments · Fixed by #168
Closed

Unalign ABI promises are wrong #164

Noratrieb opened this issue Mar 17, 2023 · 3 comments · Fixed by #168

Comments

@Noratrieb
Copy link

Noratrieb commented Mar 17, 2023

https://docs.rs/zerocopy/latest/zerocopy/struct.Unalign.html states

Unalign<T> has the same size and ABI as T

This is wrong, as the type is #[repr(C, packed)] and not repr(transparent).

The Rust reference about repr(transparent):

This is different than the C representation because a struct with the C representation will always have the ABI of a C struct while, for example, a struct with the transparent representation with a primitive field will have the ABI of the primitive field.

Here's an example of such an ABI mismatch in practice: https://godbolt.org/z/bhP71dnh4

The docs should be relaxed to not promise full ABI compatibility, but only promise what repr(C) promises (which is probably strong enough for most use cases).

@Lokathor
Copy link

Specifically, the memory layout will still be the same as that of the wrapped type, but the function calling convention can differ.

@chorman0773
Copy link

This seems like it should also affect any x86-64 Sys-V platform.

When determining the class of structures,

If the size of an object is larger than four eightbytes, or it contains unaligned
fields, it has class MEMORY.

joshlf added a commit that referenced this issue Apr 26, 2023
@joshlf
Copy link
Member

joshlf commented Apr 26, 2023

Good catch, thanks @Nilstrieb! Uploaded a fix here.

joshlf added a commit that referenced this issue Apr 26, 2023
joshlf added a commit that referenced this issue Apr 26, 2023
joshlf added a commit that referenced this issue Aug 3, 2023
joshlf added a commit that referenced this issue Aug 3, 2023
@joshlf joshlf mentioned this issue Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants