Skip to content
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

std: Render large exit codes as hex on Windows #57473

Merged
merged 1 commit into from
Jan 13, 2019

Conversation

alexcrichton
Copy link
Member

On Windows process exit codes are never signals but rather always 32-bit
integers. Most faults like segfaults and such end up having large
integers used to represent them, like STATUS_ACCESS_VIOLATION being
0xC0000005. Currently, however, when an ExitStatus is printed this
ends up getting rendered as 3221225477 which is somewhat more difficult
to debug.

This commit adds a branch in Display for ExitStatus on Windows which
handles exit statuses where the high bit is set and prints those exit
statuses as hex instead of with decimals. This will hopefully preserve
the current display for small exit statuses (like exit code: 22), but
assist in quickly debugging segfaults/access violations/etc. I've
found at least that the hex codes are easier to search for than decimal.

I wasn't able to find any official documentation saying that all system
exit codes have the high bit set, but I figure it's a good enough
heuristic for now.

@rust-highfive
Copy link
Collaborator

r? @Kimundi

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 9, 2019
@nagisa
Copy link
Member

nagisa commented Jan 9, 2019

I think it is a good heuristic.

cc @retep998

@retep998
Copy link
Member

retep998 commented Jan 9, 2019

Generally the system exit codes are due to an unhandled exception, and unhandled exceptions have an NTSTATUS code, which is the code that is returned when the program is terminated due to the unhandled exception. An NTSTATUS with the high bit set indicates a warning or an error. Since it's unlikely that the program would be crashing due to an NTSTATUS that indicates success, checking for the high bit is a very valid heuristic.

@Kimundi
Copy link
Member

Kimundi commented Jan 10, 2019

Seems sensible, and @retep998 approves, so...

@bors: r+

@bors
Copy link
Contributor

bors commented Jan 10, 2019

📌 Commit 415f514687e406f9f63a15604d1f3cb2fd92029d has been approved by Kimundi

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 10, 2019
@@ -393,7 +393,11 @@ impl From<c::DWORD> for ExitStatus {

impl fmt::Display for ExitStatus {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "exit code: {}", self.0)
if self.0 & 0x80000000 != 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have a comment explaining this heuristic briefly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@varkor Apparently, ExitProcess is a newtype for u32 on Windows. And Windows uses sometimes system error codes, sometimes NTSTATUS.

The self.0 & 0x80000000 != 0 for u32 acts like self.0 < 0 for i32, but I'd render informational status codes (the ones within 0x40000000 − 0x7FFFFFFF range) as hexadecimal as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why only large codes are printed in hexadecimal. What's the problem with printing all codes in hexadecimal? We wouldn't need any heuristics and comments then.
(Small codes are documented and searchable in both hexadecimal and decimal forms.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any examples of exceptions crashing the process that have an informational status codes though?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petrochenkov Because system error codes are decimal. For example, 12000 - 12175 is reserved for Internet Error codes. So, displaying it in hexadecimal would be the opposite of this PR.

@bors
Copy link
Contributor

bors commented Jan 10, 2019

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Jan 10, 2019

📌 Commit 415f514687e406f9f63a15604d1f3cb2fd92029d has been approved by Kimundi

On Windows process exit codes are never signals but rather always 32-bit
integers. Most faults like segfaults and such end up having large
integers used to represent them, like STATUS_ACCESS_VIOLATION being
0xC0000005. Currently, however, when an `ExitStatus` is printed this
ends up getting rendered as 3221225477 which is somewhat more difficult
to debug.

This commit adds a branch in `Display for ExitStatus` on Windows which
handles exit statuses where the high bit is set and prints those exit
statuses as hex instead of with decimals. This will hopefully preserve
the current display for small exit statuses (like `exit code: 22`), but
assist in quickly debugging segfaults/access violations/etc. I've
found at least that the hex codes are easier to search for than decimal.

I wasn't able to find any official documentation saying that all system
exit codes have the high bit set, but I figure it's a good enough
heuristic for now.
@alexcrichton
Copy link
Member Author

@bors: r=Kimundi

@bors
Copy link
Contributor

bors commented Jan 10, 2019

📌 Commit bbb5448 has been approved by Kimundi

pietroalbini added a commit to pietroalbini/rust that referenced this pull request Jan 12, 2019
…, r=Kimundi

std: Render large exit codes as hex on Windows

On Windows process exit codes are never signals but rather always 32-bit
integers. Most faults like segfaults and such end up having large
integers used to represent them, like STATUS_ACCESS_VIOLATION being
0xC0000005. Currently, however, when an `ExitStatus` is printed this
ends up getting rendered as 3221225477 which is somewhat more difficult
to debug.

This commit adds a branch in `Display for ExitStatus` on Windows which
handles exit statuses where the high bit is set and prints those exit
statuses as hex instead of with decimals. This will hopefully preserve
the current display for small exit statuses (like `exit code: 22`), but
assist in quickly debugging segfaults/access violations/etc. I've
found at least that the hex codes are easier to search for than decimal.

I wasn't able to find any official documentation saying that all system
exit codes have the high bit set, but I figure it's a good enough
heuristic for now.
Centril added a commit to Centril/rust that referenced this pull request Jan 13, 2019
…, r=Kimundi

std: Render large exit codes as hex on Windows

On Windows process exit codes are never signals but rather always 32-bit
integers. Most faults like segfaults and such end up having large
integers used to represent them, like STATUS_ACCESS_VIOLATION being
0xC0000005. Currently, however, when an `ExitStatus` is printed this
ends up getting rendered as 3221225477 which is somewhat more difficult
to debug.

This commit adds a branch in `Display for ExitStatus` on Windows which
handles exit statuses where the high bit is set and prints those exit
statuses as hex instead of with decimals. This will hopefully preserve
the current display for small exit statuses (like `exit code: 22`), but
assist in quickly debugging segfaults/access violations/etc. I've
found at least that the hex codes are easier to search for than decimal.

I wasn't able to find any official documentation saying that all system
exit codes have the high bit set, but I figure it's a good enough
heuristic for now.
bors added a commit that referenced this pull request Jan 13, 2019
Rollup of 16 pull requests

Successful merges:

 - #57351 (Don't actually create a full MIR stack frame when not needed)
 - #57353 (Optimise floating point `is_finite` (2x) and `is_infinite` (1.6x).)
 - #57412 (Improve the wording)
 - #57436 (save-analysis: use a fallback when access levels couldn't be computed)
 - #57453 (lldb_batchmode.py: try `import _thread` for Python 3)
 - #57454 (Some cleanups for core::fmt)
 - #57461 (Change `String` to `&'static str` in `ParseResult::Failure`.)
 - #57473 (std: Render large exit codes as hex on Windows)
 - #57474 (save-analysis: Get path def from parent in case there's no def for the path itself.)
 - #57494 (Speed up item_bodies for large match statements involving regions)
 - #57496 (re-do docs for core::cmp)
 - #57508 (rustdoc: Allow inlining of reexported crates and crate items)
 - #57547 (Use `ptr::eq` where applicable)
 - #57557 (resolve: Mark extern crate items as used in more cases)
 - #57560 (hygiene: Do not treat `Self` ctor as a local variable)
 - #57564 (Update the const fn tracking issue to the new metabug)

Failed merges:

r? @ghost
@bors bors merged commit bbb5448 into rust-lang:master Jan 13, 2019
@alexcrichton alexcrichton deleted the hex-display-on-windows branch January 17, 2019 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants