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 Reader::Address associated type #409

Open
philipc opened this issue Mar 13, 2019 · 5 comments · May be fixed by #735
Open

Add Reader::Address associated type #409

philipc opened this issue Mar 13, 2019 · 5 comments · May be fixed by #735

Comments

@philipc
Copy link
Collaborator

philipc commented Mar 13, 2019

This should support the following uses:

  • better support for relocated addresses (eg when using function sections, which have every function symbol at address 0)
  • reuse of the same address type for conversion when both reading and writing
  • optional support for segmented addresses (Support non-zero segment_selector_size #357)
  • using the native address size
  • better type checking of our address handling

I wanted this when doing the relocated address support, but didn't do it because of how ugly the Reader::Offset support had been. However, I don't think that's so bad now after the default offset was changed in #392.

Any thoughts on this @fitzgen ?

@fitzgen
Copy link
Member

fitzgen commented Mar 13, 2019

Sounds good to me! At the least it would be nice not to force u64 upon everyone.

@vaibspider
Copy link
Contributor

Could you please provide an estimate of the efforts required to implement this functionality?
If this is going to help relocatable addresses handling, I wish to contribute to this issue. Could you please suggest a good starting point?
Thanks.

@philipc
Copy link
Collaborator Author

philipc commented Apr 3, 2020

@vaibspider It's not a beginner task, and will require widespread changes to the read module. Unless someone has better ideas, the process will be:

  • define a trait for operations that addresses must support (initially it won't need any operations, but we'll add them as required in the process of fixing everything)
  • implement this trait for u64
  • add the Reader::Address associated type
  • figure out how Reader is going to convert an address-sized word into Reader::Address (this is harder than it was for offsets because the conversion may require some extra state)
  • change Reader::read_address to return Reader::Address
  • fix everything that uses Reader::read_address (which will require many flow-on changes).

I'm not sure if there is any way to break that last step into something more manageable.

Note that in theory relocatable addresses should already be possible. This issue would just make it nicer. So unless you're keen to do this, I would try to use the existing method. I'm currently updating my rewrite repository to check that this all works still.

@vaibspider
Copy link
Contributor

Thanks for the insight! I tested reading a couple of relocatable object files, regenerating them (using object library) and linking them to get an executable. I noticed that object library worked fine with a relocatable object file but had issues with an executable file(couldn't reproduce all the contents and couldn't create an executable file). On the other hand, gimli seems to be working fine with executable files (I can read DWARF sections and modify them) but has issues when given a relocatable object file as input - (getting InvalidAttributeValue error). Could you please provide your input on this?
Thanks.

@philipc
Copy link
Collaborator Author

philipc commented Apr 3, 2020

Replying in #482.

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 a pull request may close this issue.

3 participants