Implement writing of .debug_info, .debug_abbrev and .debug_str #340
Conversation
philipc
changed the title
|
This now has good test coverage and minimal docs. Some notes about the implementation:
|
| @@ -241,5 +243,8 @@ pub mod read; | |||
| // For backwards compat. | |||
| pub use read::*; | |||
|
|
|||
| #[cfg(feature = "std")] | |||
| pub mod write; | |||
There was a problem hiding this comment.
I think it is worth having two traits: read and write that enable their corresponding modules. This way, compilers that are emitting DWARF don't need to pay compile time for DWARF reading, and a stack unwinder doesn't need to pay compile time for DWARF writing.
There was a problem hiding this comment.
I'll do this in a follow up PR.
| pub fn write<W: Writer>(&self, w: &mut DebugAbbrev<W>) -> Result<()> { | ||
| w.write_uleb128(self.tag.0)?; | ||
| w.write_u8(if self.has_children { | ||
| constants::DW_CHILDREN_yes.0 |
There was a problem hiding this comment.
It seems our dw! macro for constants could also generate a thin little write method. Although, I suppose most of the time we want to write LEB128s, so maybe it wouldn't be super useful and this location would still be the exception.
There was a problem hiding this comment.
I'm not sure about the usefulness:
- they would be simple one line functions
- many of them could be written in multiple forms that the caller would need to decide, since attribute values could potentially use either LEB128 or a fixed size, and this needs to match the form, so it's better to have this code closer to the form decision.
src/write/str.rs
Outdated
| // - able to get an existing value given an id | ||
| // | ||
| // Limitations of current implementation (using IndexSet): | ||
| // - inserting a duplicate requires an allocation or a double lookup |
There was a problem hiding this comment.
This is one of my longest standing annoyances with hash tables in Rust in general (mostly std, but apparently indexmap as well). I suspect a duplicated lookup is probably cheaper than an allocation (I haven't read further to see what you ended up implementing).
There was a problem hiding this comment.
It's actually allocation for duplicate, and double lookup for non-duplicate, so will depend on how often there are duplicates. I did the simplest (allocation), and we can change based on benchmarks.
| parent: Option<UnitEntryId>, | ||
| tag: constants::DwTag, | ||
| /// Whether to emit `DW_AT_sibling`. | ||
| // TODO: should this be automatic? |
There was a problem hiding this comment.
What heuristics would we use to decide whether to emit this? Whether the children forest is of a certain size? It seems like maybe certain DIEs always end up having their children walked, while others are skipped past, so maybe there would be a whitelist of tags that should get the sibling links.
Anyways, agree that for now this should be manual.
There was a problem hiding this comment.
I was thinking maybe we could do it based on a tag whitelist. The whitelist members would be chosen based on typical access patterns of debuggers/unwinders.
|
Thanks for the review @fitzgen! I've added a commit with doc fixes. |
|
Hooray |
Also implements reading debug sections and converting into the write data structures.