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: add support for DWP index sections #588

Merged
merged 1 commit into from
Sep 16, 2021
Merged

Conversation

philipc
Copy link
Collaborator

@philipc philipc commented Sep 9, 2021

r? @khuey (since this builds on your DWO work)

@khuey
Copy link
Contributor

khuey commented Sep 9, 2021

Are "DWP index sections" documented somewhere?

@philipc
Copy link
Collaborator Author

philipc commented Sep 9, 2021

DWARF 5 documents them in 7.3.5 (DWARF Package Files). The GNU extension to DWARF 4 is documented at https://gcc.gnu.org/wiki/DebugFissionDWP.

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.

I believe the handling of the DwoId/DebugTypeSignature to hash conversions has incorrect endian handling.

The docs (7.3.5.3 specifically) say "Define REP(X) to be the value of X interpreted as an unsigned 64-bit integer in the target byte order." As far as I can tell there's no allowance for cases where the target endianness and the program endianness are different like there is elsewhere in gimli.

@philipc
Copy link
Collaborator Author

philipc commented Sep 14, 2021

I don't understand what you mean. DwoId/DebugTypeSignature are already 64-bit integers that were obtained by parsing bytes in the target byte order (that's what Reader::read_u64 does).

@khuey
Copy link
Contributor

khuey commented Sep 14, 2021

They're parsed in the target byte order, yes, but if I'm reading the spec correctly the hash must also be computed with those 64 bit integers in the target byte order, which doesn't happen here.

@khuey
Copy link
Contributor

khuey commented Sep 14, 2021

(And that inherently makes some amount of sense. The on disk hashes were computed on the target machine and our computation needs to match that exactly in order to produce the same hash.)

@philipc
Copy link
Collaborator Author

philipc commented Sep 14, 2021

Right, the on disk hash and the on disk DwoId/TypeSignature are both computed using the target machine byte order. But we read both of those using the target byte order. The hash operations don't care about endianness. Once you have a 64-bit integer, a right shift 32 means the same regardless of the original byte order. That's the point of converting it to a 64-bit integer instead of using a byte array.

@khuey
Copy link
Contributor

khuey commented Sep 14, 2021

Ah, hrm, maybe I'm misunderstanding what "interpreted in the target byte order" means then.

@philipc
Copy link
Collaborator Author

philipc commented Sep 14, 2021

"Given an 8-byte compilation unit ID or type signature X, an entry in the hash table is located as follows:

  1. Define REP (X) to be the value of X interpreted as an unsigned 64-bit integer in the target byte order."

My interpretation is that if you start with an 8-byte compilation unit ID or type signature X (which is a [u8; 8]), then you must convert it to a u64 using the target byte order, which is what Reader::read_u64 does.

@philipc philipc merged commit e318388 into gimli-rs:master Sep 16, 2021
@philipc philipc deleted the index branch September 16, 2021 01:27
@khuey
Copy link
Contributor

khuey commented Sep 16, 2021

fwiw nothing else raised my eye beyond that potential endianness issue.

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