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

Add GnuDebuglink #262

Closed
wants to merge 1 commit into from
Closed

Add GnuDebuglink #262

wants to merge 1 commit into from

Conversation

sfackler
Copy link
Contributor

Closes #260

@coveralls
Copy link

Coverage Status

Coverage remained the same at 0.0% when pulling e940e07 on sfackler:gnu-debuglink into 1c20ba6 on gimli-rs:master.

/// Returns the CRC32 checksum of the debug information file.
pub fn crc32(&self) -> Result<u32> {
let mut section = self.gnu_debuglink_section.clone();
let base = section.len() - ReaderOffset::from_u32(4);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This works fine for valid data, but doesn't handle invalid data very well. It'd be better to have a single method that reads both the filename and crc32. I guess it doesn't matter too much though because the crc won't match if it's wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I wasn't sure what the right API is here - is there another section type that's similar? It seems like the others currently supported are all sequences.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't see any other sections that are similar. I think just a fn read(&self) -> Result<(R, u32)> would be fine.

@philipc
Copy link
Collaborator

philipc commented Nov 13, 2017

Is there a compatible implementation of the crc32 that we can use in conjunction with this?

@sfackler
Copy link
Contributor Author

I think https://docs.rs/crc/1.5.0/crc/crc32/fn.checksum_ieee.html is the right crc variant?

@tromey
Copy link
Member

tromey commented Nov 13, 2017

I wonder if this should be in moria, not gimli. It doesn't really seem DWARF-related to me.

OTOH I suppose it wouldn't be too bad to just move it to moria when that has some content.

@sfackler
Copy link
Contributor Author

Yeah it might make sense for this to live in Moria. We'll also need to pull .note.gnu.build-id, which is even less related to DWARF stuff.

@tromey
Copy link
Member

tromey commented Nov 13, 2017

We'll also need to pull .note.gnu.build-id, which is even less related to DWARF stuff.

I checked and goblin can already extract this: https://github.com/m4b/goblin/blob/master/src/elf/note.rs

@luser
Copy link
Contributor

luser commented Dec 15, 2017

I'm going to submit a PR to make moria an actual crate soon: gimli-rs/locate-dwarf#1 (comment) so you could definitely put your work into it then!

@luser
Copy link
Contributor

luser commented Dec 17, 2017

My moria PR is up: gimli-rs/locate-dwarf#2

I wound up adding APIs to object to make this easier: gimli-rs/object#39

You'll probably want to implement Object::debug_file_info for ELF: gimli-rs/object@ae5b66a#diff-11f7414d9fed445f4231a47e0491cadaR157 and add some other variants to the DebugFileInfo enum to support Build ID etc: gimli-rs/object@ae5b66a#diff-b4aea3e418ccdb71239b96952d9cddb6R62

@philipc
Copy link
Collaborator

philipc commented May 19, 2018

Implemented in the object crate instead (and used by moria).

@philipc philipc closed this May 19, 2018
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.

5 participants