feat: Access embedded PortablePDB in DLL

[vaind]

replaces & closes #659

PE (a.k.a a .dll) may contain embedded Portable PDB (when built with <DebugType>embedded</DebugType>) as shown in the following screenshot from dotPeek

We need to access that but it seems it's currently impossible with the used PE reader, see the tracking issue: m4b/goblin#314

[codecov]

Codecov Report

Merging #752 (5b37e98) into master (498d9c5) will increase coverage by 0.05%.
The diff coverage is 79.10%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #752      +/-   ##
==========================================
+ Coverage   73.82%   73.88%   +0.05%     
==========================================
  Files          69       69              
  Lines       14884    14951      +67     
==========================================
+ Hits        10988    11046      +58     
- Misses       3896     3905       +9     

[Swatinem]

This is awesome work! Reminds me that I have m4b/goblin#319 just laying there since half a year :-D

Missed some necessary changes

[Swatinem]

As for the Symbolicator integration, I believe there are two ways to approach this.

1. As you mentioned, we will not use the PE itself anymore, so we can decide to "unpack" the Portable PDB when fetching the object here: https://github.com/getsentry/symbolicator/blob/9014ed70d127fb33d051b37efa781b01e7e598e2/crates/symbolicator-service/src/services/objects/data_cache.rs#L135-L172
   That way, we can use it "cheaply" for source lookups, and directly create a ppdb cache from it.
   The advantage here is that the source lookups and ppdb cache creation are unchanged, and fast (no decompression needed anymore). The disadvantage is that we throw away the "outer" PE.
2. You could do the get_embedded_ppdb dance both in source lookup, and in ppdb conversion.
   This is a bit more work, and would still have to do decompression on every source lookup. But it has the advantage of keeping the original PE intact.

Given that we (at this time) are not using the outer PE for anything, I would recommend to just go with 1 and unpack directly when downloading.

[vaind]

As for the Symbolicator integration, I believe there are two ways to approach this.

1. As you mentioned, we will not use the PE itself anymore, so we can decide to "unpack" the Portable PDB when fetching the object here: getsentry/symbolicator@9014ed7/crates/symbolicator-service/src/services/objects/data_cache.rs#L135-L172
   That way, we can use it "cheaply" for source lookups, and directly create a ppdb cache from it.
   The advantage here is that the source lookups and ppdb cache creation are unchanged, and fast (no decompression needed anymore). The disadvantage is that we throw away the "outer" PE.
2. You could do the get_embedded_ppdb dance both in source lookup, and in ppdb conversion.
   This is a bit more work, and would still have to do decompression on every source lookup. But it has the advantage of keeping the original PE intact.

Given that we (at this time) are not using the outer PE for anything, I would recommend to just go with 1 and unpack directly when downloading.

I was thinking of a combination of both. The first point for actual symbolication, and the second one (or just a part of it) to display the correct information in the UI (that it has sources). On the other hand, doing an unpack just to check whether there are sources doesn't sound like a great idea and maybe we should skip that for now...

[Swatinem]

almost there :-)

[Swatinem]

Suggested change

	let mut signature: [u8; 4] = [0; 4];
	self.data
	.gread_inout(&mut offset, &mut signature)
	.map_err(PeError::new)?;
	if signature != "MPDB".as_bytes() {
	let signature: [u8; 4] = self.data
	.gread(&mut offset)
	.map_err(PeError::new)?;
	if signature != b"MPDB" {

First time I saw gread_inout.

[vaind]

not the same thing - gread doesn't work with slices

[vaind]

error[E0277]: the trait bound `[u8; 4]: TryFromCtx<'_, _>` is not satisfied
   --> symbolic-debuginfo\src\pe.rs:313:42
    |
313 |                 let signature: [u8; 4] = self.data.gread(&mut offset).map_err(PeError::new)?;
    |                                          ^^^^^^^^^ ----- required by a bound introduced by this call
    |                                          |
    |                                          the trait `TryFromCtx<'_, _>` is not implemented for `[u8; 4]`
    |
    = help: the trait `TryFromCtx<'a, usize>` is implemented for `&'a [u8]`
note: required by a bound in `gread`
   --> C:\Users\dlugo\.cargo\registry\src\github.com-1ecc6299db9ec823\scroll-0.11.0\src\pread.rs:88:21
    |
88  |     fn gread<'a, N: TryFromCtx<'a, Ctx, Self, Error = E>>(
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `gread`
    ```