Consider returning errors instead of `throw_machine_stop!`

[YohDeadfall]

The current behavior for Windows implies that Windows API shims always return the success status code (or abort execution), but that might not be the desired behavior, or at least not always. There are just two shims that use throw_machine_stop! via the invalid_handle function:

• CloseHandle,
• WaitForSingleObject.

[YohDeadfall]

@rustbot label +A-windows +C-proposal

[RalfJung]

The current behavior for Windows implies that Windows API shims always return the success status code,

I am confused by this remark, in combination with the issue title. If they always return success, then they can never throw "machine stop"...?

[RalfJung]

Regarding the behavior on an invalid handle, I am not a Windows API expert.

@ChrisDenton is program abort on invalid handle in CloseHandle a reasonable behavior or could there be reasonable programs that require continuing execution with a proper error code instead?

[YohDeadfall]

I am confused by this remark, in combination with the issue title. If they always return success, then they can never throw "machine stop"...?

I meant that it's either throw_machine_stop! or the success status code, but never an error status code. For other platforms invalid handles cause errors to be returned.

[RalfJung]

Yeah but other platforms are not Windows. ;)

These are fully invalid handles, not just already-closed file descriptors. Handles have a lot more structure than FDs do on Unix.

[YohDeadfall]

Windows has some kind of a virtual file system, actually, but that's different from what Unix has. The kernel has a root directory \ containing other directories or objects under it (and any directory is an object too, even an object type is represented as an object under \ObjectTypes). Each object has a set of methods provided by the corresponding type:

• DumpProcedure
• OpenProcedure
• CloseProcedure
• DeleteProcedure
• ParseProcedure
• SecurityProcedure
• QueryNameProcedure
• OkayToCloseProcedure

The parse and query methods are used for objects relying in a namespace different from the object manager namespace. So, it's possible to have secondary namespaces like for file systems. As an example, when one opens a file named C:\Users\RalfJing\example.txt the kernel performs the following steps:

1. Checks \GLOBAL?? for the first part of the path, C:.
2. Follows the symbolic link \GLOBAL??\C: that goes to \Device\HarddiskVolumeN where N is a volume number.
3. Executes ParseProcedure for HarddiskVolumeN with the rest of the provided path, i.e. Users\RalfJing\example.txt

And here's the difference, in Unix-like systems there's a single namespace, while Windows supports multiple of them. And the object manager in Windows isn't able to see objects in these secondary namespaces.

Invalid file handles doesn't crash Windows, even when provided for functions not expecting invalid handles at all like GetMessage. Still there's a nuance, the GetMessage function I mentioned returns BOOL (defined as int) that has two possible values, TRUE (1) and FALSE (0), but the function also returns -1 when an invalid handle passed to it.

Now imagine that one writes an app like:

MSG msg;
while (GetMessage(&msg, NULL, 0, 0)) {
 ...
}

Then it will loop forever even being totally invalid. In that case I would expect Miri to crash the app. Moreover, Windows did exactly that originally. The explanation of this you can find in Raymond's blog, Only an idiot would have parameter validation, and only an idiot would not have it. And I'm sure you will enjoy that short reading (:

Not a Windows expert anymore, but I still remember something. In the worst case there's always MSDN, Windows Internals books, and a possibility to run a Windows VM.

[ChrisDenton]

Regarding the behavior on an invalid handle, I am not a Windows API expert.

@ChrisDenton is program abort on invalid handle in CloseHandle a reasonable behavior or could there be reasonable programs that require continuing execution with a proper error code instead?

For CloseHandle specifically. the docs say it will throw an exception when debugging.

If the application is running under a debugger, the function will throw an exception if it receives either a handle value that is not valid or a pseudo-handle value. This can happen if you close a handle twice, or if you call CloseHandle on a handle returned by the FindFirstFile function instead of calling the FindClose function.

Personally I think it's reasonable for miri to have debugger-like behaviour. I can't think of a good reason to pass an invalid or pseudo handle to CloseHandle in a language with strong typing, such as Rust.

EDIT: which is not to state that such reasons definitely don't exist. But I think it reasonable to default to the stricter behaviour and only loosen it if a need arises.

[YohDeadfall]

Personally I think it's reasonable for miri to have debugger-like behaviour. I can't think of a good reason to pass an invalid or pseudo handle to CloseHandle in a language with strong typing, such as Rust.

Should WaitForSingleObject be the only case where an error is returned?

[ChrisDenton]

Personally I think it's reasonable for miri to have debugger-like behaviour. I can't think of a good reason to pass an invalid or pseudo handle to CloseHandle in a language with strong typing, such as Rust.

Should WaitForSingleObject be the only case where an error is returned?

I'm not sure how far miri wants to go in terms of accurately modelling Windows behaviour vs catching programmer errors. Passing an invalid HANDLE to WaitForSingleObject always indicates a programmer error (though perhaps an argument could be made for allowing null to always error). Tbh, even passing a pseudo handle is a programmer error because I can't think of any circumstances where that would be intended. Waiting on the current process or current thread cannot do anything but deadlock.

[RalfJung]

In that case I think I'd prefer to make this a program abort, until someone files an issue saying this is a problem for them.

[YohDeadfall]

Not yet completed, @RalfJung. WaitForSingleObject allows deadlocking:

https://github.com/rust-lang/miri/blob/052bdcb72dda94f0ac0284304503d25de28acc6b/src/shims/windows/thread.rs#L72

[RalfJung]

Yeah we'll report deadlocks via the usual detection.

I guess we won't notice the deadlock if there are other threads still making progress, but that's a general thing, not just specifically here. So IMO it's not worth tracking separately for Windows.