Skip to content

TSCBasic: properly track abnormal exits on Windows #271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 20, 2021

Conversation

compnerd
Copy link
Member

The process handling did not properly account for abnormal process
termination. Unlike Unix platforms, Windows does not have a concept of
signals. However, a process may be terminated abnormally by means of an
exception. The exception code is then transliterated into the exit code
for the process. In most circumstances, the exit code for the process
in such a case is either a NTSTATUS or HRESULT. Accordingly, the
way to detect the exception is to check the severity code of the
exception. The HRESULT and NTSTATUS codes are similarly structured:

                     1                     2                  3
 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|S|R|C|N|X|F A C I L I T Y      |C O D E                          |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
S: Severity; If set, indicates a failure result. If clear, indicates a success result.
R: Reserved; If N is clear, this bit MUST be set to 0.
C: Customer; If set, indicates that this a customer defined error code.
N: If set, indicates that the error code is a NTSTATUS value.
X: Reserved; MBZ
Facility: An indicator of the source of the error.
Code: The remainder of the error code.

We would previously only check for the top nibble being 0xC or 0xE.
However, a 0x8 value is also permissible and indicates a failure. Plumb
this through as an abnormal exit with the specified exception code.

@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd
Copy link
Member Author

@swift-ci please test

The process handling did not properly account for abnormal process
termination.  Unlike Unix platforms, Windows does not have a concept of
signals.  However, a process may be terminated abnormally by means of an
exception.  The exception code is then transliterated into the exit code
for the process.  In most circumstances, the exit code for the process
in such a case is either a `NTSTATUS` or `HRESULT`.  Accordingly, the
way to detect the exception is to check the severity code of the
exception.  The `HRESULT` and `NTSTATUS` codes are similarly structured:

~~~
                     1                     2                  3
 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|S|R|C|N|X|F A C I L I T Y      |C O D E                          |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
S: Severity; If set, indicates a failure result. If clear, indicates a success result.
R: Reserved; If N is clear, this bit MUST be set to 0.
C: Customer; If set, indicates that this a customer defined error code.
N: If set, indicates that the error code is a NTSTATUS value.
X: Reserved; MBZ
Facility: An indicator of the source of the error.
Code: The remainder of the error code.
~~~

We would previously only check for the top nibble being 0xC or 0xE.
However, a 0x8 value is also permissible and indicates a failure.  Plumb
this through as an abnormal exit with the specified exception code.
@compnerd
Copy link
Member Author

@swift-ci please test

@tomerd
Copy link
Contributor

tomerd commented Dec 20, 2021

@compnerd please also create a cherry pick to 5.6 once merged

@compnerd compnerd merged commit 3b5b1e5 into swiftlang:main Dec 20, 2021
@compnerd compnerd deleted the exception branch December 20, 2021 19:52
@compnerd
Copy link
Member Author

#272 is the cherry-pick for 5.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants