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 EM_RISCV constant #71

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nomadium
Copy link

Hi folks,

I was working with some RISC-V ELF binaries and I thought it would be convenient to have them properly recognised by this library so I'm submitting a small change to improve RISC-V architecture support.

Thanks.


References:

@@ -436,6 +437,7 @@ def self.mapping(val)
when EM_IA_64 then 'Intel IA-64'
when EM_AARCH64 then 'AArch64'
when EM_X86_64 then 'Advanced Micro Devices X86-64'
when EM_RISCV then 'RISC-V'
Copy link
Owner

Choose a reason for hiding this comment

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

need to enhance the tests to cover this line

@david942j
Copy link
Owner

david942j commented Feb 22, 2023

Thanks for the PR! Only one feedback that I want the coverage be kept as 100%, please enhance the tests to cover the new line.

@nomadium
Copy link
Author

Thanks for the feedback. Sure, no problem, I'll update the PR soon.

@nomadium
Copy link
Author

@david942j it took me ages to add unit tests to this PR but hey, such is life.

My only concern at the moment it's that I added a binary (it's just a hello world) riscv64 file to the repo without a clear way how to build it.

I could update the Makefile to add a cross-compiler in similar way to what is done here:

https://github.com/popovicu/risc-v-bare-metal-fake-bios/blob/main/Makefile

But then this small library would be depending on the whole riscv64 toolchain and I don't think that's the right path.

@david942j
Copy link
Owner

How about adding a Dockerfile that could include all needed toolchain?

@nomadium
Copy link
Author

I like that idea, do you have an example of another ruby gem shipping a Dockerfile?

@david942j
Copy link
Owner

You don't need a Ruby gem, just provide a Dockerfile under spec/files along with a README to describe the sequence of commands to generate the riscv binary (i.e. docker build; docker run... docker exec gcc ...). The Dockerfile can be based on the latest Ubuntu image and install required cross compiler.

As long as I can follow the README you provided to generate the same binary it's good enough :)

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