Skip to content

[mono] Allocate helper page with mono_valloc so it doesn't confuse instruments#120060

Merged
BrzVlad merged 2 commits intodotnet:mainfrom
BrzVlad:fix-interp-instruments2
Sep 25, 2025
Merged

[mono] Allocate helper page with mono_valloc so it doesn't confuse instruments#120060
BrzVlad merged 2 commits intodotnet:mainfrom
BrzVlad:fix-interp-instruments2

Conversation

@BrzVlad
Copy link
Member

@BrzVlad BrzVlad commented Sep 24, 2025

mono_valloc uses mmap behind the scene.

@BrzVlad BrzVlad requested a review from steveisok as a code owner September 24, 2025 15:35
Copilot AI review requested due to automatic review settings September 24, 2025 15:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR changes how the helper page is allocated in the mono runtime's process-wide memory barrier implementation to avoid confusing profiling instruments. The key change replaces posix_memalign with mono_valloc (which uses mmap) and restructures when memory protection is set.

  • Replaces posix_memalign allocation with mono_valloc to use mmap-based allocation
  • Moves memory protection setup to occur before page access rather than after
  • Removes the protection restoration logic that was specifically added to avoid confusing profiling tools
Comments suppressed due to low confidence (1)

src/mono/mono/utils/mono-threads-posix.c:1

  • The code writes to the helper page at line 304 but then immediately removes all access permissions at line 306. This creates a memory leak scenario where the page remains allocated but inaccessible for future barrier operations, potentially causing crashes on subsequent calls to this function.
/**

@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 24, 2025
@BrzVlad
Copy link
Member Author

BrzVlad commented Sep 24, 2025

Reverts #119754, following discussion with @jkotas on #119885

@BrzVlad BrzVlad added area-VM-meta-mono and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Sep 24, 2025
@BrzVlad BrzVlad requested a review from lateralusX September 24, 2025 15:36
@BrzVlad
Copy link
Member Author

BrzVlad commented Sep 25, 2025

/ba-g This change doesn't touch wasm bits.

@BrzVlad BrzVlad merged commit 0cbfea4 into dotnet:main Sep 25, 2025
70 of 73 checks passed
@BrzVlad
Copy link
Member Author

BrzVlad commented Oct 1, 2025

/backport to release/10.0

@github-actions
Copy link
Contributor

github-actions bot commented Oct 1, 2025

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants