Skip to content
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

runtime: don't crash on nil g in sigtramp on windows arm64 platform #50889

Closed
wants to merge 1 commit into from

Conversation

marler8997
Copy link

@marler8997 marler8997 commented Jan 28, 2022

This fixes the sigtramp handler on Windows. Note that unlike linuxy
platforms, sigtramp is not actually a "trampoline" function. Instead,
The code needed to handle interrupts in userspace is provided by
Windows by some other means. On Windows the mechanism they provide is
called "Vectored Exception Handling". Windows provides the
AddVectoredExceptionHandler to register a callback function whenever an
interrupt occurs which could be triggered by a signal/exception/etc.
Windows supports multiple handlers and will call them one after the other
stopping or continuing based on the return value as it goes along.

In the case of go, it appears that sigtramp doesn't need to handle
exceptions on threads that it hasn't initialized with the runtime. So
in the case of windows, returning 0 from the exception handler will
cause it to move on to the next exception handler. This is the change
I've made here.

This change has been built and verified to fix #43800 on a hololens
arm64 windows device.

Note that this also appears to be a problem for arm (sys_windows_arm.s).
I'm currently not set up to test this on arm so maybe this could be
fixed in a follow up change.

Fixes #43800

@gopherbot
Copy link
Contributor

This PR (HEAD: 824a4f4) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/381775 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Ian Lance Taylor:

Patch Set 1: Run-TryBot+1

(4 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/381775.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/381775.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 1: TryBot-Result+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/381775.
After addressing review feedback, remember to publish your drafts!

This fixes the sigtramp handler on Windows.  Note that unlike linuxy
platforms, sigtramp is not actually a "trampoline" function.  Instead,
The code needed to handle interrupts in userspace is provided by
Windows by some other means.  On Windows the mechanism they provide is
called "Vectored Exception Handling".  Windows provides the
AddVectoredExceptionHandler to register a callback function whenever an
interrupt occurs which could be triggered by a signal/exception/etc.
Windows supports multiple handlers and will call them one after the other
stopping or continuing based on the return value as it goes along.

In the case of go, it appears that sigtramp doesn't need to handle
exceptions on threads that it hasn't initialized with the runtime.  So
in the case of windows, returning 0 from the exception handler will
cause it to move on to the next exception handler.  This is the change
I've made here.

This change has been built and verified to fix golang#43800 on a hololens
arm64 windows device.

Note that this also appears to be a problem for arm (sys_windows_arm.s).
I'm currently not set up to test this on arm so maybe this could be
fixed in a follow up change.

Fixes golang#43800
@gopherbot
Copy link
Contributor

This PR (HEAD: 5602298) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/381775 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@marler8997 marler8997 changed the title runtime: fix sigtramp on windows arm64 platform runtime: don't crash on nil g in sigtramp on windows arm64 platform Jan 28, 2022
@gopherbot
Copy link
Contributor

Message from Jonathan Marler:

Patch Set 4:

(3 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/381775.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Cherry Mui:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/381775.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jonathan Marler:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/381775.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jonathan Marler:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/381775.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Cherry Mui:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/381775.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jonathan Marler:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/381775.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jonathan Marler:

Patch Set 4: Code-Review-1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/381775.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jonathan Marler:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/381775.
After addressing review feedback, remember to publish your drafts!

@marler8997 marler8997 closed this Oct 20, 2022
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.

cmd/link: -buildmode=c-shared not supported on windows/arm
2 participants