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

Unbreak build on x86 #35

Closed
wants to merge 1 commit into from
Closed

Unbreak build on x86 #35

wants to merge 1 commit into from

Conversation

jbeich
Copy link

@jbeich jbeich commented Jan 14, 2019

Fixes #34

src/test.h:37:1: fatal error: static_assert failed
static_assert(sizeof(TestCase) == 4312);
^             ~~~~~~~~~~~~~~~~~~~~~~~~
@wwylele
Copy link
Owner

wwylele commented Jan 15, 2019

Do you know where the layout difference come from? If it is a missing/extra padding in between some members, adding padding field at the end is incorrect; if it is just size difference due to not respecting u64 8-byte alignment, alignas(8) might be a better way to go.

@wwylele
Copy link
Owner

wwylele commented Jul 31, 2019

Closing due to inactivity

@wwylele wwylele closed this Jul 31, 2019
@jbeich
Copy link
Author

jbeich commented Sep 16, 2019

I confirm, adding alignas(8) helps. Can you reopen or cherry-pick 5862fd0?

@wwylele
Copy link
Owner

wwylele commented Sep 16, 2019

Github doesn't allow me to reopen this. Please open a new PR and let CI check stuff because I currently don't have the dev environment set up.

@wwylele
Copy link
Owner

wwylele commented Sep 16, 2019

Also adding alignas(8) on the top level struct isn't always correct either. It ensures the correct total size, but doesn't ensure the inner layout (for example all fields after a u64 field might be offset by 4 bytes). It should be added to all u64-type fields. A simple example:

struct { u32 a; u64 b; u32 c; u32 d;}

This struct on x64 would be layout as

| a: 4 bytes | padding: 4 bytes | b: 8 bytes | c: 4 bytes | d: 4 bytes |

which is 24 bytes in total.

On x86 where u64 alignment is 4-byte, it would be layout as

| a: 4 bytes | b: 8 bytes | c: 4 bytes | d: 4 bytes |

which is 20 bytes in total.

Adding alignas(8) on the top level (alignas(8) struct { u32 a; u64 b; u32 c; u32 d;}):

| a: 4 bytes | b: 8 bytes | c: 4 bytes | d: 4 bytes | padding: 4 bytes |

which is 24 bytes in total, but has a different layout from the x64 one (notice the different position of the padding)

Adding alignas(8) on the field (struct { u32 a; alignas(8) u64 b; u32 c; u32 d;}):

| a: 4 bytes | padding: 4 bytes | b: 8 bytes | c: 4 bytes | d: 4 bytes |

This produce the same layout as the x64 one

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 this pull request may close these issues.

Fails to build on x86
2 participants