Skip to content

Commit

Permalink
Save and restore GetLastError() on Windows.
Browse files Browse the repository at this point in the history
This is a patch for the problem outlined in issue Ravenbrook#61 - that the value in
GetLastError() is not automatically saved and restored when a vectored exception
handler is called. This is solved the same way as errno was handled on POSIX
systems.

This patch does *not* save and restore errno on Windows, since it seems like the
MPS does not call any functions that modify errno on the critical path on
Windows (generally only functions from POSIX do that).
  • Loading branch information
fstromback committed Mar 13, 2021
1 parent 90c5972 commit ef5382a
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
10 changes: 10 additions & 0 deletions code/protw3.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,20 @@ LONG WINAPI ProtSEHfilter(LPEXCEPTION_POINTERS info)
AccessSet mode;
Addr base, limit;
LONG action;
DWORD lastError;
MutatorContextStruct context;

er = info->ExceptionRecord;

if(er->ExceptionCode != EXCEPTION_ACCESS_VIOLATION)
return EXCEPTION_CONTINUE_SEARCH;

/* This is the first point where we call a Windows API function that
* might change the last error. There are also no early returns from
* this point onwards.
*/
lastError = GetLastError();

MutatorContextInitFault(&context, info);

/* assert that the exception is continuable */
Expand Down Expand Up @@ -97,6 +104,9 @@ LONG WINAPI ProtSEHfilter(LPEXCEPTION_POINTERS info)
action = EXCEPTION_CONTINUE_SEARCH;
}

/* Restore the last error value before returning. */
SetLastError(lastError);

return action;
}

Expand Down
13 changes: 13 additions & 0 deletions design/thread-manager.txt
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ blocked in a system call, and the MPS signal handlers updating

.. _GitHub issue #10: https://github.com/ravenbrook/mps/issues/10

_`.req.thread.lasterror`: It would be nice if on Windows systems the
MPS does not cause system calls in the mutator to update the value
returned from ``GetLastError()`` when the exception handler is called
due to a fault. This may cause the MPS to destroy the previous value
there. (See `GitHub issue #61`_.)

.. _GitHub issue #61: https://github.com/Ravenbrook/mps/issues/61

Design
------
Expand Down Expand Up @@ -136,6 +143,12 @@ independent thread, should save and restore its value." All MPS
signals handlers therefore save and restore ``errno`` using the macros
``ERRNO_SAVE`` and ``ERRNO_RESTORE``.

_`.sol.thread.lasterror`: The documentation for ``AddVectoredExceptionHandler``
does not mention ``GetLastError()`` at all, but testing_ the behaviour
reveals that any value in ``GetLastError()`` is not preserved. Therefore,
this value is saved using ``LAST_ERROR_SAVE`` and ``LAST_ERROR_RESTORE``.

.. _testing: https://github.com/Ravenbrook/mps/issues/61

Interface
---------
Expand Down

0 comments on commit ef5382a

Please sign in to comment.