Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...` #124480

Enselic · 2024-04-28T16:10:17Z

In the stabilization attempt of #[unix_sigpipe = "sig_dfl"], a concern was raised related to using a language attribute for the feature: Long term, we want fn lang_start() to be definable by any crate, not just libstd. Having a special language attribute in that case becomes awkward.

So as a first step towards the next stabilization attempt, this PR changes the #[unix_sigpipe = "..."] attribute to a compiler flag -Zon-broken-pipe=... to remove that concern, since now the language is not "contaminated" by this feature.

Another point was also raised, namely that the ui should not leak how it does things, but rather what the end effect is. The new flag uses the proposed naming. This is of course something that can be iterated on further before stabilization.

Tracking issue: #97889

rustbot · 2024-04-28T16:10:26Z

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-04-28T16:10:28Z

The Miri subtree was changed

cc @rust-lang/miri

These commits modify compiler targets.
(See the Target Tier Policy.)

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

lnicola · 2024-04-28T16:37:17Z

LGTM from the rust-analyzer side.

jieyouxu

The changes look good to me on the compiler side. I'll roll a libs reviewer for the library part of this PR.

src/doc/unstable-book/src/compiler-flags/on-broken-pipe.md

jieyouxu · 2024-04-28T18:12:18Z

Rolling a libs reviewer for the libs part of this PR. If the changes also look good from the libs side, feel free to r=me.

r? libs

bors · 2024-04-29T12:09:06Z

☔ The latest upstream changes (presumably #124505) made this pull request unmergeable. Please resolve the merge conflicts.

src/tools/miri/src/eval.rs

Enselic · 2024-04-29T19:33:17Z

I have addressed comments and rebased (because of a smalll conflict with master).

diff of diff

FWIW, here is the diff of the reviewed diff and the rebased and updated diff:

$ git range-diff c4109eb^- 03dcde0^-
1:  c4109eb191a ! 1:  03dcde080e7 Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...`
    @@ Commit message
         libstd. Having a special language attribute in that case becomes
         awkward.

    -    So as a first step towards stabilization, this PR changes the
    -    `#[unix_sigpipe = "..."]` attribute to a compiler flag
    -    `-Zon-broken-pipe=...`, to remove that concern, since now the language
    +    So as a first step towards towards the next stabilization attempt, this
    +    PR changes the `#[unix_sigpipe = "..."]` attribute to a compiler flag
    +    `-Zon-broken-pipe=...` to remove that concern, since now the language
         is not "contaminated" by this feature.

         Another point was also raised, namely that the ui should not leak
    @@ src/doc/unstable-book/src/compiler-flags/on-broken-pipe.md (new)
     +
     +---
     +
    -+The `-Zon-broken-pipe=...` compiler flag  can be used to specify how libstd shall setup `SIGPIPE` on Unix platforms before invoking `fn main()`. This flag is ignored on non-Unix targets. There are three variants:
    -+* `-Zon-broken-pipe=inherit`
    -+* `-Zon-broken-pipe=kill`
    -+* `-Zon-broken-pipe=error`
     +
    -+## `-Zon-broken-pipe=inherit`
    ++## Overview
    ++
    ++The `-Zon-broken-pipe=...` compiler flag can be used to specify how libstd shall setup `SIGPIPE` on Unix platforms before invoking `fn main()`. This flag is ignored on non-Unix targets. The flag can be used with three different values or be omitted entirely. It affects `SIGPIPE` before `fn main()` and before children get `exec()`'ed:
    ++
    ++| Compiler flag              | `SIGPIPE` before `fn main()` | `SIGPIPE` before child `exec()` |
    ++|----------------------------|------------------------------|---------------------------------|
    ++| not used                   | `SIG_DFL`                    | `SIG_DFL`                       |
    ++| `-Zon-broken-pipe=kill`    | `SIG_DFL`                    | not touched                     |
    ++| `-Zon-broken-pipe=error`   | `SIG_IGN`                    | not touched                     |
    ++| `-Zon-broken-pipe=inherit` | not touched                  | not touched                     |
    ++
    ++
    ++## `-Zon-broken-pipe` not used
    ++
    ++If `-Zon-broken-pipe` is not used, libstd will behave in the manner it has since 2014, before Rust 1.0. `SIGPIPE` will be set to `SIG_IGN` before `fn main()` and result in `EPIPE` errors which are converted to `std::io::ErrorKind::BrokenPipe`.
    ++
    ++When spawning child processes, `SIGPIPE` will be set to `SIG_DFL` before doing the underlying `exec()` syscall.
     +
    -+Leave `SIGPIPE` untouched before entering `fn main()`. Unless the parent process has changed the default `SIGPIPE` handler from `SIG_DFL` to something else, this will behave the same as `-Zon-broken-pipe=kill`.
     +
     +## `-Zon-broken-pipe=kill`
     +
    -+Set the `SIGPIPE` handler to `SIG_DFL`. This will result in your program getting killed if it tries to write to a closed pipe. This is normally what you want if your program produces textual output.
    ++Set the `SIGPIPE` handler to `SIG_DFL` before invoking `fn main()`. This will result in your program getting killed if it tries to write to a closed pipe. This is normally what you want if your program produces textual output.
    ++
    ++When spawning child processes, `SIGPIPE` will not be touched. This normally means child processes inherit `SIG_DFL` for `SIGPIPE`.
     +
     +### Example
     +
    @@ src/doc/unstable-book/src/compiler-flags/on-broken-pipe.md (new)
     +
     +Set the `SIGPIPE` handler to `SIG_IGN` before invoking `fn main()`. This will result in `ErrorKind::BrokenPipe` errors if you program tries to write to a closed pipe. This is normally what you want if you for example write socket servers, socket clients, or pipe peers.
     +
    -+This is what libstd has done by default since 2014. (However, see the note on child processes below.)
    ++When spawning child processes, `SIGPIPE` will not be touched. This normally means child processes inherit `SIG_IGN` for `SIGPIPE`.
     +
     +### Example
     +
    @@ src/doc/unstable-book/src/compiler-flags/on-broken-pipe.md (new)
     +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
     +```
     +
    -+### Note on child processes
    ++## `-Zon-broken-pipe=inherit`
     +
    -+When spawning child processes, the legacy Rust behavior if `-Zon-broken-pipe=...` is not specified is to reset `SIGPIPE` to `SIG_DFL` first.
    ++Leave `SIGPIPE` untouched before entering `fn main()`. Unless the parent process has changed the default `SIGPIPE` handler from `SIG_DFL` to something else, this will behave the same as `-Zon-broken-pipe=kill`.
     +
    -+If `-Zon-broken-pipe=...` is specified, no matter what its value is, the signal disposition of `SIGPIPE` is no longer reset. This means that the child inherits the parent's `SIGPIPE` behavior.
    ++When spawning child processes, `SIGPIPE` will not be touched. This normally means child processes inherit `SIG_DFL` for `SIGPIPE`.

      ## src/doc/unstable-book/src/language-features/unix-sigpipe.md (deleted) ##
     @@
    @@ src/tools/miri/src/eval.rs: pub fn create_ecx<'mir, 'tcx: 'mir>(

                  // Always using DEFAULT is okay since we don't support signals in Miri anyway.
     -            // (This means we are effectively ignoring `#[unix_sigpipe]`.)
    -+            // (This means we are effectively ignoring `SIGPIPE`.)
    ++            // (This means we are effectively ignoring `-Zon-broken-pipe`.)
                  let sigpipe = rustc_session::config::sigpipe::DEFAULT;

                  ecx.call_function(
    @@ tests/ui/attributes/unix_sigpipe/unix_sigpipe-inherit.rs => tests/ui/runtime/on-

      extern crate libc;

    - // By default the Rust runtime resets SIGPIPE to SIG_DFL before exec:ing child
    + // By default the Rust runtime resets SIGPIPE to SIG_DFL before exec'ing child
     -// processes so opt-out of that with `#[unix_sigpipe = "sig_dfl"]`. See
     +// processes so opt-out of that with `-Zon-broken-pipe=kill`. See
      // https://github.com/rust-lang/rust/blob/bf4de3a874753bbee3323081c8b0c133444fed2d/library/std/src/sys/pal/unix/process/process_unix.rs#L359-L384

src/doc/unstable-book/src/compiler-flags/on-broken-pipe.md

Amanieu

r=me for the library part

compiler/rustc_passes/src/entry.rs

…pe=...` In the stabilization attempt of `#[unix_sigpipe = "sig_dfl"]`, a concern was raised related to using a language attribute for the feature: Long term, we want `fn lang_start()` to be definable by any crate, not just libstd. Having a special language attribute in that case becomes awkward. So as a first step towards towards the next stabilization attempt, this PR changes the `#[unix_sigpipe = "..."]` attribute to a compiler flag `-Zon-broken-pipe=...` to remove that concern, since now the language is not "contaminated" by this feature. Another point was also raised, namely that the ui should not leak **how** it does things, but rather what the **end effect** is. The new flag uses the proposed naming. This is of course something that can be iterated on further before stabilization.

Amanieu · 2024-05-02T18:45:19Z

@bors r=jieyouxu

bors · 2024-05-02T18:45:21Z

📌 Commit cde0cde has been approved by jieyouxu

It is now in the queue for this repository.

Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...` In the stabilization [attempt](rust-lang#120832) of `#[unix_sigpipe = "sig_dfl"]`, a concern was [raised ](rust-lang#120832 (comment)) related to using a language attribute for the feature: Long term, we want `fn lang_start()` to be definable by any crate, not just libstd. Having a special language attribute in that case becomes awkward. So as a first step towards the next stabilization attempt, this PR changes the `#[unix_sigpipe = "..."]` attribute to a compiler flag `-Zon-broken-pipe=...` to remove that concern, since now the language is not "contaminated" by this feature. Another point was [also raised](rust-lang#120832 (comment)), namely that the ui should not leak **how** it does things, but rather what the **end effect** is. The new flag uses the proposed naming. This is of course something that can be iterated on further before stabilization. Tracking issue: rust-lang#97889

…mpiler-errors Rollup of 8 pull requests Successful merges: - rust-lang#124293 (Let miri and const eval execute intrinsics' fallback bodies) - rust-lang#124418 (Use a proof tree visitor to refine the `Obligation` for error reporting in new solver) - rust-lang#124480 (Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...`) - rust-lang#124648 (Trim crate graph) - rust-lang#124656 (release notes 1.78: add link to interior-mut breaking change) - rust-lang#124658 (Migrate `run-make/doctests-keep-binaries` to new rmake.rs format) - rust-lang#124681 (zkvm: fix run_tests) - rust-lang#124687 (Make `Bounds.clauses` private) r? `@ghost` `@rustbot` modify labels: rollup

Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...` In the stabilization [attempt](rust-lang#120832) of `#[unix_sigpipe = "sig_dfl"]`, a concern was [raised ](rust-lang#120832 (comment)) related to using a language attribute for the feature: Long term, we want `fn lang_start()` to be definable by any crate, not just libstd. Having a special language attribute in that case becomes awkward. So as a first step towards the next stabilization attempt, this PR changes the `#[unix_sigpipe = "..."]` attribute to a compiler flag `-Zon-broken-pipe=...` to remove that concern, since now the language is not "contaminated" by this feature. Another point was [also raised](rust-lang#120832 (comment)), namely that the ui should not leak **how** it does things, but rather what the **end effect** is. The new flag uses the proposed naming. This is of course something that can be iterated on further before stabilization. Tracking issue: rust-lang#97889

…mpiler-errors Rollup of 8 pull requests Successful merges: - rust-lang#124418 (Use a proof tree visitor to refine the `Obligation` for error reporting in new solver) - rust-lang#124480 (Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...`) - rust-lang#124648 (Trim crate graph) - rust-lang#124656 (release notes 1.78: add link to interior-mut breaking change) - rust-lang#124658 (Migrate `run-make/doctests-keep-binaries` to new rmake.rs format) - rust-lang#124678 (Stabilize `split_at_checked`) - rust-lang#124681 (zkvm: fix run_tests) - rust-lang#124687 (Make `Bounds.clauses` private) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#124480 - Enselic:on-broken-pipe, r=jieyouxu Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...` In the stabilization [attempt](rust-lang#120832) of `#[unix_sigpipe = "sig_dfl"]`, a concern was [raised ](rust-lang#120832 (comment)) related to using a language attribute for the feature: Long term, we want `fn lang_start()` to be definable by any crate, not just libstd. Having a special language attribute in that case becomes awkward. So as a first step towards the next stabilization attempt, this PR changes the `#[unix_sigpipe = "..."]` attribute to a compiler flag `-Zon-broken-pipe=...` to remove that concern, since now the language is not "contaminated" by this feature. Another point was [also raised](rust-lang#120832 (comment)), namely that the ui should not leak **how** it does things, but rather what the **end effect** is. The new flag uses the proposed naming. This is of course something that can be iterated on further before stabilization. Tracking issue: rust-lang#97889

RalfJung · 2024-06-25T09:05:53Z

src/bootstrap/src/core/build_steps/tool.rs

+        // FIXME: Synthetic #[cfg(bootstrap)]. Remove when the bootstrap compiler supports it.
+        if build_compiler.stage > 0 {
+            cargo.rustflag("-Zon-broken-pipe=kill");
+        }


Turns out this regressed #123177.

…r-ozkan Revert rust-lang#131060 "Drop conditionally applied cargo `-Zon-broken-pipe=kill` flags" In [rust-lang#131059] we found out that `-Zon-broken-pipe=kill` is actually **load-bearing**[^1] for (at least) `rustc` and `rustdoc` to have the kill-process-on-broken-pipe behavior, e.g. `rustc --print=sysroot | false` will ICE and `rustdoc --print=sysroot | false` will panic on a broken pipe. This PR reverts 5a7058c (reverts PR rust-lang#131060) in favor of a future fix to *unconditionally* apply `-Zon-broken-pipe=kill` to tool builds and also not drop the `-Zon-broken-pipe=kill` flag for rustc binary builds. I could not figure out how to write a regression test for the `rustc --print=sysroot | false` behavior on Unix, so this is a plain revert for now. This revert will unfortunately reintroduce rust-lang#130980 until we fix it again with the different approach. See more details at <rust-lang#131059 (comment)> and in the timeline below. ### Timeline of kill-process-on-broken-pipe behavior changes See [`unix_sigpipe` tracking issue rust-lang#97889][rust-lang#97889] for more context around unix sigpipe handling. - From the very beginning since 2014, Rust binaries by default use `sig_ign`. This meant that if output pipe is broken yet the program tries to use `println!` and such, there will be a broken pipe panic from std. This lead to ICEs from e.g. `rustc --help | false` [rust-lang#34376]. - [rust-lang#49606] mitigated [rust-lang#34376] by adding an explicit signal handler to `rustc_driver` register a sigpipe handler with `SIG_DFL` which will cause the binary using `rustc_driver` to terminate if `rustc_driver::set_sigpipe_handler()` is called. `rustc`'s main binary wrapper uses `rustc_driver::set_sigpipe_handler()`, and so does `rustdoc`. - A more universal way to set sigpipe behavior for Unix was introduced as part of [rust-lang#97889], i.e. `# [unix_sigpipe = "sig_dfl"]` attribute. - [rust-lang#102587] migrated `rustc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. - [rust-lang#103495] migrated `rustdoc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. `rustc_driver::set_sigpipe_handler` was removed. - Following concerns about sigpipe setting UI in [rust-lang#97889], the UI for specifying sigpipe behavior was changed in [rust-lang#124480] from `#[unix_sigpipe = "sig_dfl"]` attribute to the commmand line flag `-Zon-broken-pipe=kill`. - In the same PR, `#[unix_sigpipe = "sig_dfl"]` were removed from `rustc` and `rustdoc` main binary crate entry points in favor of the command line flag. Kill-process-on-broken-pipe behavior was preserved by adding `-Zon-broken-pipe=kill` for `rustdoc` tool build step and `rustc` during compile steps. - [rust-lang#126934] added `-Zon-broken-pipe=kill` for tool builds *except* for cargo to help with some miri tests because at the time the PR was written, this would lead to a couple of cargo test failures. Conditionally setting `RUSTFLAGS` can lead to tool build invalidation, e.g. building `cargo` without `-Zon-broken-pipe=kill` but `clippy` with the flag can lead to invalidation of the tool build cache. This is not a problem at the time, because nothing (not even miri) tests built stage 1 cargo (all used initial cargo). - In [rust-lang#130634] we found out that `run-make` tests like `compiler-builtins` needed stage 1 cargo, not just beta bootstrap cargo, because there can be changes that are present in stage 1 cargo but absent in beta cargo, which was blocking a beta backport. - [rust-lang#130642] and later [rust-lang#130739] now build stage 1 cargo. And as previously mentioned, since `-Zon-broken-pipe=kill` was specifically *not* set for cargo, this caused tool build cache invalidation meaning rebuilds of stage 1 even if nothing in source was changed due to differing `RUSTFLAGS` since `run-make` also builds `rustdoc` and such [rust-lang#130980]. [rust-lang#34376]: rust-lang#34376 [rust-lang#49606]: rust-lang#49606 [rust-lang#97889]: rust-lang#97889 [rust-lang#102587]: rust-lang#102587 [rust-lang#103495]: rust-lang#103495 [rust-lang#124480]: rust-lang#124480 [rust-lang#130634]: rust-lang#130634 [rust-lang#130642]: rust-lang#130642 [rust-lang#130739]: rust-lang#130739 [rust-lang#130980]: rust-lang#130980 [rust-lang#131059]: rust-lang#131059 [^1]: rust-lang#131059 (comment) r? `@onur-ozkan` (or bootstrap)

…r-ozkan Revert rust-lang#131060 "Drop conditionally applied cargo `-Zon-broken-pipe=kill` flags" In [rust-lang#131059] we found out that `-Zon-broken-pipe=kill` is actually **load-bearing**[^1] for (at least) `rustc` and `rustdoc` to have the kill-process-on-broken-pipe behavior, e.g. `rustc --print=sysroot | false` will ICE and `rustdoc --print=sysroot | false` will panic on a broken pipe. This PR reverts 5a7058c (reverts PR rust-lang#131060) in favor of a future fix to *unconditionally* apply `-Zon-broken-pipe=kill` to tool builds and also not drop the `-Zon-broken-pipe=kill` flag for rustc binary builds. I could not figure out how to write a regression test for the `rustc --print=sysroot | false` behavior on Unix, so this is a plain revert for now. This revert will unfortunately reintroduce rust-lang#130980 until we fix it again with the different approach. See more details at <rust-lang#131059 (comment)> and in the timeline below. ### Timeline of kill-process-on-broken-pipe behavior changes See [`unix_sigpipe` tracking issue rust-lang#97889][rust-lang#97889] for more context around unix sigpipe handling. - From the very beginning since 2014, Rust binaries by default use `sig_ign`. This meant that if output pipe is broken yet the program tries to use `println!` and such, there will be a broken pipe panic from std. This lead to ICEs from e.g. `rustc --help | false` [rust-lang#34376]. - [rust-lang#49606] mitigated [rust-lang#34376] by adding an explicit signal handler to `rustc_driver` register a sigpipe handler with `SIG_DFL` which will cause the binary using `rustc_driver` to terminate if `rustc_driver::set_sigpipe_handler()` is called. `rustc`'s main binary wrapper uses `rustc_driver::set_sigpipe_handler()`, and so does `rustdoc`. - A more universal way to set sigpipe behavior for Unix was introduced as part of [rust-lang#97889], i.e. `# [unix_sigpipe = "sig_dfl"]` attribute. - [rust-lang#102587] migrated `rustc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. - [rust-lang#103495] migrated `rustdoc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. `rustc_driver::set_sigpipe_handler` was removed. - Following concerns about sigpipe setting UI in [rust-lang#97889], the UI for specifying sigpipe behavior was changed in [rust-lang#124480] from `#[unix_sigpipe = "sig_dfl"]` attribute to the commmand line flag `-Zon-broken-pipe=kill`. - In the same PR, `#[unix_sigpipe = "sig_dfl"]` were removed from `rustc` and `rustdoc` main binary crate entry points in favor of the command line flag. Kill-process-on-broken-pipe behavior was preserved by adding `-Zon-broken-pipe=kill` for `rustdoc` tool build step and `rustc` during compile steps. - [rust-lang#126934] added `-Zon-broken-pipe=kill` for tool builds *except* for cargo to help with some miri tests because at the time the PR was written, this would lead to a couple of cargo test failures. Conditionally setting `RUSTFLAGS` can lead to tool build invalidation, e.g. building `cargo` without `-Zon-broken-pipe=kill` but `clippy` with the flag can lead to invalidation of the tool build cache. This is not a problem at the time, because nothing (not even miri) tests built stage 1 cargo (all used initial cargo). - In [rust-lang#130634] we found out that `run-make` tests like `compiler-builtins` needed stage 1 cargo, not just beta bootstrap cargo, because there can be changes that are present in stage 1 cargo but absent in beta cargo, which was blocking a beta backport. - [rust-lang#130642] and later [rust-lang#130739] now build stage 1 cargo. And as previously mentioned, since `-Zon-broken-pipe=kill` was specifically *not* set for cargo, this caused tool build cache invalidation meaning rebuilds of stage 1 even if nothing in source was changed due to differing `RUSTFLAGS` since `run-make` also builds `rustdoc` and such [rust-lang#130980]. [rust-lang#34376]: rust-lang#34376 [rust-lang#49606]: rust-lang#49606 [rust-lang#97889]: rust-lang#97889 [rust-lang#102587]: rust-lang#102587 [rust-lang#103495]: rust-lang#103495 [rust-lang#124480]: rust-lang#124480 [rust-lang#130634]: rust-lang#130634 [rust-lang#130642]: rust-lang#130642 [rust-lang#130739]: rust-lang#130739 [rust-lang#130980]: rust-lang#130980 [rust-lang#131059]: rust-lang#131059 [^1]: rust-lang#131059 (comment) r? ``@onur-ozkan`` (or bootstrap)

Rollup merge of rust-lang#131108 - jieyouxu:revert-broken-pipe, r=onur-ozkan Revert rust-lang#131060 "Drop conditionally applied cargo `-Zon-broken-pipe=kill` flags" In [rust-lang#131059] we found out that `-Zon-broken-pipe=kill` is actually **load-bearing**[^1] for (at least) `rustc` and `rustdoc` to have the kill-process-on-broken-pipe behavior, e.g. `rustc --print=sysroot | false` will ICE and `rustdoc --print=sysroot | false` will panic on a broken pipe. This PR reverts 5a7058c (reverts PR rust-lang#131060) in favor of a future fix to *unconditionally* apply `-Zon-broken-pipe=kill` to tool builds and also not drop the `-Zon-broken-pipe=kill` flag for rustc binary builds. I could not figure out how to write a regression test for the `rustc --print=sysroot | false` behavior on Unix, so this is a plain revert for now. This revert will unfortunately reintroduce rust-lang#130980 until we fix it again with the different approach. See more details at <rust-lang#131059 (comment)> and in the timeline below. ### Timeline of kill-process-on-broken-pipe behavior changes See [`unix_sigpipe` tracking issue rust-lang#97889][rust-lang#97889] for more context around unix sigpipe handling. - From the very beginning since 2014, Rust binaries by default use `sig_ign`. This meant that if output pipe is broken yet the program tries to use `println!` and such, there will be a broken pipe panic from std. This lead to ICEs from e.g. `rustc --help | false` [rust-lang#34376]. - [rust-lang#49606] mitigated [rust-lang#34376] by adding an explicit signal handler to `rustc_driver` register a sigpipe handler with `SIG_DFL` which will cause the binary using `rustc_driver` to terminate if `rustc_driver::set_sigpipe_handler()` is called. `rustc`'s main binary wrapper uses `rustc_driver::set_sigpipe_handler()`, and so does `rustdoc`. - A more universal way to set sigpipe behavior for Unix was introduced as part of [rust-lang#97889], i.e. `# [unix_sigpipe = "sig_dfl"]` attribute. - [rust-lang#102587] migrated `rustc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. - [rust-lang#103495] migrated `rustdoc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. `rustc_driver::set_sigpipe_handler` was removed. - Following concerns about sigpipe setting UI in [rust-lang#97889], the UI for specifying sigpipe behavior was changed in [rust-lang#124480] from `#[unix_sigpipe = "sig_dfl"]` attribute to the commmand line flag `-Zon-broken-pipe=kill`. - In the same PR, `#[unix_sigpipe = "sig_dfl"]` were removed from `rustc` and `rustdoc` main binary crate entry points in favor of the command line flag. Kill-process-on-broken-pipe behavior was preserved by adding `-Zon-broken-pipe=kill` for `rustdoc` tool build step and `rustc` during compile steps. - [rust-lang#126934] added `-Zon-broken-pipe=kill` for tool builds *except* for cargo to help with some miri tests because at the time the PR was written, this would lead to a couple of cargo test failures. Conditionally setting `RUSTFLAGS` can lead to tool build invalidation, e.g. building `cargo` without `-Zon-broken-pipe=kill` but `clippy` with the flag can lead to invalidation of the tool build cache. This is not a problem at the time, because nothing (not even miri) tests built stage 1 cargo (all used initial cargo). - In [rust-lang#130634] we found out that `run-make` tests like `compiler-builtins` needed stage 1 cargo, not just beta bootstrap cargo, because there can be changes that are present in stage 1 cargo but absent in beta cargo, which was blocking a beta backport. - [rust-lang#130642] and later [rust-lang#130739] now build stage 1 cargo. And as previously mentioned, since `-Zon-broken-pipe=kill` was specifically *not* set for cargo, this caused tool build cache invalidation meaning rebuilds of stage 1 even if nothing in source was changed due to differing `RUSTFLAGS` since `run-make` also builds `rustdoc` and such [rust-lang#130980]. [rust-lang#34376]: rust-lang#34376 [rust-lang#49606]: rust-lang#49606 [rust-lang#97889]: rust-lang#97889 [rust-lang#102587]: rust-lang#102587 [rust-lang#103495]: rust-lang#103495 [rust-lang#124480]: rust-lang#124480 [rust-lang#130634]: rust-lang#130634 [rust-lang#130642]: rust-lang#130642 [rust-lang#130739]: rust-lang#130739 [rust-lang#130980]: rust-lang#130980 [rust-lang#131059]: rust-lang#131059 [^1]: rust-lang#131059 (comment) r? ``@onur-ozkan`` (or bootstrap)

Revert #131060 "Drop conditionally applied cargo `-Zon-broken-pipe=kill` flags" In [#131059] we found out that `-Zon-broken-pipe=kill` is actually **load-bearing**[^1] for (at least) `rustc` and `rustdoc` to have the kill-process-on-broken-pipe behavior, e.g. `rustc --print=sysroot | false` will ICE and `rustdoc --print=sysroot | false` will panic on a broken pipe. This PR reverts 5a7058c5a542ec42d1fa9b524f7b4f7d6845d1e9 (reverts PR #131060) in favor of a future fix to *unconditionally* apply `-Zon-broken-pipe=kill` to tool builds and also not drop the `-Zon-broken-pipe=kill` flag for rustc binary builds. I could not figure out how to write a regression test for the `rustc --print=sysroot | false` behavior on Unix, so this is a plain revert for now. This revert will unfortunately reintroduce #130980 until we fix it again with the different approach. See more details at <rust-lang/rust#131059 (comment)> and in the timeline below. ### Timeline of kill-process-on-broken-pipe behavior changes See [`unix_sigpipe` tracking issue #97889][#97889] for more context around unix sigpipe handling. - From the very beginning since 2014, Rust binaries by default use `sig_ign`. This meant that if output pipe is broken yet the program tries to use `println!` and such, there will be a broken pipe panic from std. This lead to ICEs from e.g. `rustc --help | false` [#34376]. - [#49606] mitigated [#34376] by adding an explicit signal handler to `rustc_driver` register a sigpipe handler with `SIG_DFL` which will cause the binary using `rustc_driver` to terminate if `rustc_driver::set_sigpipe_handler()` is called. `rustc`'s main binary wrapper uses `rustc_driver::set_sigpipe_handler()`, and so does `rustdoc`. - A more universal way to set sigpipe behavior for Unix was introduced as part of [#97889], i.e. `# [unix_sigpipe = "sig_dfl"]` attribute. - [#102587] migrated `rustc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. - [#103495] migrated `rustdoc` to use `#[unix_sigpipe = "sig_dfl"]` instead of `rustc_driver::set_sigpipe_handler`. `rustc_driver::set_sigpipe_handler` was removed. - Following concerns about sigpipe setting UI in [#97889], the UI for specifying sigpipe behavior was changed in [#124480] from `#[unix_sigpipe = "sig_dfl"]` attribute to the commmand line flag `-Zon-broken-pipe=kill`. - In the same PR, `#[unix_sigpipe = "sig_dfl"]` were removed from `rustc` and `rustdoc` main binary crate entry points in favor of the command line flag. Kill-process-on-broken-pipe behavior was preserved by adding `-Zon-broken-pipe=kill` for `rustdoc` tool build step and `rustc` during compile steps. - [#126934] added `-Zon-broken-pipe=kill` for tool builds *except* for cargo to help with some miri tests because at the time the PR was written, this would lead to a couple of cargo test failures. Conditionally setting `RUSTFLAGS` can lead to tool build invalidation, e.g. building `cargo` without `-Zon-broken-pipe=kill` but `clippy` with the flag can lead to invalidation of the tool build cache. This is not a problem at the time, because nothing (not even miri) tests built stage 1 cargo (all used initial cargo). - In [#130634] we found out that `run-make` tests like `compiler-builtins` needed stage 1 cargo, not just beta bootstrap cargo, because there can be changes that are present in stage 1 cargo but absent in beta cargo, which was blocking a beta backport. - [#130642] and later [#130739] now build stage 1 cargo. And as previously mentioned, since `-Zon-broken-pipe=kill` was specifically *not* set for cargo, this caused tool build cache invalidation meaning rebuilds of stage 1 even if nothing in source was changed due to differing `RUSTFLAGS` since `run-make` also builds `rustdoc` and such [#130980]. [#34376]: rust-lang/rust#34376 [#49606]: rust-lang/rust#49606 [#97889]: rust-lang/rust#97889 [#102587]: rust-lang/rust#102587 [#103495]: rust-lang/rust#103495 [#124480]: rust-lang/rust#124480 [#130634]: rust-lang/rust#130634 [#130642]: rust-lang/rust#130642 [#130739]: rust-lang/rust#130739 [#130980]: rust-lang/rust#130980 [#131059]: rust-lang/rust#131059 [^1]: rust-lang/rust#131059 (comment) r? ``@onur-ozkan`` (or bootstrap)

rustbot assigned jieyouxu Apr 28, 2024

lnicola approved these changes Apr 28, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

Enselic force-pushed the on-broken-pipe branch from b7d6669 to c4109eb Compare April 28, 2024 17:04

jieyouxu approved these changes Apr 28, 2024

View reviewed changes

src/doc/unstable-book/src/compiler-flags/on-broken-pipe.md Outdated Show resolved Hide resolved

rustbot assigned Amanieu and unassigned jieyouxu Apr 28, 2024

RalfJung reviewed Apr 29, 2024

View reviewed changes

src/tools/miri/src/eval.rs Outdated Show resolved Hide resolved

Enselic force-pushed the on-broken-pipe branch from c4109eb to 03dcde0 Compare April 29, 2024 19:27

Enselic commented May 1, 2024

View reviewed changes

src/doc/unstable-book/src/compiler-flags/on-broken-pipe.md Outdated Show resolved Hide resolved

Amanieu reviewed May 2, 2024

View reviewed changes

compiler/rustc_passes/src/entry.rs Outdated Show resolved Hide resolved

Enselic force-pushed the on-broken-pipe branch from 03dcde0 to cde0cde Compare May 2, 2024 17:54

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 May 2, 2024

compiler-errors mentioned this pull request May 4, 2024

Rollup of 8 pull requests #124688

Closed

This was referenced May 4, 2024

Rollup of 8 pull requests #124693

Closed

Rollup of 8 pull requests #124694

Merged

bors merged commit 9dfd527 into rust-lang:master May 4, 2024
6 checks passed

rustbot added this to the 1.80.0 milestone May 4, 2024

Enselic deleted the on-broken-pipe branch May 4, 2024 08:27

Enselic mentioned this pull request May 4, 2024

Tracking Issue for unix_sigpipe #97889

Open

23 tasks

RalfJung reviewed Jun 25, 2024

View reviewed changes

This was referenced Sep 30, 2024

Adjusting -Zon-broken-pipe=kill to only be applied to rustc/rustdoc binary wrappers break two cargo tests #131059

Closed

Revert #131060 "Drop conditionally applied cargo -Zon-broken-pipe=kill flags" #131108

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...` #124480

Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...` #124480

Enselic commented Apr 28, 2024 •

edited

Loading

rustbot commented Apr 28, 2024

rustbot commented Apr 28, 2024

lnicola commented Apr 28, 2024

This comment has been minimized.

jieyouxu left a comment

jieyouxu commented Apr 28, 2024

bors commented Apr 29, 2024

Enselic commented Apr 29, 2024 •

edited

Loading

Amanieu left a comment

Amanieu commented May 2, 2024

bors commented May 2, 2024

RalfJung Jun 25, 2024

Change SIGPIPE ui from #[unix_sigpipe = "..."] to -Zon-broken-pipe=... #124480

Change SIGPIPE ui from #[unix_sigpipe = "..."] to -Zon-broken-pipe=... #124480

Conversation

Enselic commented Apr 28, 2024 • edited Loading

rustbot commented Apr 28, 2024

rustbot commented Apr 28, 2024

lnicola commented Apr 28, 2024

This comment has been minimized.

jieyouxu left a comment

Choose a reason for hiding this comment

jieyouxu commented Apr 28, 2024

bors commented Apr 29, 2024

Enselic commented Apr 29, 2024 • edited Loading

Amanieu left a comment

Choose a reason for hiding this comment

Amanieu commented May 2, 2024

bors commented May 2, 2024

RalfJung Jun 25, 2024

Choose a reason for hiding this comment

Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...` #124480

Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...` #124480

Enselic commented Apr 28, 2024 •

edited

Loading

Enselic commented Apr 29, 2024 •

edited

Loading