Upstream findings from investigation of windows fault handler

[MarkusPettersson98]

This PR adds some fixes and comments that was found during my investigation of our exception handler on Windows that spawned from #7137. It contributes the following:

• Don't redefine Windows API function signatures, import them from windows_sys
• Get rid of unsafe where safety comments were missing and there were safe alternatives
• Add SAFETY comments to unsafe code where there exists documentation around what properties that need to be upheld. Add appropriate checks of properties if they were missing

---

This change is 

[linear]

DES-1432 Investigate Windows fault-handler

[dlon]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @MarkusPettersson98)

---

mullvad-daemon/src/exception_logging/win.rs line 47 at r1 (raw file):

fn generate_minidump(
    dump_file: &Path,
    exception_pointers: *mut EXCEPTION_POINTERS,

Why did this change from immutable to mutable? According to the docs (https://learn.microsoft.com/en-us/windows/win32/api/minidumpapiset/nf-minidumpapiset-minidumpwritedump), this parameter is in, so I suspect that it is in fact fine to pass in an immutable ref, regardless of what the type says 🤷 .

---

mullvad-daemon/src/exception_logging/win.rs line 139 at r1 (raw file):

    static REENTRANCY_GUARD: AtomicBool = AtomicBool::new(false);
    if REENTRANCY_GUARD.swap(true, Ordering::SeqCst) {
        // `process::abort` is signal-safe, unlike `process::exit`.

This comment ("signal-safe") is probably not relevant here?

[MarkusPettersson98]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dlon)

---

mullvad-daemon/src/exception_logging/win.rs line 47 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

Why did this change from immutable to mutable? According to the docs (https://learn.microsoft.com/en-us/windows/win32/api/minidumpapiset/nf-minidumpapiset-minidumpwritedump), this parameter is in, so I suspect that it is in fact fine to pass in an immutable ref, regardless of what the type says 🤷 .

Because MINIDUMP_EXCEPTION_INFORMATION expects a mutable pointer, so I reflected this in the signature of generate_minidump. What do you think is appropriate here?

---

mullvad-daemon/src/exception_logging/win.rs line 139 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

This comment ("signal-safe") is probably not relevant here?

Probably not. But should we return EXCEPTION_CONTINUE_SEARCH instead of aborting maybe?

[dlon]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @MarkusPettersson98)

---

mullvad-daemon/src/exception_logging/win.rs line 47 at r1 (raw file):

Previously, MarkusPettersson98 (Markus Pettersson) wrote…

Because MINIDUMP_EXCEPTION_INFORMATION expects a mutable pointer, so I reflected this in the signature of generate_minidump. What do you think is appropriate here?

I think generate_minidump should take in *const EXCEPTION_POINTERS and convert it to a mut pointer when calling MiniDumpWriteDump. So we don't treat it as mutable for longer than we need to.

---

mullvad-daemon/src/exception_logging/win.rs line 139 at r1 (raw file):
That seems fine:

Proceed with normal execution of UnhandledExceptionFilter. That means obeying the SetErrorMode flags, or invoking the Application Error pop-up message box.

Or simply EXCEPTION_EXECUTE_HANDLER, which is what this function returns in the normal case. I'm not sure what the difference is in practice. I doubt that a message box will be shown given that it is a service.