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

Support #[unix_sigpipe = "inherit|sig_dfl"] on fn main() to prevent ignoring SIGPIPE #97802

Merged
merged 5 commits into from
Sep 2, 2022
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
15 changes: 9 additions & 6 deletions compiler/rustc_codegen_cranelift/src/main_shim.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use rustc_hir::LangItem;
use rustc_middle::ty::subst::GenericArg;
use rustc_middle::ty::AssocKind;
use rustc_session::config::EntryFnType;
use rustc_session::config::{sigpipe, EntryFnType};
use rustc_span::symbol::Ident;

use crate::prelude::*;
Expand All @@ -15,12 +15,12 @@ pub(crate) fn maybe_create_entry_wrapper(
is_jit: bool,
is_primary_cgu: bool,
) {
let (main_def_id, is_main_fn) = match tcx.entry_fn(()) {
let (main_def_id, (is_main_fn, sigpipe)) = match tcx.entry_fn(()) {
Some((def_id, entry_ty)) => (
def_id,
match entry_ty {
EntryFnType::Main => true,
EntryFnType::Start => false,
EntryFnType::Main { sigpipe } => (true, sigpipe),
EntryFnType::Start => (false, sigpipe::DEFAULT),
},
),
None => return,
Expand All @@ -35,7 +35,7 @@ pub(crate) fn maybe_create_entry_wrapper(
return;
}

create_entry_fn(tcx, module, unwind_context, main_def_id, is_jit, is_main_fn);
create_entry_fn(tcx, module, unwind_context, main_def_id, is_jit, is_main_fn, sigpipe);

fn create_entry_fn(
tcx: TyCtxt<'_>,
Expand All @@ -44,6 +44,7 @@ pub(crate) fn maybe_create_entry_wrapper(
rust_main_def_id: DefId,
ignore_lang_start_wrapper: bool,
is_main_fn: bool,
sigpipe: u8,
) {
let main_ret_ty = tcx.fn_sig(rust_main_def_id).output();
// Given that `main()` has no arguments,
Expand Down Expand Up @@ -83,6 +84,7 @@ pub(crate) fn maybe_create_entry_wrapper(
bcx.switch_to_block(block);
let arg_argc = bcx.append_block_param(block, m.target_config().pointer_type());
let arg_argv = bcx.append_block_param(block, m.target_config().pointer_type());
let arg_sigpipe = bcx.ins().iconst(types::I8, sigpipe as i64);

let main_func_ref = m.declare_func_in_func(main_func_id, &mut bcx.func);

Expand Down Expand Up @@ -143,7 +145,8 @@ pub(crate) fn maybe_create_entry_wrapper(
let main_val = bcx.ins().func_addr(m.target_config().pointer_type(), main_func_ref);

let func_ref = m.declare_func_in_func(start_func_id, &mut bcx.func);
let call_inst = bcx.ins().call(func_ref, &[main_val, arg_argc, arg_argv]);
let call_inst =
bcx.ins().call(func_ref, &[main_val, arg_argc, arg_argv, arg_sigpipe]);
bcx.inst_results(call_inst)[0]
} else {
// using user-defined start fn
Expand Down
16 changes: 10 additions & 6 deletions compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,15 +389,14 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(

let main_llfn = cx.get_fn_addr(instance);

let use_start_lang_item = EntryFnType::Start != entry_type;
let entry_fn = create_entry_fn::<Bx>(cx, main_llfn, main_def_id, use_start_lang_item);
let entry_fn = create_entry_fn::<Bx>(cx, main_llfn, main_def_id, entry_type);
return Some(entry_fn);

fn create_entry_fn<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
cx: &'a Bx::CodegenCx,
rust_main: Bx::Value,
rust_main_def_id: DefId,
use_start_lang_item: bool,
entry_type: EntryFnType,
) -> Bx::Function {
// The entry function is either `int main(void)` or `int main(int argc, char **argv)`,
// depending on whether the target needs `argc` and `argv` to be passed in.
Expand Down Expand Up @@ -442,7 +441,7 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
let i8pp_ty = cx.type_ptr_to(cx.type_i8p());
let (arg_argc, arg_argv) = get_argc_argv(cx, &mut bx);

let (start_fn, start_ty, args) = if use_start_lang_item {
let (start_fn, start_ty, args) = if let EntryFnType::Main { sigpipe } = entry_type {
let start_def_id = cx.tcx().require_lang_item(LangItem::Start, None);
let start_fn = cx.get_fn_addr(
ty::Instance::resolve(
Expand All @@ -454,8 +453,13 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
.unwrap()
.unwrap(),
);
let start_ty = cx.type_func(&[cx.val_ty(rust_main), isize_ty, i8pp_ty], isize_ty);
(start_fn, start_ty, vec![rust_main, arg_argc, arg_argv])

let i8_ty = cx.type_i8();
let arg_sigpipe = bx.const_u8(sigpipe);

let start_ty =
cx.type_func(&[cx.val_ty(rust_main), isize_ty, i8pp_ty, i8_ty], isize_ty);
(start_fn, start_ty, vec![rust_main, arg_argc, arg_argv, arg_sigpipe])
} else {
debug!("using user-defined start fn");
let start_ty = cx.type_func(&[isize_ty, i8pp_ty], isize_ty);
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,8 @@ 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)
(active, type_changing_struct_update, "1.58.0", Some(86555), None),
/// Enables rustc to generate code that instructs libstd to NOT ignore SIGPIPE.
(active, unix_sigpipe, "CURRENT_RUSTC_VERSION", Some(97889), None),
/// Allows unsized fn parameters.
(active, unsized_fn_params, "1.49.0", Some(48055), None),
/// Allows unsized rvalues at arguments and parameters.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
),

// Entry point:
gated!(unix_sigpipe, Normal, template!(Word, NameValueStr: "inherit|sig_ign|sig_dfl"), ErrorFollowing, experimental!(unix_sigpipe)),
ungated!(start, Normal, template!(Word), WarnFollowing),
ungated!(no_start, CrateLevel, template!(Word), WarnFollowing),
ungated!(no_main, CrateLevel, template!(Word), WarnFollowing),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1315,7 +1315,7 @@ impl<'v> RootCollector<'_, 'v> {
/// the return type of `main`. This is not needed when
/// the user writes their own `start` manually.
fn push_extra_entry_roots(&mut self) {
let Some((main_def_id, EntryFnType::Main)) = self.entry_fn else {
let Some((main_def_id, EntryFnType::Main { .. })) = self.entry_fn else {
return;
};

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2117,6 +2117,7 @@ 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
56 changes: 42 additions & 14 deletions compiler/rustc_passes/src/entry.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use rustc_ast::{entry::EntryPointType, Attribute};
use rustc_ast::entry::EntryPointType;
use rustc_errors::struct_span_err;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID, LOCAL_CRATE};
use rustc_hir::{ItemId, Node, CRATE_HIR_ID};
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::{DefIdTree, TyCtxt};
use rustc_session::config::{CrateType, EntryFnType};
use rustc_session::config::{sigpipe, CrateType, EntryFnType};
use rustc_session::parse::feature_err;
use rustc_span::symbol::sym;
use rustc_span::{Span, Symbol, DUMMY_SP};
Expand Down Expand Up @@ -71,14 +71,12 @@ fn entry_point_type(ctxt: &EntryContext<'_>, id: ItemId, at_root: bool) -> Entry
}
}

fn err_if_attr_found(ctxt: &EntryContext<'_>, attrs: &[Attribute], sym: Symbol) {
fn err_if_attr_found(ctxt: &EntryContext<'_>, id: ItemId, sym: Symbol, details: &str) {
let attrs = ctxt.tcx.hir().attrs(id.hir_id());
if let Some(attr) = ctxt.tcx.sess.find_by_name(attrs, sym) {
ctxt.tcx
.sess
.struct_span_err(
attr.span,
&format!("`{}` attribute can only be used on functions", sym),
)
.struct_span_err(attr.span, &format!("`{}` attribute {}", sym, details))
.emit();
}
}
Expand All @@ -87,14 +85,16 @@ fn find_item(id: ItemId, ctxt: &mut EntryContext<'_>) {
let at_root = ctxt.tcx.opt_local_parent(id.def_id) == Some(CRATE_DEF_ID);

match entry_point_type(ctxt, id, at_root) {
EntryPointType::None => (),
EntryPointType::None => {
err_if_attr_found(ctxt, id, sym::unix_sigpipe, "can only be used on `fn main()`");
}
_ if !matches!(ctxt.tcx.def_kind(id.def_id), DefKind::Fn) => {
let attrs = ctxt.tcx.hir().attrs(id.hir_id());
err_if_attr_found(ctxt, attrs, sym::start);
err_if_attr_found(ctxt, attrs, sym::rustc_main);
err_if_attr_found(ctxt, id, sym::start, "can only be used on functions");
err_if_attr_found(ctxt, id, sym::rustc_main, "can only be used on functions");
}
EntryPointType::MainNamed => (),
EntryPointType::OtherMain => {
err_if_attr_found(ctxt, id, sym::unix_sigpipe, "can only be used on root `fn main()`");
ctxt.non_main_fns.push(ctxt.tcx.def_span(id.def_id));
}
EntryPointType::RustcMainAttr => {
Expand All @@ -116,6 +116,7 @@ fn find_item(id: ItemId, ctxt: &mut EntryContext<'_>) {
}
}
EntryPointType::Start => {
err_if_attr_found(ctxt, id, sym::unix_sigpipe, "can only be used on `fn main()`");
if ctxt.start_fn.is_none() {
ctxt.start_fn = Some((id.def_id, ctxt.tcx.def_span(id.def_id.to_def_id())));
} else {
Expand All @@ -136,8 +137,9 @@ fn find_item(id: ItemId, ctxt: &mut EntryContext<'_>) {
fn configure_main(tcx: TyCtxt<'_>, visitor: &EntryContext<'_>) -> Option<(DefId, EntryFnType)> {
if let Some((def_id, _)) = visitor.start_fn {
Some((def_id.to_def_id(), EntryFnType::Start))
} else if let Some((def_id, _)) = visitor.attr_main_fn {
Some((def_id.to_def_id(), EntryFnType::Main))
} 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) }))
} else {
if let Some(main_def) = tcx.resolutions(()).main_def && let Some(def_id) = main_def.opt_fn_def_id() {
// non-local main imports are handled below
Expand All @@ -161,13 +163,39 @@ fn configure_main(tcx: TyCtxt<'_>, visitor: &EntryContext<'_>) -> Option<(DefId,
)
.emit();
}
return Some((def_id, EntryFnType::Main));
return Some((def_id, EntryFnType::Main { sigpipe: sigpipe(tcx, def_id) }));
}
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(_)) => {
// Keep going so that `fn emit_malformed_attribute()` can print
// an excellent error message
sigpipe::DEFAULT
}
_ => {
tcx.sess
.struct_span_err(
attr.span,
"valid values for `#[unix_sigpipe = \"...\"]` are `inherit`, `sig_ign`, or `sig_dfl`",
)
.emit();
sigpipe::DEFAULT
}
}
} else {
sigpipe::DEFAULT
}
}

fn no_main_err(tcx: TyCtxt<'_>, visitor: &EntryContext<'_>) {
let sp = tcx.def_span(CRATE_DEF_ID);
if *tcx.sess.parse_sess.reached_eof.borrow() {
Expand Down
12 changes: 11 additions & 1 deletion compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ use std::iter::{self, FromIterator};
use std::path::{Path, PathBuf};
use std::str::{self, FromStr};

pub mod sigpipe;

/// The different settings that the `-C strip` flag can have.
#[derive(Clone, Copy, PartialEq, Hash, Debug)]
pub enum Strip {
Expand Down Expand Up @@ -798,7 +800,15 @@ impl UnstableOptions {
// The type of entry function, so users can have their own entry functions
#[derive(Copy, Clone, PartialEq, Hash, Debug, HashStable_Generic)]
pub enum EntryFnType {
Main,
Main {
/// Specifies what to do with `SIGPIPE` before calling `fn main()`.
///
/// What values that are valid and what they mean must be in sync
/// across rustc and libstd, but we don't want it public in libstd,
/// so we take a bit of an unusual approach with simple constants
/// and an `include!()`.
sigpipe: u8,
},
Start,
}

Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_session/src/config/sigpipe.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
//! NOTE: Keep these constants in sync with `library/std/src/sys/unix/mod.rs`!

/// Do not touch `SIGPIPE`. Use whatever the parent process uses.
#[allow(dead_code)]
pub const INHERIT: u8 = 1;

/// Change `SIGPIPE` to `SIG_IGN` so that failed writes results in `EPIPE`
/// that are eventually converted to `ErrorKind::BrokenPipe`.
#[allow(dead_code)]
pub const SIG_IGN: u8 = 2;

/// Change `SIGPIPE` to `SIG_DFL` so that the process is killed when trying
/// to write to a closed pipe. This is usually the desired behavior for CLI
/// apps that produce textual output that you want to pipe to other programs
/// such as `head -n 1`.
#[allow(dead_code)]
pub const SIG_DFL: u8 = 3;

/// `SIG_IGN` has been the Rust default since 2014. See
/// <https://github.com/rust-lang/rust/issues/62569>.
#[allow(dead_code)]
pub const DEFAULT: u8 = SIG_IGN;
4 changes: 4 additions & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -819,6 +819,7 @@ symbols! {
infer_outlives_requirements,
infer_static_outlives_requirements,
inherent_associated_types,
inherit,
inlateout,
inline,
inline_const,
Expand Down Expand Up @@ -1301,6 +1302,8 @@ symbols! {
should_panic,
shr,
shr_assign,
sig_dfl,
sig_ign,
simd,
simd_add,
simd_and,
Expand Down Expand Up @@ -1519,6 +1522,7 @@ symbols! {
unit,
universal_impl_trait,
unix,
unix_sigpipe,
unlikely,
unmarked_api,
unpin,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ fn check_start_fn_ty(tcx: TyCtxt<'_>, start_def_id: DefId) {

fn check_for_entry_fn(tcx: TyCtxt<'_>) {
match tcx.entry_fn(()) {
Some((def_id, EntryFnType::Main)) => check_main_fn_ty(tcx, def_id),
Some((def_id, EntryFnType::Main { .. })) => check_main_fn_ty(tcx, def_id),
Some((def_id, EntryFnType::Start)) => check_start_fn_ty(tcx, def_id),
_ => {}
}
Expand Down
31 changes: 28 additions & 3 deletions library/std/src/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,29 @@ macro_rules! rtunwrap {
// Runs before `main`.
// SAFETY: must be called only once during runtime initialization.
// NOTE: this is not guaranteed to run, for example when Rust code is called externally.
//
// # The `sigpipe` parameter
//
// 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
// 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.
//
// The `sigpipe` parameter to this function gets its value via the code that
// rustc generates to invoke `fn lang_start()`. The reason we have `sigpipe` for
// all platforms and not only Unix, is because std is not allowed to have `cfg`
// directives as this high level. See the module docs in
// `src/tools/tidy/src/pal.rs` for more info. On all other platforms, `sigpipe`
// has a value, but its value is ignored.
//
// Even though it is an `u8`, it only ever has 3 values. These are documented in
// `compiler/rustc_session/src/config/sigpipe.rs`.
#[cfg_attr(test, allow(dead_code))]
unsafe fn init(argc: isize, argv: *const *const u8) {
unsafe fn init(argc: isize, argv: *const *const u8, sigpipe: u8) {
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to have a comment here explaining the new argument (since unlike the other two it is not standard).

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

Copy link
Member

Choose a reason for hiding this comment

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

When just reading this file that comment remains rather mysterious though -- there are 256 possible values for sigpipe and it is unclear what they would all mean.

Looks like the concrete meaning is platform-dependent? That should be said, maybe with a reference to the file where unix defines its meanings.

Copy link
Member

Choose a reason for hiding this comment

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

Ah no actually compiler/rustc_session/src/config/sigpipe.rs is not target-dependent. So that should probably be referenced.

But then I don't understand why the std version of that file is inside unix.

Copy link
Member Author

Choose a reason for hiding this comment

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

@RalfJung If you would like to have more of a "full context" kind of doc at this location, that's perfectly fine with me. Would you mind us iterating the exact contents of that comment via GitHub comments rather than commits though?

Here is my proposal on what we could write here:

// # The `sigpipe` parameter
//
// 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
// 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.
//
// The `sigpipe` parameter to this function gets its value via the code that
// rustc generates to invoke `fn lang_start()`. The reason we have `sigpipe` for
// all platforms and not only Unix, is because std is not allowed to have `cfg`
// directives as this high level. See the module docs in
// `src/tools/tidy/src/pal.rs` for more info. On all other platforms, `sigpipe`
// has a value, but its value is ignored.
//
// Even though it is an `u8`, it only ever has 3 values. These are documented in
// `compiler/rustc_session/src/config/sigpipe.rs`.

I propose that we do not duplicate this comment also in library/std/src/sys/unix/mod.rs though, i.e. that we only put that comment at the location you made this comment.

Looking forward to your response.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that sounds good! I don't have a strong preference whether library/std/src/rt.rs points to library/std/src/sys/unix/mod.rs or vice versa.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great! Commit pushed.

unsafe {
sys::init(argc, argv);
sys::init(argc, argv, sigpipe);

let main_guard = sys::thread::guard::init();
// Next, set up the current Thread with the guard information we just
Expand Down Expand Up @@ -107,6 +126,7 @@ fn lang_start_internal(
main: &(dyn Fn() -> i32 + Sync + crate::panic::RefUnwindSafe),
argc: isize,
argv: *const *const u8,
sigpipe: u8,
) -> Result<isize, !> {
use crate::{mem, panic};
let rt_abort = move |e| {
Expand All @@ -124,7 +144,7 @@ fn lang_start_internal(
// prevent libstd from accidentally introducing a panic to these functions. Another is from
// user code from `main` or, more nefariously, as described in e.g. issue #86030.
// SAFETY: Only called once during runtime initialization.
panic::catch_unwind(move || unsafe { init(argc, argv) }).map_err(rt_abort)?;
panic::catch_unwind(move || unsafe { init(argc, argv, sigpipe) }).map_err(rt_abort)?;
let ret_code = panic::catch_unwind(move || panic::catch_unwind(main).unwrap_or(101) as isize)
.map_err(move |e| {
mem::forget(e);
Expand All @@ -140,11 +160,16 @@ fn lang_start<T: crate::process::Termination + 'static>(
main: fn() -> T,
argc: isize,
argv: *const *const u8,
#[cfg(not(bootstrap))] sigpipe: u8,
) -> isize {
let Ok(v) = lang_start_internal(
&move || crate::sys_common::backtrace::__rust_begin_short_backtrace(main).report().to_i32(),
argc,
argv,
#[cfg(bootstrap)]
2, // Temporary inlining of sigpipe::DEFAULT until bootstrap stops being special
#[cfg(not(bootstrap))]
sigpipe,
);
v
}
Loading