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

DWO cleanups #569

Merged
merged 4 commits into from
Jul 16, 2021
Merged

DWO cleanups #569

merged 4 commits into from
Jul 16, 2021

Conversation

philipc
Copy link
Collaborator

@philipc philipc commented Jul 16, 2021

Follow up to #568
r? @khuey

Copy link
Contributor

@khuey khuey left a comment

Choose a reason for hiding this comment

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

generally lgtm

};
let dwo_parent = gimli::Dwarf::load(&mut load_dwo_parent_section)?;
dwarf.debug_addr = dwo_parent.debug_addr.clone();
dwarf.ranges = dwo_parent.ranges.clone();
Copy link
Contributor

Choose a reason for hiding this comment

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

debug_line/debug_line_str really should be copied from the parent, and ranges should be copied if and only if the dwo doesn't have debug_rnglists (in other words if this is the GNU split dwarf extension and not the standard DWARF 5 version)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok. This isn't something I changed in this PR is it? This sounds like something to be addressed as part of adding a method to Dwarf to do this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, nothing changed here afaict.

@@ -671,6 +684,11 @@ impl<R: Reader> Unit<R> {
unit.rnglists_base = base;
}
}
constants::DW_AT_GNU_dwo_id => {
if let AttributeValue::DwoId(dwo_id) = attr.value() {
unit.dwo_id = Some(dwo_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we check to see if this is already present? DW_AT_GNU_dwo_id shouldn't really be present in DWARF 5 programs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok. It shouldn't matter, but it doesn't take much to add the check.

/// If this is a from a DWARF 4 DWO file, then it must additionally be offset by the
/// value of `DW_AT_GNU_ranges_base`. You can use `Unit::ranges_offset_from_raw` to do this.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct RawRangeListsOffset<T = usize>(pub T);
Copy link
Contributor

Choose a reason for hiding this comment

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

My only concern about this is that it might be confused as being related to raw_ranges, but I don't have a better suggestion for the name.

@philipc philipc merged commit 2c5afbf into gimli-rs:master Jul 16, 2021
@philipc philipc deleted the dwo branch July 16, 2021 05:28
@khuey
Copy link
Contributor

khuey commented Jul 19, 2021

Can we (you :) ) do a 0.25 with this?

@philipc
Copy link
Collaborator Author

philipc commented Jul 20, 2021

I will in a few days after an object release.

@philipc
Copy link
Collaborator Author

philipc commented Jul 26, 2021

0.25 is released.

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