Skip to content

Update RSP with fault address for stack overflow (case 1148592) #1171

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

Merged

Conversation

joncham
Copy link
Member

@joncham joncham commented Apr 29, 2019

Use actual stack fault address rather than relying on the
SP of the current frame. If we try to enter a method with a
large prolog (many locals) we may try to allocate more stack
than is available, however the SP has not been updated. This
means the stack overflow heuristic to free up enough space
may fail, as the used stack may be smaller than the amount
of stack that was attempted to be unwound.

@joncham joncham requested a review from joshpeterson April 29, 2019 20:29
@joncham joncham self-assigned this Apr 29, 2019
@@ -151,6 +151,17 @@ LONG CALLBACK seh_vectored_exception_handler(EXCEPTION_POINTERS* ep)
switch (er->ExceptionCode) {
case EXCEPTION_STACK_OVERFLOW:
if (!mono_aot_only && restore_stack) {
if (er->NumberParameters == 2) {

Choose a reason for hiding this comment

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

What is the significance of the value 2 here? Can you add a comment with some details about that?

@joshpeterson
Copy link

A few questions?

  • Does this need release notes?
  • Should it be back-ported?
  • Should it be pushed upstream?

@joncham
Copy link
Member Author

joncham commented May 23, 2019

Release Notes:
case 1148592 - Fix crash when large array initializer is used.

No backporting

I'll open PR for upstream.

Use actual stack fault address rather than relying on the
SP of the current frame. If we try to enter a method with a
large prolog (many locals) we may try to allocate more stack
than is available, however the SP has not been updated. This
means the stack overflow heuristic to free up enough space
may fail, as the used stack may be smaller than the amount
of stack that was attempted to be unwound.
@joncham joncham force-pushed the unity-master-stack-overflow-windows-sp-address branch from e426d26 to 6ae9b7c Compare May 23, 2019 18:45
@joncham joncham merged commit f5bd600 into unity-master May 30, 2019
@joncham joncham deleted the unity-master-stack-overflow-windows-sp-address branch May 30, 2019 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants