Skip to content

Conversation

gregg-miskelly
Copy link
Contributor

In Microsoft Cloud Test environments, the target process would crash without stopping when running under a managed debugger and the target process triggered a stack overflow exception. This increases the default count of thread guard pages to avoid this.

@Copilot Copilot AI review requested due to automatic review settings August 1, 2025 15:47
Copy link
Contributor

@Copilot 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 increases the default stack guard pages on x86 Windows to prevent crashes in Microsoft Cloud Test environments when a managed debugger encounters stack overflow exceptions. The change restructures the conditional compilation logic to provide more consistent guard page allocation across platforms.

  • Consolidates debug-specific guard page logic under a single INDEBUG macro
  • Increases default guard pages for 32-bit platforms from 0 to 1 page
  • Maintains existing 64-bit platform defaults (3 pages plus debug overhead)

@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 Aug 1, 2025
@gregg-miskelly gregg-miskelly marked this pull request as draft August 1, 2025 15:48
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Aug 1, 2025
@gregg-miskelly gregg-miskelly force-pushed the dev/greggm/X86DefaultExtraPageCount branch from e293592 to 5d76a67 Compare August 1, 2025 18:16
@gregg-miskelly gregg-miskelly marked this pull request as ready for review August 1, 2025 18:16
@teo-tsirpanis teo-tsirpanis added area-VM-coreclr and removed community-contribution Indicates that the PR has been added by a community member needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 3, 2025
Copy link
Contributor

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

@gregg-miskelly gregg-miskelly force-pushed the dev/greggm/X86DefaultExtraPageCount branch from 5d76a67 to d9e7e06 Compare August 5, 2025 16:12
@tommcdon tommcdon requested review from janvorli and mangod9 August 5, 2025 16:15
Copy link
Member

@tommcdon tommcdon left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@janvorli
Copy link
Member

janvorli commented Aug 5, 2025

I have noticed that the comments at the changed code about various sizes are completely outdated, we should either remove it completely or update it to match the reality.
Edit: I don't mean doing it as part of this PR.

@janvorli
Copy link
Member

janvorli commented Aug 5, 2025

I've created #118401 to track the comment update.

@gregg-miskelly
Copy link
Contributor Author

@janvorli can you merge this for me? I don't have rights

@tommcdon tommcdon merged commit dc341ae into dotnet:main Aug 6, 2025
92 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 5, 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