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

Fix assertion when capturing dump on ARM64 #112550

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

cshung
Copy link
Member

@cshung cshung commented Feb 14, 2025

This change fixes an assertion fired on OSX/ARM64 while capturing a heap dump.

The problem is that on OSX/ARM64, the page size is 16k (instead of the usual 4k), so SpecialDiagInfoSize is not a multiple of the page size, the end will not be page aligned.

And by the time this special region get to CombineMemoryRegions, it won't usually get combined with any existing region, but it itself is not page aligned, and so triggering the assertion.

We aren't really writing the actual page to the dump anyway, might as well just assert the exact size for this particular region instead.

This change is only tested for OSX/ARM64, and it should have no retail impact (just failing for check/debug builds)

@cshung cshung self-assigned this Feb 14, 2025
@Copilot Copilot bot review requested due to automatic review settings February 14, 2025 03:17

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (2)
  • src/coreclr/debug/createdump/memoryregion.h: Language not supported
  • src/coreclr/debug/createdump/specialdiaginfo.h: Language not supported
Copy link
Contributor

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

@cshung cshung merged commit fe8b347 into dotnet:main Feb 14, 2025
99 of 101 checks passed
@cshung cshung deleted the public/fix-arm64-assertion branch February 14, 2025 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants