Skip to content

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

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
May 4, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions compiler/rustc/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![feature(unix_sigpipe)]

// A note about jemalloc: rustc uses jemalloc when built for CI and
// distribution. The obvious way to do this is with the `#[global_allocator]`
// mechanism. However, for complicated reasons (see
@@ -34,7 +32,6 @@
// https://github.com/rust-lang/rust/commit/b90cfc887c31c3e7a9e6d462e2464db1fe506175#diff-43914724af6e464c1da2171e4a9b6c7e607d5bc1203fa95c0ab85be4122605ef
// for an example of how to do so.

#[unix_sigpipe = "sig_dfl"]
fn main() {
// See the comment at the top of this file for an explanation of this.
#[cfg(feature = "jemalloc-sys")]
4 changes: 0 additions & 4 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
@@ -396,10 +396,6 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
),

// Entry point:
gated!(
unix_sigpipe, Normal, template!(NameValueStr: "inherit|sig_ign|sig_dfl"), ErrorFollowing,
EncodeCrossCrate::Yes, experimental!(unix_sigpipe)
),
ungated!(start, Normal, template!(Word), WarnFollowing, EncodeCrossCrate::No),
ungated!(no_start, CrateLevel, template!(Word), WarnFollowing, EncodeCrossCrate::No),
ungated!(no_main, CrateLevel, template!(Word), WarnFollowing, EncodeCrossCrate::No),
2 changes: 0 additions & 2 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
@@ -619,8 +619,6 @@ declare_features! (
/// Allows creation of instances of a struct by moving fields that have
/// not changed from prior instances of the same struct (RFC #2528)
(unstable, type_changing_struct_update, "1.58.0", Some(86555)),
/// Enables rustc to generate code that instructs libstd to NOT ignore SIGPIPE.
(unstable, unix_sigpipe, "1.65.0", Some(97889)),
/// Allows unnamed fields of struct and union type
(incomplete, unnamed_fields, "1.74.0", Some(49804)),
/// Allows unsized fn parameters.
3 changes: 2 additions & 1 deletion compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
@@ -20,7 +20,7 @@ use rustc_span::source_map::{RealFileLoader, SourceMapInputs};
use rustc_span::symbol::sym;
use rustc_span::{FileName, SourceFileHashAlgorithm};
use rustc_target::spec::{
CodeModel, LinkerFlavorCli, MergeFunctions, PanicStrategy, RelocModel, WasmCAbi,
CodeModel, LinkerFlavorCli, MergeFunctions, OnBrokenPipe, PanicStrategy, RelocModel, WasmCAbi,
};
use rustc_target::spec::{RelroLevel, SanitizerSet, SplitDebuginfo, StackProtector, TlsModel};
use std::collections::{BTreeMap, BTreeSet};
@@ -809,6 +809,7 @@ fn test_unstable_options_tracking_hash() {
tracked!(no_profiler_runtime, true);
tracked!(no_trait_vptr, true);
tracked!(no_unique_section_names, true);
tracked!(on_broken_pipe, OnBrokenPipe::Kill);
tracked!(oom, OomStrategy::Panic);
tracked!(osx_rpath_install_name, true);
tracked!(packed_bundled_libs, true);
3 changes: 0 additions & 3 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
@@ -695,9 +695,6 @@ passes_transparent_incompatible =
passes_undefined_naked_function_abi =
Rust ABI is unsupported in naked functions

passes_unix_sigpipe_values =
valid values for `#[unix_sigpipe = "..."]` are `inherit`, `sig_ign`, or `sig_dfl`

passes_unknown_external_lang_item =
unknown external lang item: `{$lang_item}`

1 change: 0 additions & 1 deletion compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
@@ -2522,7 +2522,6 @@ fn check_invalid_crate_level_attr(tcx: TyCtxt<'_>, attrs: &[Attribute]) {
sym::automatically_derived,
sym::start,
sym::rustc_main,
sym::unix_sigpipe,
sym::derive,
sym::test,
sym::test_case,
43 changes: 10 additions & 33 deletions compiler/rustc_passes/src/entry.rs
Original file line number Diff line number Diff line change
@@ -12,8 +12,7 @@ use rustc_span::symbol::sym;
use rustc_span::{Span, Symbol};

use crate::errors::{
AttrOnlyInFunctions, AttrOnlyOnMain, AttrOnlyOnRootMain, ExternMain, MultipleRustcMain,
MultipleStartFunctions, NoMainErr, UnixSigpipeValues,
AttrOnlyInFunctions, ExternMain, MultipleRustcMain, MultipleStartFunctions, NoMainErr,
};

struct EntryContext<'tcx> {
@@ -67,11 +66,7 @@ fn find_item(id: ItemId, ctxt: &mut EntryContext<'_>) {
ctxt.tcx.opt_item_name(id.owner_id.to_def_id()),
);
match entry_point_type {
EntryPointType::None => {
if let Some(span) = attr_span_by_symbol(ctxt, id, sym::unix_sigpipe) {
ctxt.tcx.dcx().emit_err(AttrOnlyOnMain { span, attr: sym::unix_sigpipe });
}
}
EntryPointType::None => (),
_ if !matches!(ctxt.tcx.def_kind(id.owner_id), DefKind::Fn) => {
for attr in [sym::start, sym::rustc_main] {
if let Some(span) = attr_span_by_symbol(ctxt, id, attr) {
@@ -81,9 +76,6 @@ fn find_item(id: ItemId, ctxt: &mut EntryContext<'_>) {
}
EntryPointType::MainNamed => (),
EntryPointType::OtherMain => {
if let Some(span) = attr_span_by_symbol(ctxt, id, sym::unix_sigpipe) {
ctxt.tcx.dcx().emit_err(AttrOnlyOnRootMain { span, attr: sym::unix_sigpipe });
}
ctxt.non_main_fns.push(ctxt.tcx.def_span(id.owner_id));
}
EntryPointType::RustcMainAttr => {
@@ -98,9 +90,6 @@ fn find_item(id: ItemId, ctxt: &mut EntryContext<'_>) {
}
}
EntryPointType::Start => {
if let Some(span) = attr_span_by_symbol(ctxt, id, sym::unix_sigpipe) {
ctxt.tcx.dcx().emit_err(AttrOnlyOnMain { span, attr: sym::unix_sigpipe });
}
if ctxt.start_fn.is_none() {
ctxt.start_fn = Some((id.owner_id.def_id, ctxt.tcx.def_span(id.owner_id)));
} else {
@@ -120,7 +109,7 @@ fn configure_main(tcx: TyCtxt<'_>, visitor: &EntryContext<'_>) -> Option<(DefId,
Some((def_id.to_def_id(), EntryFnType::Start))
} else if let Some((local_def_id, _)) = visitor.attr_main_fn {
let def_id = local_def_id.to_def_id();
Some((def_id, EntryFnType::Main { sigpipe: sigpipe(tcx, def_id) }))
Some((def_id, EntryFnType::Main { sigpipe: sigpipe(tcx) }))
} else {
if let Some(main_def) = tcx.resolutions(()).main_def
&& let Some(def_id) = main_def.opt_fn_def_id()
@@ -133,31 +122,19 @@ fn configure_main(tcx: TyCtxt<'_>, visitor: &EntryContext<'_>) -> Option<(DefId,
return None;
}

return Some((def_id, EntryFnType::Main { sigpipe: sigpipe(tcx, def_id) }));
return Some((def_id, EntryFnType::Main { sigpipe: sigpipe(tcx) }));
}
no_main_err(tcx, visitor);
None
}
}

fn sigpipe(tcx: TyCtxt<'_>, def_id: DefId) -> u8 {
if let Some(attr) = tcx.get_attr(def_id, sym::unix_sigpipe) {
match (attr.value_str(), attr.meta_item_list()) {
(Some(sym::inherit), None) => sigpipe::INHERIT,
(Some(sym::sig_ign), None) => sigpipe::SIG_IGN,
(Some(sym::sig_dfl), None) => sigpipe::SIG_DFL,
(Some(_), None) => {
tcx.dcx().emit_err(UnixSigpipeValues { span: attr.span });
sigpipe::DEFAULT
}
_ => {
// Keep going so that `fn emit_malformed_attribute()` can print
// an excellent error message
sigpipe::DEFAULT
}
}
} else {
sigpipe::DEFAULT
fn sigpipe(tcx: TyCtxt<'_>) -> u8 {
match tcx.sess.opts.unstable_opts.on_broken_pipe {
rustc_target::spec::OnBrokenPipe::Default => sigpipe::DEFAULT,
rustc_target::spec::OnBrokenPipe::Kill => sigpipe::SIG_DFL,
rustc_target::spec::OnBrokenPipe::Error => sigpipe::SIG_IGN,
rustc_target::spec::OnBrokenPipe::Inherit => sigpipe::INHERIT,
}
}

7 changes: 0 additions & 7 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1259,13 +1259,6 @@ pub struct ExternMain {
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(passes_unix_sigpipe_values)]
pub struct UnixSigpipeValues {
#[primary_span]
pub span: Span,
}

pub struct NoMainErr {
pub sp: Span,
pub crate_name: Symbol,
5 changes: 4 additions & 1 deletion compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
@@ -2891,7 +2891,9 @@ pub(crate) mod dep_tracking {
use rustc_feature::UnstableFeatures;
use rustc_span::edition::Edition;
use rustc_span::RealFileName;
use rustc_target::spec::{CodeModel, MergeFunctions, PanicStrategy, RelocModel, WasmCAbi};
use rustc_target::spec::{
CodeModel, MergeFunctions, OnBrokenPipe, PanicStrategy, RelocModel, WasmCAbi,
};
use rustc_target::spec::{
RelroLevel, SanitizerSet, SplitDebuginfo, StackProtector, TargetTriple, TlsModel,
};
@@ -2955,6 +2957,7 @@ pub(crate) mod dep_tracking {
InstrumentXRay,
CrateType,
MergeFunctions,
OnBrokenPipe,
PanicStrategy,
RelroLevel,
OptLevel,
2 changes: 1 addition & 1 deletion compiler/rustc_session/src/config/sigpipe.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! NOTE: Keep these constants in sync with `library/std/src/sys/pal/unix/mod.rs`!

/// The default value if `#[unix_sigpipe]` is not specified. This resolves
/// The default value if `-Zon-broken-pipe=...` is not specified. This resolves
/// to `SIG_IGN` in `library/std/src/sys/pal/unix/mod.rs`.
///
/// Note that `SIG_IGN` has been the Rust default since 2014. See
16 changes: 15 additions & 1 deletion compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ use rustc_span::edition::Edition;
use rustc_span::RealFileName;
use rustc_span::SourceFileHashAlgorithm;
use rustc_target::spec::{
CodeModel, LinkerFlavorCli, MergeFunctions, PanicStrategy, SanitizerSet, WasmCAbi,
CodeModel, LinkerFlavorCli, MergeFunctions, OnBrokenPipe, PanicStrategy, SanitizerSet, WasmCAbi,
};
use rustc_target::spec::{
RelocModel, RelroLevel, SplitDebuginfo, StackProtector, TargetTriple, TlsModel,
@@ -378,6 +378,7 @@ mod desc {
pub const parse_time_passes_format: &str = "`text` (default) or `json`";
pub const parse_passes: &str = "a space-separated list of passes, or `all`";
pub const parse_panic_strategy: &str = "either `unwind` or `abort`";
pub const parse_on_broken_pipe: &str = "either `kill`, `error`, or `inherit`";
pub const parse_opt_panic_strategy: &str = parse_panic_strategy;
pub const parse_oom_strategy: &str = "either `panic` or `abort`";
pub const parse_relro_level: &str = "one of: `full`, `partial`, or `off`";
@@ -708,6 +709,17 @@ mod parse {
true
}

pub(crate) fn parse_on_broken_pipe(slot: &mut OnBrokenPipe, v: Option<&str>) -> bool {
match v {
// OnBrokenPipe::Default can't be explicitly specified
Some("kill") => *slot = OnBrokenPipe::Kill,
Some("error") => *slot = OnBrokenPipe::Error,
Some("inherit") => *slot = OnBrokenPipe::Inherit,
_ => return false,
}
true
}

pub(crate) fn parse_oom_strategy(slot: &mut OomStrategy, v: Option<&str>) -> bool {
match v {
Some("panic") => *slot = OomStrategy::Panic,
@@ -1839,6 +1851,8 @@ options! {
"do not use unique names for text and data sections when -Z function-sections is used"),
normalize_docs: bool = (false, parse_bool, [TRACKED],
"normalize associated items in rustdoc when generating documentation"),
on_broken_pipe: OnBrokenPipe = (OnBrokenPipe::Default, parse_on_broken_pipe, [TRACKED],
"behavior of std::io::ErrorKind::BrokenPipe (SIGPIPE)"),
oom: OomStrategy = (OomStrategy::Abort, parse_oom_strategy, [TRACKED],
"panic strategy for out-of-memory handling"),
osx_rpath_install_name: bool = (false, parse_bool, [TRACKED],
1 change: 0 additions & 1 deletion compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
@@ -1935,7 +1935,6 @@ symbols! {
unit,
universal_impl_trait,
unix,
unix_sigpipe,
unlikely,
unmarked_api,
unnamed_fields,
8 changes: 8 additions & 0 deletions compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
@@ -778,6 +778,14 @@ pub enum PanicStrategy {
Abort,
}

#[derive(Clone, Copy, Debug, PartialEq, Hash, Encodable, Decodable, HashStable_Generic)]
pub enum OnBrokenPipe {
Default,
Kill,
Error,
Inherit,
}

impl PanicStrategy {
pub fn desc(&self) -> &str {
match *self {
2 changes: 1 addition & 1 deletion library/std/src/rt.rs
Original file line number Diff line number Diff line change
@@ -74,7 +74,7 @@ macro_rules! rtunwrap {
//
// Since 2014, the Rust runtime on Unix has set the `SIGPIPE` handler to
// `SIG_IGN`. Applications have good reasons to want a different behavior
// though, so there is a `#[unix_sigpipe = "..."]` attribute on `fn main()` that
// though, so there is a `-Zon-broken-pipe` compiler flag that
// can be used to select how `SIGPIPE` shall be setup (if changed at all) before
// `fn main()` is called. See <https://github.com/rust-lang/rust/issues/97889>
// for more info.
12 changes: 6 additions & 6 deletions library/std/src/sys/pal/unix/mod.rs
Original file line number Diff line number Diff line change
@@ -55,8 +55,8 @@ pub unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
// want!
//
// Hence, we set SIGPIPE to ignore when the program starts up in order
// to prevent this problem. Add `#[unix_sigpipe = "..."]` above `fn main()` to
// alter this behavior.
// to prevent this problem. Use `-Zon-broken-pipe=...` to alter this
// behavior.
reset_sigpipe(sigpipe);

stack_overflow::init();
@@ -194,7 +194,7 @@ pub unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
_ => unreachable!(),
};
if sigpipe_attr_specified {
UNIX_SIGPIPE_ATTR_SPECIFIED.store(true, crate::sync::atomic::Ordering::Relaxed);
ON_BROKEN_PIPE_FLAG_USED.store(true, crate::sync::atomic::Ordering::Relaxed);
}
if let Some(handler) = handler {
rtassert!(signal(libc::SIGPIPE, handler) != libc::SIG_ERR);
@@ -214,7 +214,7 @@ pub unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
target_os = "fuchsia",
target_os = "horizon",
)))]
static UNIX_SIGPIPE_ATTR_SPECIFIED: crate::sync::atomic::AtomicBool =
static ON_BROKEN_PIPE_FLAG_USED: crate::sync::atomic::AtomicBool =
crate::sync::atomic::AtomicBool::new(false);

#[cfg(not(any(
@@ -223,8 +223,8 @@ static UNIX_SIGPIPE_ATTR_SPECIFIED: crate::sync::atomic::AtomicBool =
target_os = "fuchsia",
target_os = "horizon",
)))]
pub(crate) fn unix_sigpipe_attr_specified() -> bool {
UNIX_SIGPIPE_ATTR_SPECIFIED.load(crate::sync::atomic::Ordering::Relaxed)
pub(crate) fn on_broken_pipe_flag_used() -> bool {
ON_BROKEN_PIPE_FLAG_USED.load(crate::sync::atomic::Ordering::Relaxed)
}

// SAFETY: must be called only once during runtime cleanup.
18 changes: 9 additions & 9 deletions library/std/src/sys/pal/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
@@ -353,11 +353,11 @@ impl Command {
// Inherit the signal mask from the parent rather than resetting it (i.e. do not call
// pthread_sigmask).

// If #[unix_sigpipe] is specified, don't reset SIGPIPE to SIG_DFL.
// If #[unix_sigpipe] is not specified, reset SIGPIPE to SIG_DFL for backward compatibility.
// If -Zon-broken-pipe is used, don't reset SIGPIPE to SIG_DFL.
// If -Zon-broken-pipe is not used, reset SIGPIPE to SIG_DFL for backward compatibility.
//
// #[unix_sigpipe] is an opportunity to change the default here.
if !crate::sys::pal::unix_sigpipe_attr_specified() {
// -Zon-broken-pipe is an opportunity to change the default here.
if !crate::sys::pal::on_broken_pipe_flag_used() {
#[cfg(target_os = "android")] // see issue #88585
{
let mut action: libc::sigaction = mem::zeroed();
@@ -455,7 +455,7 @@ impl Command {
) -> io::Result<Option<Process>> {
use crate::mem::MaybeUninit;
use crate::sys::weak::weak;
use crate::sys::{self, cvt_nz, unix_sigpipe_attr_specified};
use crate::sys::{self, cvt_nz, on_broken_pipe_flag_used};

if self.get_gid().is_some()
|| self.get_uid().is_some()
@@ -617,11 +617,11 @@ impl Command {
// Inherit the signal mask from this process rather than resetting it (i.e. do not call
// posix_spawnattr_setsigmask).

// If #[unix_sigpipe] is specified, don't reset SIGPIPE to SIG_DFL.
// If #[unix_sigpipe] is not specified, reset SIGPIPE to SIG_DFL for backward compatibility.
// If -Zon-broken-pipe is used, don't reset SIGPIPE to SIG_DFL.
// If -Zon-broken-pipe is not used, reset SIGPIPE to SIG_DFL for backward compatibility.
//
// #[unix_sigpipe] is an opportunity to change the default here.
if !unix_sigpipe_attr_specified() {
// -Zon-broken-pipe is an opportunity to change the default here.
if !on_broken_pipe_flag_used() {
let mut default_set = MaybeUninit::<libc::sigset_t>::uninit();
cvt(sigemptyset(default_set.as_mut_ptr()))?;
cvt(sigaddset(default_set.as_mut_ptr(), libc::SIGPIPE))?;
7 changes: 7 additions & 0 deletions src/bootstrap/src/core/build_steps/compile.rs
Original file line number Diff line number Diff line change
@@ -1006,6 +1006,13 @@ pub fn rustc_cargo(

cargo.rustdocflag("-Zcrate-attr=warn(rust_2018_idioms)");

// If the rustc output is piped to e.g. `head -n1` we want the process to be
// killed, rather than having an error bubble up and cause a panic.
// FIXME: Synthetic #[cfg(bootstrap)]. Remove when the bootstrap compiler supports it.
if compiler.stage != 0 {
cargo.rustflag("-Zon-broken-pipe=kill");
}

// We currently don't support cross-crate LTO in stage0. This also isn't hugely necessary
// and may just be a time sink.
if compiler.stage != 0 {
10 changes: 9 additions & 1 deletion src/bootstrap/src/core/build_steps/tool.rs
Original file line number Diff line number Diff line change
@@ -472,7 +472,7 @@ impl Step for Rustdoc {
features.push("jemalloc".to_string());
}

let cargo = prepare_tool_cargo(
let mut cargo = prepare_tool_cargo(
builder,
build_compiler,
Mode::ToolRustc,
@@ -483,6 +483,14 @@ impl Step for Rustdoc {
features.as_slice(),
);

// If the rustdoc output is piped to e.g. `head -n1` we want the process
// to be killed, rather than having an error bubble up and cause a
// panic.
// FIXME: Synthetic #[cfg(bootstrap)]. Remove when the bootstrap compiler supports it.
if build_compiler.stage > 0 {
cargo.rustflag("-Zon-broken-pipe=kill");
}
Copy link
Member

Choose a reason for hiding this comment

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

Turns out this regressed #123177.


let _guard = builder.msg_tool(
Kind::Build,
Mode::ToolRustc,
Loading
Loading