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

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

Merged
merged 1 commit into from
May 4, 2024
Merged
Show file tree
Hide file tree
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
Expand Down Expand Up @@ -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")]
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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}`
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
43 changes: 10 additions & 33 deletions compiler/rustc_passes/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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) {
Expand All @@ -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 => {
Expand All @@ -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 {
Expand All @@ -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()
Expand All @@ -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,
}
}

Expand Down
7 changes: 0 additions & 7 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -2955,6 +2957,7 @@ pub(crate) mod dep_tracking {
InstrumentXRay,
CrateType,
MergeFunctions,
OnBrokenPipe,
PanicStrategy,
RelroLevel,
OptLevel,
Expand Down
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
Expand Down
16 changes: 15 additions & 1 deletion compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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`";
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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],
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1935,7 +1935,6 @@ symbols! {
unit,
universal_impl_trait,
unix,
unix_sigpipe,
unlikely,
unmarked_api,
unnamed_fields,
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 6 additions & 6 deletions library/std/src/sys/pal/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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(
Expand All @@ -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.
Expand Down
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
Expand Up @@ -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();
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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))?;
Expand Down
7 changes: 7 additions & 0 deletions src/bootstrap/src/core/build_steps/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 9 additions & 1 deletion src/bootstrap/src/core/build_steps/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
Loading
Loading