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

read: improve handling of 0 for tombstones #750

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

philipc
Copy link
Collaborator

@philipc philipc commented Sep 3, 2024

Where possible, ignore these instead of returning them or returning an error. This often isn't possible because 0 is a valid address, but we can handle it for DW_LNE_set_address in the middle of a line sequence, and for address pairs.

This fixes one of the problems reported in gimli-rs/addr2line#326.

@philipc philipc changed the title read/line: handle 0 tombstone address during sequence read: improve handling of 0 for tombstones Sep 3, 2024
@philipc philipc force-pushed the tombstone branch 2 times, most recently from 67dbf2f to f514536 Compare September 5, 2024 07:52
Where possible, ignore these instead of returning them or returning
an error. This often isn't possible because 0 is a valid address,
but we can handle it for `DW_LNE_set_address` in the middle of a
line sequence, and for address pairs.
@philipc philipc merged commit 7ba06e8 into gimli-rs:master Sep 6, 2024
20 checks passed
@philipc philipc deleted the tombstone branch September 6, 2024 02:28
@marxin
Copy link
Contributor

marxin commented Sep 9, 2024

I've just noticed this PR when I was working on davidlattimore/wild#37.
@philipc Can you please provide a link to the DWARF standard where are the tombstone values defined?

@philipc
Copy link
Collaborator Author

philipc commented Sep 10, 2024

It's actually only DWARF 6 that will specify this. The issue is https://dwarfstd.org/issues/200609.1.html, and you can also check the draft DWARF 6 standard. lld tried changing to -1 but it broke too many DWARF consumers, so it changed to 0, with a plan to try -1 again sometime in the future. https://reviews.llvm.org/D84825 contains a lot of relevant discussion.

@marxin
Copy link
Contributor

marxin commented Sep 10, 2024

Thank you for the useful pointers!

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.

2 participants