Basic support for DWARF 5 .debug_info #257
Conversation
src/abbrev.rs
Outdated
| @@ -157,6 +157,7 @@ pub struct Abbreviation { | |||
| tag: constants::DwTag, | |||
| has_children: constants::DwChildren, | |||
| attributes: Vec<AttributeSpecification>, | |||
| implicit_consts: Vec<i64>, | |||
There was a problem hiding this comment.
I feel that we should add the implicit constant to the AttributeSpecification instead. It'll simplify things a lot, and I don't think the increased memory usage will be significant (it may even be less memory for abbreviations with only a couple of attributes).
There was a problem hiding this comment.
I had assumed that an AttributeSpecification was 4 bytes but it looks like it's actually 16 (why are DwAt and DwForm u64s and not u16s?) so we'd break even after 3 attributes. I could go either way here.
There was a problem hiding this comment.
DwAt and DwForm are u64s because that's what read_uleb128 returns, and the standard doesn't explicitly define otherwise. Probably not a good reason, and I had noticed it in the past but never got around to doing anything about it. So this is something we may change in future.
This is a 2-3% performance hit to bench_parsing_debug_info (although I'm comparing to before this change, not the alternative implementation). So due to that, and due to the code complexity, I prefer changing this, but not strongly enough to require it. FWIW, gdb and llvm both store it in the attribute specification.
There was a problem hiding this comment.
On my system this actually seems to speed up bench_parsing_debug_info, not that that makes a ton of sense.
I don't feel very strongly about this so I'll change it.
src/unit.rs
Outdated
| address_size = rest.read_u8()?; | ||
| offset = parse_debug_abbrev_offset(&mut rest, format)?; | ||
| } else { | ||
| unreachable!() |
There was a problem hiding this comment.
Delete parse_version and return UnknownVersion here instead.
There was a problem hiding this comment.
I assume you mean unreachable!?
There was a problem hiding this comment.
No, I meant to delete parse_version and replace it with read_u16. I don't think parse_version provides any value: we do a check of the version in it, then we need to immediately repeat the check when it returns, so I think the code is clearer without parse_version.
src/unit.rs
Outdated
| address_size = rest.read_u8()?; | ||
| } else if version == 5 { | ||
| let unit_type = parse_compilation_unit_type(&mut rest)?; | ||
| if unit_type != constants::DW_UT_compile && unit_type != constants::DW_UT_type { |
There was a problem hiding this comment.
Change this to only accept DW_UT_compile. Currently it will not work for DW_UT_type, because we only parse the extra type unit fields when it is in the .debug_types section, but DWARF 5 type units are in the .debug_info section.
|
Comments addressed. |
| mut specs: &'abbrev [AttributeSpecification], | ||
| ) -> Result<(Attribute<R>, &'abbrev [AttributeSpecification])> { | ||
| let spec = specs[0]; | ||
| specs = &specs[1..]; |
There was a problem hiding this comment.
This change isn't needed anymore, and it's part of what I thought would affect performance, but reverting it didn't help, so going to leave any cleanup here for future performance work.
|
Thanks @khuey! |
The compilation unit header layout has changed, and the new
DW_FORM_implicit_constrequires handling while traversing the abbreviations.