Skip to content

feat: PE DWARF companion support#3240

Draft
supervacuus wants to merge 4 commits intogetsentry:masterfrom
supervacuus:feat/pe-dwarf-companion-upload-support
Draft

feat: PE DWARF companion support#3240
supervacuus wants to merge 4 commits intogetsentry:masterfrom
supervacuus:feat/pe-dwarf-companion-upload-support

Conversation

@supervacuus
Copy link
Copy Markdown
Contributor

@supervacuus supervacuus commented Mar 24, 2026

Description

Extracted from #3237

Some tests currently fail, because symbolic updated its zip dependency (2.4.2 to 7.2.0) since the last bump. This changed the encoding and invalidated a bunch of test assertions. A fix proposal for the failing tests is #3239.

This change was triggered by bumping symbolic to 12.17.3 to include getsentry/symbolic#960.

The new permissive PE parser fixes in getsentry/symbolic#960 exposed performance edge cases for some of the debug companions during PE import-table parsing, the result of which neither symbolic nor sentry-cli really needs.

The PR addresses the issue in the following way:

I introduced a new goblin option that excludes the PE import table from parsing. Of course, this requires further upstream PRs (cargo points goblin and symbolic to dev branches) before we can merge this, but I wanted to check whether you are generally in agreement with these changes before going upstream.

Upstream changes required:

Ideally, this PR gets merged after #3238 and #3239, because both solve issues this PR is affected by. If we drop or defer #3239, we need to update all snapshots and magic digests here first.

Issues

At least partially fixes getsentry/sentry#104738

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@supervacuus supervacuus requested review from a team and szokeasaurusrex as code owners March 24, 2026 19:05
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment thread Cargo.toml
@supervacuus
Copy link
Copy Markdown
Contributor Author

supervacuus commented Apr 15, 2026

Just to be clear, @szokeasaurusrex, this PR still depends on upstream changes, and that is twofold:

  1. fix(goblin): disable PE import table parser symbolic#964 depends on feat(pe): add parse_imports option to ParseOptions m4b/goblin#522, besides a ❤️, I haven't gotten a response there yet. So that PR chain is currently on hold.
  2. fix(goblin): disable PE import table parser symbolic#964 was built on top of fix: use goblin permissive PE parse-mode symbolic#960, which was already released, and integrating that release led to my PRs here. However, @loewenheim recently had to revert because it is a breaking change and was included in a patch-level release. So a new release of symbolic is out that no longer has this change, and the latest version (which was only a minor bump) didn't have it either.

The latter is not really a blocker, because we can reintroduce the accumulated change via getsentry/symbolic#964, but I wanted to let you know nonetheless, as this means it will likely only be introduced into sentry-cli when symbolic has its next major release.

I will ping upstream and switch this back to draft until further notice. No need to review (whatever that would mean for a version bump), unless you have spare capacity.

@supervacuus supervacuus marked this pull request as draft April 15, 2026 13:51
Copy link
Copy Markdown
Member

Got it. Then, I'll hold off on the review until this PR is unblocked

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.

Support for Windows applications compiled with MSYS2+Mingw-w64 GCC

2 participants