Skip to content

Increase default stack guard on x86 Windows#118277

Merged
tommcdon merged 1 commit into
dotnet:mainfrom
gregg-miskelly:dev/greggm/X86DefaultExtraPageCount
Aug 6, 2025
Merged

Increase default stack guard on x86 Windows#118277
tommcdon merged 1 commit into
dotnet:mainfrom
gregg-miskelly:dev/greggm/X86DefaultExtraPageCount

Conversation

@gregg-miskelly

Copy link
Copy Markdown
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 AI review requested due to automatic review settings August 1, 2025 15:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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
@dotnet-policy-service

Copy link
Copy Markdown
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

@tommcdon tommcdon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

@janvorli janvorli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thank you!

@janvorli

janvorli commented Aug 5, 2025

Copy link
Copy Markdown
Member

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

janvorli commented Aug 5, 2025

Copy link
Copy Markdown
Member

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

@gregg-miskelly

Copy link
Copy Markdown
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.

5 participants