unix: Check that file descriptors obtained from `--jobserver-auth=R,W` actually refer to a pipe

[petrochenkov]

cargo and rustc (and other tools using jobserver) will fail with a "failed to acquire jobserver token: early EOF on jobserver pipe" error in the next scenario reported in https://github.com/rust-lang/rust/issues/46981#issuecomment-681090839 as ICE in rustc:

• make executes a subprocess (e.g. shell) with the jobserver pipe closed (marked with FD_CLOEXEC), but the MAKEFLAGS env var will still contain --jobserver-auth=3,4 (or some other pair of integers) in the subprocess. This is where make is wrong, if it closes the descriptors for a subprocess, then it should probably clean up MAKEFLAGS for it as well, but it doesn't, so we have to live with make possibly providing garbage descriptors.
• the subprocess (e.g. shell) opens some files so descriptors 3 and 4 are taken again now, but refer to entirely unrelated files.
• the shell runs cargo, which sees that 3 and 4 are open, concludes that they refer to a jobserver, and fails when trying to read from them.

jobserver could be more resilient in the face of garbage descriptors if it checked not only that the descriptors are valid, but also that they actually refer to a pipe rather than random unrelated files.

[alexcrichton]

Currently there's a check that the fd is at leaset least a valid file descriptor, and adding another check there that it's a pipe makes sense to me!

[petrochenkov]

Sigh, the is_pipe logic was implemented originally, but then replaced with is_valid_fd in https://github.com/alexcrichton/jobserver-rs/pull/6. cc @ishitatsuyuki

[ishitatsuyuki]

I'm totally OK with reverting that change, as I have found that a socket wouldn't be any useful for jobservers.

It does make me wonder that make has the same checking logic but doesn't get affected. Not sure what's going on, but maybe submake gets some special treatment?

[petrochenkov]

@ishitatsuyuki Submake does get a special treatment, because the jobserver pipe is still open for it, so submake can inherit it. I'll try to check what happens if nested make gets garbage (but open) descriptors and tries to read from them, from source code it looks like it should fail with a fatal error, similarly to cargo.

[petrochenkov]

Ok, I think it's better to be consistent with make and keep the current behavior, so I'll close this issue.

However, what needs to be done is a better diagnostic in the erroneous cases.

• If cargo/rustc sees one of the MAKEFLAGS env vars, but the descriptors in it are closed, then it needs to emit a warning similar to make's "jobserver unavailable: using -j1. Add '+' to parent make rule.". Right now it will silently create a new jobserver leaving the user unaware about issues in their build system.
• If cargo/rustc fails to acquire a token from something that it thinks is a jobserver, we need to check whether it's a pipe and report if it's not, again making the user aware of misconfiguration. Also, rustc shouldn't ICE on this, only produce an error - build system misconfiguration is certainly not an internal compiler errors.

To do this the jobserver crate needs to produce slightly more detailed error types from from_env and acquire than it does now. I'll try to prototype these improvements for rustc and make a PR.