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

Allow remapping all of line program when rewriting #563

Open
RReverser opened this issue Jun 20, 2021 · 5 comments
Open

Allow remapping all of line program when rewriting #563

RReverser opened this issue Jun 20, 2021 · 5 comments

Comments

@RReverser
Copy link
Contributor

When modifying source locations in the binary (like in Wasm use-cases), it's desirable to rewrite all of the debugging information to match the new addresses.

Dwarf::from accepts convert_address, which seems like a good place for doing such rewriting, but currently when it reaches the LineProgram conversion, it only executes the callback for the base address (SetAddress) instruction and not for individual offsets after that, making granular per-instruction address updates impossible.

One way to work around this issue would be to get mutable access to the instructions property of gimli::write::LineProgram and walk over instructions and updates offsets manually, but currently that property isn't exposed either.

What would be the best way to allow such granular rewrites? Should we call convert_address on each individual instruction and recalculate offsets, or would it be better to add a separate callback to the API, or expose the instructions mutable iterator?

@RReverser
Copy link
Contributor Author

Note: this is related to this comment: rustwasm/walrus#71 (comment) by @philipc.

@RReverser
Copy link
Contributor Author

RReverser commented Jun 20, 2021

Should we call convert_address on each individual instruction and recalculate offsets

FWIW this seems most reasonable IMO, and doesn't require any API changes, but I'm not sure if there are any use-cases it would break (like relocations perhaps?).

@philipc
Copy link
Collaborator

philipc commented Jun 20, 2021

I would like to note that convert_address currently isn't intended for the kind of transformation that walrus needs. It exists only because the write module needs addresses in a different form from the read module, and if #409 was implemented then it wouldn't be needed at all.

From the comment you linked:

I had originally thought that transformations would work by calling gimli's convert, and then mutating the resulting write::Dwarf

So following that approach, the solution is:

One way to work around this issue would be to get mutable access to the instructions property of gimli::write::LineProgram and walk over instructions and updates offsets manually, but currently that property isn't exposed either.

If that approach isn't suitable then we can certainly consider other solutions, such as a framework for transforming anything (not just addresses) during conversion, and the existing conversion would be the identity transformation.

Or another option may be to make it easier to use the existing conversion as a library for converting individual entries, rather than simply converting entire sections at once.

@RReverser
Copy link
Contributor Author

So for now I played with a branch to implement conversion directly in convert_address - that works somewhat fine, but tricky to extend correctly to relocations.

But yeah personally I think

One way to work around this issue would be to get mutable access to the instructions property of gimli::write::LineProgram and walk over instructions and updates offsets manually, but currently that property isn't exposed either.

would be just as fine a solution, my only concern is the comment on that property which suggests that maybe the instructions property shouldn't exist at all and should be replaced with rows representation instead. If we expose it as public, I suspect it will be much harder to do this kind of refactoring in future.

Or maybe if we expose it purely as iterator (fn() -> impl Iterator<Item = &mut Instruction>) then it's fine?

@philipc
Copy link
Collaborator

philipc commented Jun 21, 2021

I think impl Iterator would be fine. The reason for that comment is to allow transformations such as deleting a sequence, or adding/deleting/reordering rows.

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

No branches or pull requests

2 participants