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

[MachO] Decl alignment #91

Merged
merged 7 commits into from
Sep 4, 2019
Merged

[MachO] Decl alignment #91

merged 7 commits into from
Sep 4, 2019

Conversation

bjorn3
Copy link
Contributor

@bjorn3 bjorn3 commented Aug 24, 2019

The alignment of data got ignored before.

Fixes #87

@bjorn3 bjorn3 marked this pull request as ready for review August 24, 2019 15:11
Copy link
Owner

@m4b m4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow this looked like a lot of work! Is there anyway we can add some kind of test for this ?

src/mach.rs Show resolved Hide resolved
@bjorn3
Copy link
Contributor Author

bjorn3 commented Aug 24, 2019

Wow this looked like a lot of work!

Several hours. (including finding amd reading machO docs and experimentally figuring out the correct way to pad the data)

Is there anyway we can add some kind of test for this ?

Maybe run objdump -rs to get section data and reloctions and then compare it against a fixed output. We could also add a main function to the prototype output and check alignment in there. Another would be running bingrep and checking all symbols are 8/16 byte aligned.

@m4b
Copy link
Owner

m4b commented Aug 25, 2019

I'll squash and merge once the comment about why 0xaa is in

@bjorn3
Copy link
Contributor Author

bjorn3 commented Aug 25, 2019

This is a breaking change as the alignment is now specified using an u64 instead of usize. I don't necessarily need a new release published, as the breaking change means that I need to update cranelift, which is blocked on bytecodealliance/cranelift#795.

@pchickey pchickey merged commit d1bf163 into m4b:master Sep 4, 2019
@pchickey
Copy link
Collaborator

pchickey commented Sep 4, 2019

Published in 0.11.0

@bjorn3 bjorn3 deleted the mach_o_alignment branch September 4, 2019 19:24
bjorn3 added a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request Sep 14, 2019
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.

Mach-O and ELF backend use incompatible definitions of alignment
3 participants