[release/8.0] Fix stack_limit handling #91095
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There is a problem with computing stack_limit in the presence of inactive InlinedCallFrame. We were trying to call GetCallSiteSP on that frame to get the call site SP. But when the inlined call frame is not active (there is no call to native code in progress), that method returns random junk. But even if we checked for an active call before reading the value, it would not get correct stack limit on x86 windows when there was no active call with the inlined call frame being the topmost one. That can happen with hardware exception handling on Windows x86. On other targets, we always have other explicit frame on top of the explicit frames stack, but on windows x86, we don't use the FaultingExceptionFrame for hardware exceptions, so the inactive inlined call frame could be the topmost one when GC starts to scan stack roots. Since the stack_limit was introduced to fix a Unix specific problem, I have fixed that by disabling the stack_limit usage for x86 windows. Close #86265
jkotas
approved these changes
Aug 24, 2023
jeffschwMSFT
approved these changes
Aug 25, 2023
jeffschwMSFT
added
Servicing-approved
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #90937 to release/8.0
/cc @janvorli
Customer Impact
When hardware exception happens on Windows x86, there is a function on the stack that did or is potentially going to do a pinvoke and the related
InlinedCallFrameis the topmost explicit frame, the stack limit we use in GC to detect whether an address is on stack or not is a garbage. Depending on the value of the garbage, it might go unnoticed or it might cause a GC hole, because it would falsely dismiss some stack locations as not being on the current stack. That check is used to determine whether a reference is an interior pointer or not. So it may cause GC holes or other problems with GC.Testing
A targeted repro test, local coreclr / libraries testing, CI tests.
Risk
Low, the fix effectively disables the use of the stack limit extracted from the topmost frame on Windows x86. The stack limit was originally introduced to address a Unix specific issue with dynamic stack size. That is not something that can happen on Windows.