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

Introduce new enum for .dwo section IDs #716

Merged
merged 3 commits into from
May 25, 2024

Conversation

jameysharp
Copy link
Contributor

Keeping this type distinct from the SectionId enum ensures that the compiler will report an error if someone adds incomplete support for new separate-debug sections.

As discussed in: #714 (comment)

The existing SectionId::dwo_name method did not match the list of sections actually supported elsewhere in gimli. I've added the one section ID that was missing (.debug_macinfo.dwo), but I have not removed the section IDs which are only present there: .debug_str.dwo, .debug_cu_index, and .debug_tu_index.

Keeping this type distinct from the `SectionId` enum ensures that the
compiler will report an error if someone adds incomplete support for new
separate-debug sections.

As discussed in:
gimli-rs#714 (comment)

The existing `SectionId::dwo_name` method did not match the list of
sections actually supported elsewhere in gimli. I've added the one
section ID that was missing (`.debug_macinfo.dwo`), but I have not
removed the section IDs which are only present there: `.debug_str.dwo`,
`.debug_cu_index`, and `.debug_tu_index`.
Copy link
Collaborator

@philipc philipc left a comment

Choose a reason for hiding this comment

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

Sorry I made a mistake with the name, and your suggestion of IndexSectionId is better (since the DWO and DWP contain other sections that can't appear in the index).

@jameysharp
Copy link
Contributor Author

No problem; done. Almost everything I know about DWARF today is stuff I learned this week, so you could easily convince me either way. I'm always happy to take advice.

src/read/index.rs Outdated Show resolved Hide resolved
@philipc philipc merged commit 1b8a280 into gimli-rs:master May 25, 2024
20 checks passed
@jameysharp jameysharp deleted the dwo-section-ids branch May 25, 2024 04:10
@philipc
Copy link
Collaborator

philipc commented May 25, 2024

Do you have plans for more gimli work? If not I'll do a release soon (along with object and addr2line).

@jameysharp
Copy link
Contributor Author

I don't have anything more planned right now. I kind of wanted to improve the string formatting of the Error type, both to include the data associated with the various Unknown and Unsupported variants, and also ideally to attach a ReaderOffsetId to every error, not just UnexpectedEof. But I don't think I actually have the energy for either of those projects at the moment.

I intend to dig into the support for DWARF that's in Wasmtime and Cranelift, so it's possible I'll find more things I'd like from gimli along the way. But for now, I think a new release would be great. Thanks for your help getting these changes merged so promptly!

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