Partial workaround for use of win32_altstack_handler in inappropriate contexts #108
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
|
Need to add more documentation to the test. |
embray
force-pushed
the
cygwin/altstack-handler-noreserve-bug
branch
from
4c9652f
to
2fbb09a
Compare
|
The failures on the Travis build don't appear to be relevant. It's failing like |
This is supposed to be fixed by cfbd43b Can you rebase to latest master? |
|
Oh, I remember seeing that commit but didn't understand what it was for. Yes, I'll try that. |
https://trac.sagemath.org/ticket/27214#comment:11 Includes regression test. As noted in the comment, this can still invoke undesired behavior in the case where an unintialized region of an mmap created with MAP_NORESERVE is accessed from within a signal handler. At the time there is no good way to handle that case without extra support from Cygwin's API which is currently not available as far as I can tell. A workaround, for the rare code where this might be necessary, is to call mprotect (with the appropriate flags) on the region of memory that needs to be accessed before attempting to access it.
embray
force-pushed
the
cygwin/altstack-handler-noreserve-bug
branch
from
2fbb09a
to
36e767f
Compare
dimpase
pushed a commit
to sagemath/sagetrac-mirror
that referenced
this pull request
Nov 9, 2020
handler This is the fix from sagemath/cysignals#108 adapated to apply to cysignals v1.8.1. This fix is needed to resolve Trac #27214.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
See https://trac.sagemath.org/ticket/27214#comment:11
One thing I might also add is in the case of
STATUS_ACCESS_VIOLATION, one thing I can do at the very least is query the Windows VMM for the state of the accessed memory region. If it returnsMAP_RESERVEthere's a good chance we are hitting the still unhandled corner case I described in the comments, and can print an additional message with the hint about usingmprotect(...). I'm not sure it really matters though.