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

Support Aarch64Call relocations for Mach-O #465

Merged
merged 2 commits into from Oct 4, 2022

Conversation

nathanwhit
Copy link
Contributor

@nathanwhit nathanwhit commented Sep 19, 2022

This PR adds support for relocations with RelocationEncoding::Aarch64Call on Mach-O. Before this PR we would error out, as the relocation has a size of 26 which was unhandled, and we weren't setting the appropriate r_type. After this PR we support relocations with RelocationEncoding::Aarch64Call and an arbitrary addend.

This PR also handles a few extra cases of the relocation size (21 and 12) in order to support the usage of Mach-O specific relocations like ARM64_RELOC_PAGE21 and ARM64_RELOC_PAGEOFF12, respectively.

@philipc
Copy link
Contributor

philipc commented Sep 19, 2022

Thanks! Based on a brief look this is good. Where was the panic occurring?

@nathanwhit
Copy link
Contributor Author

nathanwhit commented Sep 19, 2022

The original panic was at

_ => return Err(Error(format!("unimplemented reloc size {:?}", reloc))),
Once that was fixed, it was hitting this case
return Err(Error(format!("unimplemented relocation {:?}", reloc)));

@philipc
Copy link
Contributor

philipc commented Sep 19, 2022

Ah okay, so not panics, just returning errors (maybe your crate was unwrapping that error).

@nathanwhit
Copy link
Contributor Author

nathanwhit commented Sep 19, 2022

Ah sorry, of course, don't know how I managed to mix that up. (The panic I was seeing was from cranelift unwrapping that error, here to be precise).

src/write/mod.rs Outdated Show resolved Hide resolved
src/write/macho.rs Outdated Show resolved Hide resolved
src/write/macho.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@philipc philipc left a comment

Choose a reason for hiding this comment

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

Thanks! I'll merge this after the unrelated archive code has been fixed.

@nathanwhit
Copy link
Contributor Author

(Rebased so that CI passes now that the formatting issue has been fixed on master)

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.

None yet

2 participants