Translate NTSTATUS into the last-error slot instead of discarding it

[Borega]

Split out of #4 at @Zvendson's request — this is the actual root cause he asked for:

The main issue is, that we would actually need to properly handle every win32 function call here. Instead of appending every Win32Exception (which can be indeed none), we should figure out which edge case is reporting a 0 win error that causes a successful call with a wrong result

The edge case is NtUnmapViewOfSection in SharedMemory.destroy().

Why

NTSTATUS routines report failure in their return value and never call SetLastError. The five wrappers in windows.py collapsed that to a bool:

nt_status: int = _NtUnmapViewOfSection(process_handle, base_address)
return nt_status == STATUS_SUCCESS

The NTSTATUS is the only copy of the error, and it is dropped on the floor. A Win32Exception() raised afterwards reads whatever the previous call left in the thread-local slot. Measured on Win11 x64, 32-bit CPython:

NtUnmapViewOfSection(bad addr):        NTSTATUS=0xC0000019  GetLastError=0
after SetLastError(1234), same call:   NTSTATUS=0xC0000019  GetLastError=1234   <- stale
CloseHandle(pseudo) succeeds:                               GetLastError=1234   <- success never clears

destroy() runs UnmapViewOfFile → CloseHandle → NtUnmapViewOfSection → DuplicateHandle, appending a Win32Exception() after each. So:

• steps 1–2 succeeded, step 3 fails → slot is 0 → FormatMessageW(0) → "The operation completed successfully." This is the "0 win error" from your comment.
• steps 1–2 failed → step 3 reports their code. Wrong but plausible, and harder to notice than the success message.

Either way STATUS_NOT_MAPPED_VIEW — the real failure — is gone.

Fix

ntdll ships RtlNtStatusToDosError for exactly this:

0xC0000019 -> 487  Attempt to access invalid address.
0xC0000008 ->   6  The handle is invalid.
0xC000000D ->  87  The parameter is incorrect.

_nt_ok() translates and publishes, so the failure survives to the call site regardless of what ran before. Success leaves the slot untouched, matching how a successful Win32 call behaves. Applied to all five NTSTATUS wrappers: NtMapViewOfSection, NtUnmapViewOfSection, NtQueryInformationProcess, NtSuspendProcess, NtResumeProcess.

SetLastError and RtlNtStatusToDosError are exported alongside the existing GetLastError.

Tests

tests/test_ntstatus_last_error.py (5 tests): the translation table, a real NtUnmapViewOfSection failure from both a clean (0) and a poisoned (1234) last-error state, and that success does not clobber the slot. Three of the five fail without this change — verified by reverting _nt_ok's body:

FAILED test_nt_ok_publishes_the_translated_failure
FAILED test_failure_is_reported_even_from_a_clean_last_error
FAILED test_failure_is_not_masked_by_a_stale_last_error
assert 1234 == 487

Full suite: 182 passed, 2 skipped.

Relation to #4

With this merged, #4's zero-guard stops being the fix and becomes a cheap backstop for any call site still doing it wrong. I'll rebase #4 onto this and reframe it as Win32Exception.args/__reduce__ + SharedMemoryCleanupError, which are independent of the root cause.

---

One thing I deliberately left out, happy to do it separately if you want it: every wrapper uses windll rather than WinDLL(..., use_last_error=True), so GetLastError() is a second FFI call and the CPython runtime can clobber the slot in between. ctypes.get_last_error() closes that hole, but it touches all ~70 declarations — too big to smuggle in here.

[Copilot]

Pull request overview

This PR fixes incorrect post-failure error reporting for NTSTATUS-returning ntdll calls by translating NTSTATUS failures into the thread’s Win32 last-error slot, so downstream Win32Exception() instances consistently reflect the real failure instead of GetLastError()==0 or stale values.

Changes:

• Add SetLastError, RtlNtStatusToDosError, and _nt_ok() to translate/publish NTSTATUS failures into GetLastError().
• Update the five NTSTATUS wrappers (NtMapViewOfSection, NtUnmapViewOfSection, NtQueryInformationProcess, NtSuspendProcess, NtResumeProcess) to use _nt_ok().
• Add regression tests covering translation correctness and last-error behavior across clean/poisoned states.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
MemLib/windows.py	Adds NTSTATUS→Win32 translation/publishing helpers and routes NTSTATUS wrappers through _nt_ok() so failures populate the last-error slot.
tests/test_ntstatus_last_error.py	Adds regression tests ensuring NTSTATUS failures are translated into GetLastError() and success does not clobber the slot.

Suppressed comments (1)

tests/test_ntstatus_last_error.py:41

• After renaming the NTSTATUS constant at the top of the file, this test should use the updated name for consistency.

def test_nt_ok_publishes_the_translated_failure():
    SetLastError(0)

    assert _nt_ok(STATUS_CONFLICTING_ADDRESSES) is False
    assert GetLastError() == ERROR_INVALID_ADDRESS

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Borega]

Correct, and thanks -- fixed in 66456d5. ntstatus.h confirms it:

#define STATUS_CONFLICTING_ADDRESSES     ((NTSTATUS)0xC0000018L)
#define STATUS_NOT_MAPPED_VIEW           ((NTSTATUS)0xC0000019L)

NtUnmapViewOfSection(0xDEAD0000) does return 0xC0000019, so the value was right and only the name was wrong. Both statuses happen to translate to ERROR_INVALID_ADDRESS (487), so the test passed either way -- which is exactly why the wrong name was worth fixing rather than shrugging at: it would have misled the next reader about which failure is being asserted, with nothing failing to correct them.

Renamed the constant to STATUS_NOT_MAPPED_VIEW and dropped the stale comment. Same mistake was in my commit message and PR body; the PR body is now corrected.

[Borega]

Agreed -- fixed in 66456d5. _nt_ok now calls the public SetLastError/RtlNtStatusToDosError.

The module convention already backs this up: Win32Exception.__init__ calls the public GetLastError(), and _GetLastError() is referenced exactly once, inside its own wrapper. My use of the private bindings was the outlier.

Your point about ctypes.get_last_error() is the one that decided it -- that swap would otherwise need touching every internal call site instead of just the two wrappers. Which is also why I left that swap out of this PR: all ~70 declarations use windll rather than WinDLL(..., use_last_error=True), so GetLastError() is a second FFI call and the CPython runtime can clobber the slot in between. Real hole, but too big to smuggle in here -- separate PR if @Zvendson wants it.