Skip to content

Conversation

@swurzinger
Copy link
Contributor

Fixes #858: C backend: fix internal structure size mismatch due to wrong padding when nesting packed structures

The alignment of nested structures which are packed with a smaller alignment than the natural or specified alignment of the parent structure has to be specified explicitly, otherwise the field will be packed too.

… differences

A structure which decreases its alignment (using FIELD = n) is annotated with both the "packed" and the "alignment(n)" attributes. That leads to a problem if such a structure is nested inside another structure that uses a greater alignment than the child structure. In that case the field representing the nested structure is aligned with the same properties as the nested structure.

So the alignment has to be increased explicitly if the nested structure is packed with a smaller alignment than the parent structure. That is accomplished by adding the "alignment(n)" attribute to those fields.
@swurzinger
Copy link
Contributor Author

I changed the test to work on systems with 4-byte QWord-alignment (like Linux) rather than 8-byte alignment.

@dkl
Copy link
Member

dkl commented Oct 23, 2018

I tested the old test case (with long+longint) from SourceForge ticket #858, and it indeed causes the error with -gen gcc for linux-x86_64, win32 and win64 (not for linux-x86). But the new test case (with byte+short) seems to work fine even with fbc 1.05.0.

We should probably keep both as regression tests. ABI differences between platforms can be handled with #if statements (e.g. different #asserts for each target) in the test case.

@swurzinger
Copy link
Contributor Author

swurzinger commented Oct 23, 2018

I tried on win64 only and it seems that's the only platform where the modified test works.

@swurzinger
Copy link
Contributor Author

I also tried on win32 and ubuntu-x86_64 and could reproduce the issue with the new test code as well. Are you sure that you cannot reproduce it on these platforms with the latest changes?

Maybe I should rather create a log-test instead of a unit-test?

@dkl
Copy link
Member

dkl commented Oct 24, 2018

Hmm, my mistake, I tested with just the type declarations, without the Dim statement using the type, so the struct didn't get emitted by -gen gcc. After fixing that everything works (fails) as expected.

@dkl dkl merged commit 2204d14 into freebasic:master Oct 24, 2018
@swurzinger
Copy link
Contributor Author

The issue addressed by this PR, https://sourceforge.net/p/fbc/bugs/858/, can be closed, I think.

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.

2 participants