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

ExitStatus Debug instance is misleading #74832

Closed
infinity0 opened this issue Jul 27, 2020 · 1 comment · Fixed by #88305
Closed

ExitStatus Debug instance is misleading #74832

infinity0 opened this issue Jul 27, 2020 · 1 comment · Fixed by #88305
Labels
C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@infinity0
Copy link
Contributor

On UNIX, c_int is an alias for i32 or i16 depending on the platform. ExitStatus is a wrapper around c_int. However its Debug instance only exposes the last few bits:

test.c

#include <stdio.h>
void main() {
    printf("%s\n", (void*)123);
}

running:

$ gcc test.c && ./a.out
Segmentation fault
exit code 139

test.rs

use std::process::Command;

#[derive(PartialEq, Eq, Clone, Copy, Debug)]
pub struct X(i16);
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
pub struct XX(X);

pub fn main() {
  let e = Command::new("./a.out").status().unwrap();
  let i = XX(X(139));
  println!("exit with {:?}", e);
  println!("exit with {}", e);
  println!("test reproduce {:?}", i);
}

running:

$ rustc test.rs && ./test
exit with ExitStatus(ExitStatus(11))
exit with signal: 11
test reproduce XX(X(139))

The Debug output makes it look like a regular exit, not a signal exit. This is relevant because this is what rustbuild's rustc wrapper does in src/bootstrap/bin/rustc.rs, making rustc builds harder to debug.

As you can see I tried to recreate the weirdness with XX which as far as I can tell, follows the structure of how ExitStatus is defined. However I didn't recreate the platform-dependent logic in libstd/sys/mod.rs which also includes missing_debug_implementations which might be interfering with the derivation logic.

@infinity0 infinity0 added the C-bug Category: This is a bug. label Jul 27, 2020
@jonas-schievink jonas-schievink added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jul 27, 2020
@infinity0
Copy link
Contributor Author

Looking into this in a bit more detail, my interpretation in the OP is wrong, but the C/POSIX standards are confusing for these things. Apparently exit status of a UNIX process an as output of waitpid in the parent, takes entirely different values to the exit status given as an input to exit(). If you amend test.c in the OP to this:

#include <stdio.h>
void main() {
    exit(11);
}

then run the rust code again, it will output:

$ rustc test.rs && ./test
exit with ExitStatus(ExitStatus(2816))
exit with exit code: 11
test reproduce XX(X(139))

In other words, exit(11) i.e. exit(0x0b) as an input in the child program gets translated into ExitStatus(2816) i.e. ExitStatus(0x0b00) as an output in the parent program, and the exit code (for non-signal termination) is stored in the higher bits. The value 139 is something else constructed by bash, from man bash:

       The return value of a simple command is its exit status, or 128+n if the command is terminated by signal n.

I am not sure what to do about this bug report. The Debug instance is misleading, but it is not a direct fault of rust, but of the various systems it's running on top of.

For sure I will file another PR to patch up rustbuild's rustc wrapper to use Display instead of Debug.

infinity0 added a commit to infinity0/rust that referenced this issue Jul 27, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 28, 2020
rustbuild: use Display for exit status instead of Debug, see rust-lang#74832 for justification
ijackson added a commit to ijackson/rust that referenced this issue Aug 24, 2021
These structs have misleading names.  An ExitStatus[Error] is actually
a Unix wait status; an ExitCode is actually an exit status.

The Display impls are fixed, but the Debug impls are still misleading,
as reported in rust-lang#74832.

Fix this by pretending that these internal structs are called
`unix_exit_status` and `unix_wait_status` as applicable.  (We can't
actually rename the structs because of the way that the cross-platform
machinery works: the names are cross-platform.)

Signed-off-by: Ian Jackson <[email protected]>
Manishearth added a commit to Manishearth/rust that referenced this issue Oct 4, 2021
Manual Debug for Unix ExitCode ExitStatus ExitStatusError

These structs have misleading names.  An ExitStatus[Error] is actually a Unix wait status; an ExitCode is actually an exit status.  These misleading names appear in the `Debug` output.

The `Display` impls on Unix have been improved, but the `Debug` impls are still misleading, as reported in rust-lang#74832.

Fix this by pretending that these internal structs are called `unix_exit_status` and `unix_wait_status` as applicable.  (We can't actually rename the structs because of the way that the cross-platform machinery works: the names are cross-platform.)

After this change, this program
```
#![feature(exit_status_error)]
fn main(){
    let x = std::process::Command::new("false").status().unwrap();
    dbg!(x.exit_ok());
    eprintln!("x={:?}",x);
}
```
produces this output
```
[src/main.rs:4] x.exit_ok() = Err(
    ExitStatusError(
        unix_wait_status(
            256,
        ),
    ),
)
x=ExitStatus(unix_wait_status(256))
```

Closes rust-lang#74832
@bors bors closed this as completed in e4d257e Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants