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

write: implement references within expressions #479

Merged
merged 7 commits into from
Apr 28, 2020

Conversation

philipc
Copy link
Collaborator

@philipc philipc commented Mar 30, 2020

Changes write::Expression from Vec<u8> to Vec<Operation>, and correctly handles references in the operations. The main changes of this PR are in the last commit. The other commits are mostly refactoring to make it easier to implement.

Fixes #474
Partial support for #478 (still need a way to assign symbols to DIEs)

@wwylele This is untested and still lacking a nicer API for adding operations. I'm unsure when I'll finish it off, but try it out if you want.

@coveralls

This comment has been minimized.

@wwylele
Copy link

wwylele commented Mar 30, 2020

I tried it and it worked pretty well 👍

Does not yet support them during conversion though.
Split the .debug_info conversion into two passes, one for entries
and one for attributes.

This allows us to convert attributes directly, instead of needing the
temporary AttributeValue::UnitSectionRef, and will also better support
.debug_info references in expressions.
@philipc philipc force-pushed the issue-474 branch 3 times, most recently from f18f9f1 to daa8a39 Compare April 27, 2020 06:22
This makes it easier to access the raw operation values.
This makes it easier to access the raw operation values.
This is required to support references to entries in expressions.
@philipc philipc marked this pull request as ready for review April 27, 2020 07:42
@philipc philipc requested a review from fitzgen April 27, 2020 07:42
/// Unresolved references in the `.debug_loc` section.
pub(crate) debug_loc_refs: Vec<DebugInfoReference>,
/// Unresolved references in the `.debug_loclists` section.
pub(crate) debug_loclists_refs: Vec<DebugInfoReference>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a bit ad-hoc. Maybe we should move to something a bit more generic (closer to a traditional assembler).

src/write/loc.rs Outdated Show resolved Hide resolved
src/write/op.rs Outdated Show resolved Hide resolved
});
}

/// Add a `DW_OP_GNU_parameter_ref` operation to the expression.
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to have some documentation about what those operations actually do as part of these doc comments.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that should be documented on the operations instead (and this could link to that). I started doing that, but it ends up just copying the standard. While that might be useful, it is a low priority for me, since users should be referring to the standard anyway.

Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

LGTM!

Expressions are now are Vec of operations, instead of raw bytecode.
This is required to support addresses and references within the
expression.
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.

gimli::write: support relocation in expression
5 participants