Skip to content

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

Merged
joncham merged 1 commit intounity-masterfrom
unity-master-stack-overflow-windows-sp-address
May 30, 2019
Merged

Update RSP with fault address for stack overflow (case 1148592)#1171
joncham merged 1 commit intounity-masterfrom
unity-master-stack-overflow-windows-sp-address

Conversation

@joncham
Copy link
Copy Markdown
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
switch (er->ExceptionCode) {
case EXCEPTION_STACK_OVERFLOW:
if (!mono_aot_only && restore_stack) {
if (er->NumberParameters == 2) {
Copy link
Copy Markdown

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

A few questions?

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

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