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 ability to parse pdb info from PE file #298

Merged
merged 6 commits into from
May 7, 2021

Conversation

schultetwin1
Copy link
Contributor

Just like we can get the UUID from macho files and the build-id from ELF
files, this change allows users to pull out the PDB info (path, age, and
GUID) from PE files.

macho and ELF files will return Ok(None)

Just like we can get the UUID from macho files and the build-id from ELF
files, this change allows users to pull out the PDB info (path, age, and
GUID) from PE files.

macho and ELF files will return `Ok(None)`
src/read/mod.rs Outdated Show resolved Hide resolved
src/read/mod.rs Outdated
/// PDB Information
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct CodeView<'data> {
guid: Bytes<'data>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be [u8; 16]?

I also considered the existing pe::Guid but I don't think its definition is great. Maybe it should be [u8; 16] too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, I think [u8; 16] makes sense. I believe the definition of the GUID is well defined as 16 bytes. According to Microsoft's definition at least. I'll change to [u8; 16].

I also considered the existing pe::Guid

Oh woah, I did not see this on my first pass. I'm happy to use pe::Guid or convert pe::Guid to be a [u8; 16]. The technical definition from Microsoft's definition is what is currently defined for pe::Guid. I'm happy to use that.

src/read/pe/file.rs Outdated Show resolved Hide resolved
schultetwin1 and others added 3 commits May 5, 2021 20:16
Fix comment that does not make sense.

Co-authored-by: Philip Craig <[email protected]>
src/read/mod.rs Outdated
/// PDB Information
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct CodeView<'data> {
guid: &'data [u8; 16],
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's change it from a reference to a value too, the size of this struct isn't significant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can definitely do that, but I'm curious what benefit this brings? The path value will still need to be a reference with a lifetime of 'data unless we want to copy that as well meaning this struct will still need to have the 'data lifetime.

Copy link
Contributor

@philipc philipc May 6, 2021

Choose a reason for hiding this comment

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

For consistency with mach_uuid, and for the incredibly minor benefit that the user doesn't need to dereference it. I think UUIDs are like a primitive type that you normally pass around by value, not by reference. e.g. if we were reading a u128, then you'd probably expect that to be passed by value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, thanks for the explanation!

src/read/pe/file.rs Outdated Show resolved Hide resolved
Co-authored-by: Philip Craig <[email protected]>
@philipc philipc merged commit 2e0af3f into gimli-rs:master May 7, 2021
mcbegamerxx954 pushed a commit to mcbegamerxx954/object that referenced this pull request Jun 15, 2024
Just like we can get the UUID from macho files and the build-id from ELF
files, this change allows users to pull out the PDB info (path, age, and
GUID) from PE files.

macho and ELF files will return `Ok(None)`
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