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

Add basic support for mips64/powerpc/powerpc64/riscv64 #303

Merged
merged 3 commits into from
May 21, 2021

Conversation

alexcrichton
Copy link
Contributor

@alexcrichton
Copy link
Contributor Author

I also went ahead and added SHF_EXCLUDE to the constants list for elf.rs as well since it's used by rustc.

S390x,
Wasm32,
PowerPc,
PowerPc64,
Riscv64,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add riscv32 too?

src/elf.rs Outdated Show resolved Hide resolved
@philipc
Copy link
Contributor

philipc commented May 18, 2021

Would Architecture::None be appropriate for rust-lang/rust#84449, so that you can support all architectures? Not sure if that is too much of a hack.

@alexcrichton
Copy link
Contributor Author

I originally tried that actually but it panics in too many locations during object file writing, so I wasn't able to use it. (assuming you mean the Unknown variant). Since the object files make their way to the linker I think they need to have at least the right header as well, so I don't think that a placeholder will work in this case.

@philipc
Copy link
Contributor

philipc commented May 18, 2021

We would have to add a None variant that corresponds exactly to EM_NONE. The existing Unknown means it could be any unrecognized value, and so it isn't suitable for writing. But yeah the question is whether the linker would be happy with that. I wasn't sure if it mattered since the linker doesn't need anything from the file anyway.

@alexcrichton
Copy link
Contributor Author

Ah I see! I wasn't aware of EM_NONE. (honestly I wasn't aware of basically any of this until now)

I tested out with object files that have EM_NONE but the linker (at least for s390x and mips) rejected the object files and printed an error. I think that means that we probably need to emit the right shape of object file in rustc at least. This isn't really too much of a problem though I think, there aren't really all that many architectures and we're only writing the bare minimum of an object file (just one data section with SHF_EXCLUDE)

@philipc
Copy link
Contributor

philipc commented May 20, 2021

I didn't find anything else that uses EM_NONE, so in hindsight that was unlikely to work.

I think this is good to merge if you can address bjorn3's comments.

@alexcrichton
Copy link
Contributor Author

Sure thing, done now!

@philipc philipc merged commit 8fbc81e into gimli-rs:master May 21, 2021
@philipc
Copy link
Contributor

philipc commented May 21, 2021

Let me know if you need a release for this any time soon.

@alexcrichton alexcrichton deleted the more-arch branch May 21, 2021 02:19
@alexcrichton
Copy link
Contributor Author

Ah yeah if you're up for it that'd be great, this'll be needed to land the rustc pr

@philipc
Copy link
Contributor

philipc commented May 21, 2021

Ok, this technically a breaking change. We already require rust 1.42 so I'll add in some non_exhaustive for future changes.

@philipc
Copy link
Contributor

philipc commented Jun 2, 2021

0.25.0 is published.

@alexcrichton
Copy link
Contributor Author

Thanks! 👍

mcbegamerxx954 pushed a commit to mcbegamerxx954/object that referenced this pull request Jun 15, 2024
Add basic support for mips64/powerpc/powerpc64/riscv64
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.

3 participants