From 0d508bb0cd681bf15f7861670cd38a1b352d2e40 Mon Sep 17 00:00:00 2001 From: Mohammad Omidvar Date: Mon, 15 Jul 2024 19:54:47 +0000 Subject: [PATCH 01/29] Introduce and provide a hook for `should_codegen_locally` --- compiler/rustc_middle/src/hooks/mod.rs | 4 ++++ compiler/rustc_monomorphize/src/collector.rs | 15 +++++++++++++-- compiler/rustc_monomorphize/src/lib.rs | 3 ++- compiler/rustc_monomorphize/src/partitioning.rs | 4 +++- 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_middle/src/hooks/mod.rs b/compiler/rustc_middle/src/hooks/mod.rs index bf10a71dbaec1..75dc685a16aee 100644 --- a/compiler/rustc_middle/src/hooks/mod.rs +++ b/compiler/rustc_middle/src/hooks/mod.rs @@ -103,6 +103,10 @@ declare_hooks! { /// Create a list-like THIR representation for debugging. hook thir_flat(key: LocalDefId) -> String; + + /// Returns `true` if we should codegen an instance in the local crate, or returns `false` if we + /// can just link to the upstream crate and therefore don't need a mono item. + hook should_codegen_locally(instance: crate::ty::Instance<'tcx>) -> bool; } #[cold] diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index bfd505c06726d..27d27885ea8d8 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -228,6 +228,7 @@ use rustc_middle::ty::{ self, AssocKind, GenericParamDefKind, Instance, InstanceKind, Ty, TyCtxt, TypeFoldable, TypeVisitableExt, VtblEntry, }; +use rustc_middle::util::Providers; use rustc_middle::{bug, span_bug}; use rustc_session::config::EntryFnType; use rustc_session::Limit; @@ -930,7 +931,7 @@ fn visit_instance_use<'tcx>( /// Returns `true` if we should codegen an instance in the local crate, or returns `false` if we /// can just link to the upstream crate and therefore don't need a mono item. -pub(crate) fn should_codegen_locally<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> bool { +pub(crate) fn should_codegen_locally_hook<'tcx>(tcx: TyCtxtAt<'tcx>, instance: Instance<'tcx>) -> bool { let Some(def_id) = instance.def.def_id_if_not_guaranteed_local_codegen() else { return true; }; @@ -946,7 +947,7 @@ pub(crate) fn should_codegen_locally<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance } if tcx.is_reachable_non_generic(def_id) - || instance.polymorphize(tcx).upstream_monomorphization(tcx).is_some() + || instance.polymorphize(*tcx).upstream_monomorphization(*tcx).is_some() { // We can link to the item in question, no instance needed in this crate. return false; @@ -967,6 +968,12 @@ pub(crate) fn should_codegen_locally<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance true } +/// Returns `true` if we should codegen an instance in the local crate, or returns `false` if we +/// can just link to the upstream crate and therefore don't need a mono item. +pub(crate) fn should_codegen_locally<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> bool { + tcx.should_codegen_locally(instance) +} + /// For a given pair of source and target type that occur in an unsizing coercion, /// this function finds the pair of types that determines the vtable linking /// them. @@ -1613,3 +1620,7 @@ pub(crate) fn collect_crate_mono_items<'tcx>( (mono_items, state.usage_map.into_inner()) } + +pub fn provide(providers: &mut Providers) { + providers.hooks.should_codegen_locally = should_codegen_locally_hook; +} diff --git a/compiler/rustc_monomorphize/src/lib.rs b/compiler/rustc_monomorphize/src/lib.rs index aa3b4cd5b6782..d169f08606636 100644 --- a/compiler/rustc_monomorphize/src/lib.rs +++ b/compiler/rustc_monomorphize/src/lib.rs @@ -5,12 +5,13 @@ use rustc_hir::lang_items::LangItem; use rustc_middle::bug; -use rustc_middle::query::{Providers, TyCtxtAt}; +use rustc_middle::query::TyCtxtAt; use rustc_middle::traits; use rustc_middle::ty::adjustment::CustomCoerceUnsized; use rustc_middle::ty::Instance; use rustc_middle::ty::TyCtxt; use rustc_middle::ty::{self, Ty}; +use rustc_middle::util::Providers; use rustc_span::def_id::DefId; use rustc_span::def_id::LOCAL_CRATE; use rustc_span::ErrorGuaranteed; diff --git a/compiler/rustc_monomorphize/src/partitioning.rs b/compiler/rustc_monomorphize/src/partitioning.rs index 9a7c488833a1a..8c7c5e0074abf 100644 --- a/compiler/rustc_monomorphize/src/partitioning.rs +++ b/compiler/rustc_monomorphize/src/partitioning.rs @@ -112,9 +112,9 @@ use rustc_middle::mir::mono::{ CodegenUnit, CodegenUnitNameBuilder, InstantiationMode, Linkage, MonoItem, MonoItemData, Visibility, }; -use rustc_middle::query::Providers; use rustc_middle::ty::print::{characteristic_def_id_of_type, with_no_trimmed_paths}; use rustc_middle::ty::{self, visit::TypeVisitableExt, InstanceKind, TyCtxt}; +use rustc_middle::util::Providers; use rustc_session::config::{DumpMonoStatsFormat, SwitchWithOptPath}; use rustc_session::CodegenUnits; use rustc_span::symbol::Symbol; @@ -1314,4 +1314,6 @@ pub fn provide(providers: &mut Providers) { .find(|cgu| cgu.name() == name) .unwrap_or_else(|| panic!("failed to find cgu with name {name:?}")) }; + + collector::provide(providers); } From 14430e66be02266ca59bdddd38d88a27f2e02917 Mon Sep 17 00:00:00 2001 From: Mohammad Omidvar Date: Mon, 15 Jul 2024 19:58:44 +0000 Subject: [PATCH 02/29] Use the hook on tcx instead of the local function --- compiler/rustc_monomorphize/src/collector.rs | 40 +++++++++----------- compiler/rustc_monomorphize/src/lib.rs | 4 +- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 27d27885ea8d8..3655a677ba0ad 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -400,7 +400,7 @@ fn collect_items_rec<'tcx>( let instance = Instance::mono(tcx, def_id); // Sanity check whether this ended up being collected accidentally - debug_assert!(should_codegen_locally(tcx, instance)); + debug_assert!(tcx.should_codegen_locally(instance)); let DefKind::Static { nested, .. } = tcx.def_kind(def_id) else { bug!() }; // Nested statics have no type. @@ -432,7 +432,7 @@ fn collect_items_rec<'tcx>( } MonoItem::Fn(instance) => { // Sanity check whether this ended up being collected accidentally - debug_assert!(should_codegen_locally(tcx, instance)); + debug_assert!(tcx.should_codegen_locally(instance)); // Keep track of the monomorphization recursion depth recursion_depth_reset = Some(check_recursion_limit( @@ -476,7 +476,7 @@ fn collect_items_rec<'tcx>( } hir::InlineAsmOperand::SymStatic { path: _, def_id } => { let instance = Instance::mono(tcx, *def_id); - if should_codegen_locally(tcx, instance) { + if tcx.should_codegen_locally(instance) { trace!("collecting static {:?}", def_id); used_items.push(dummy_spanned(MonoItem::Static(*def_id))); } @@ -713,7 +713,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { if let ty::Closure(def_id, args) = *source_ty.kind() { let instance = Instance::resolve_closure(self.tcx, def_id, args, ty::ClosureKind::FnOnce); - if should_codegen_locally(self.tcx, instance) { + if self.tcx.should_codegen_locally(instance) { self.used_items.push(create_fn_mono_item(self.tcx, instance, span)); } } else { @@ -723,7 +723,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { mir::Rvalue::ThreadLocalRef(def_id) => { assert!(self.tcx.is_thread_local_static(def_id)); let instance = Instance::mono(self.tcx, def_id); - if should_codegen_locally(self.tcx, instance) { + if self.tcx.should_codegen_locally(instance) { trace!("collecting thread-local static {:?}", def_id); self.used_items.push(respan(span, MonoItem::Static(def_id))); } @@ -750,7 +750,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { let tcx = self.tcx; let push_mono_lang_item = |this: &mut Self, lang_item: LangItem| { let instance = Instance::mono(tcx, tcx.require_lang_item(lang_item, Some(source))); - if should_codegen_locally(tcx, instance) { + if tcx.should_codegen_locally(instance) { this.used_items.push(create_fn_mono_item(tcx, instance, source)); } }; @@ -784,7 +784,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { } mir::InlineAsmOperand::SymStatic { def_id } => { let instance = Instance::mono(self.tcx, def_id); - if should_codegen_locally(self.tcx, instance) { + if self.tcx.should_codegen_locally(instance) { trace!("collecting asm sym static {:?}", def_id); self.used_items.push(respan(source, MonoItem::Static(def_id))); } @@ -874,7 +874,7 @@ fn visit_instance_use<'tcx>( output: &mut MonoItems<'tcx>, ) { debug!("visit_item_use({:?}, is_direct_call={:?})", instance, is_direct_call); - if !should_codegen_locally(tcx, instance) { + if !tcx.should_codegen_locally(instance) { return; } if let ty::InstanceKind::Intrinsic(def_id) = instance.def { @@ -886,13 +886,13 @@ fn visit_instance_use<'tcx>( // codegen a call to that function without generating code for the function itself. let def_id = tcx.require_lang_item(LangItem::PanicNounwind, None); let panic_instance = Instance::mono(tcx, def_id); - if should_codegen_locally(tcx, panic_instance) { + if tcx.should_codegen_locally(panic_instance) { output.push(create_fn_mono_item(tcx, panic_instance, source)); } } else if tcx.has_attr(def_id, sym::rustc_intrinsic) { // Codegen the fallback body of intrinsics with fallback bodies let instance = ty::Instance::new(def_id, instance.args); - if should_codegen_locally(tcx, instance) { + if tcx.should_codegen_locally(instance) { output.push(create_fn_mono_item(tcx, instance, source)); } } @@ -931,7 +931,7 @@ fn visit_instance_use<'tcx>( /// Returns `true` if we should codegen an instance in the local crate, or returns `false` if we /// can just link to the upstream crate and therefore don't need a mono item. -pub(crate) fn should_codegen_locally_hook<'tcx>(tcx: TyCtxtAt<'tcx>, instance: Instance<'tcx>) -> bool { +fn should_codegen_locally<'tcx>(tcx: TyCtxtAt<'tcx>, instance: Instance<'tcx>) -> bool { let Some(def_id) = instance.def.def_id_if_not_guaranteed_local_codegen() else { return true; }; @@ -968,12 +968,6 @@ pub(crate) fn should_codegen_locally_hook<'tcx>(tcx: TyCtxtAt<'tcx>, instance: I true } -/// Returns `true` if we should codegen an instance in the local crate, or returns `false` if we -/// can just link to the upstream crate and therefore don't need a mono item. -pub(crate) fn should_codegen_locally<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> bool { - tcx.should_codegen_locally(instance) -} - /// For a given pair of source and target type that occur in an unsizing coercion, /// this function finds the pair of types that determines the vtable linking /// them. @@ -1134,7 +1128,7 @@ fn create_mono_items_for_vtable_methods<'tcx>( None } VtblEntry::Method(instance) => { - Some(*instance).filter(|instance| should_codegen_locally(tcx, *instance)) + Some(*instance).filter(|instance| tcx.should_codegen_locally(*instance)) } }) .map(|item| create_fn_mono_item(tcx, item, source)); @@ -1151,7 +1145,7 @@ fn collect_alloc<'tcx>(tcx: TyCtxt<'tcx>, alloc_id: AllocId, output: &mut MonoIt GlobalAlloc::Static(def_id) => { assert!(!tcx.is_thread_local_static(def_id)); let instance = Instance::mono(tcx, def_id); - if should_codegen_locally(tcx, instance) { + if tcx.should_codegen_locally(instance) { trace!("collecting static {:?}", def_id); output.push(dummy_spanned(MonoItem::Static(def_id))); } @@ -1169,7 +1163,7 @@ fn collect_alloc<'tcx>(tcx: TyCtxt<'tcx>, alloc_id: AllocId, output: &mut MonoIt } } GlobalAlloc::Function { instance, .. } => { - if should_codegen_locally(tcx, instance) { + if tcx.should_codegen_locally(instance) { trace!("collecting {:?} with {:#?}", alloc_id, instance); output.push(create_fn_mono_item(tcx, instance, DUMMY_SP)); } @@ -1291,7 +1285,7 @@ fn visit_mentioned_item<'tcx>( if let ty::Closure(def_id, args) = *source_ty.kind() { let instance = Instance::resolve_closure(tcx, def_id, args, ty::ClosureKind::FnOnce); - if should_codegen_locally(tcx, instance) { + if tcx.should_codegen_locally(instance) { output.push(create_fn_mono_item(tcx, instance, span)); } } else { @@ -1564,7 +1558,7 @@ fn create_mono_items_for_default_impls<'tcx>( let instance = ty::Instance::expect_resolve(tcx, param_env, method.def_id, args, DUMMY_SP); let mono_item = create_fn_mono_item(tcx, instance, DUMMY_SP); - if mono_item.node.is_instantiable(tcx) && should_codegen_locally(tcx, instance) { + if mono_item.node.is_instantiable(tcx) && tcx.should_codegen_locally(instance) { output.push(mono_item); } } @@ -1622,5 +1616,5 @@ pub(crate) fn collect_crate_mono_items<'tcx>( } pub fn provide(providers: &mut Providers) { - providers.hooks.should_codegen_locally = should_codegen_locally_hook; + providers.hooks.should_codegen_locally = should_codegen_locally; } diff --git a/compiler/rustc_monomorphize/src/lib.rs b/compiler/rustc_monomorphize/src/lib.rs index d169f08606636..e5a63a98ff47b 100644 --- a/compiler/rustc_monomorphize/src/lib.rs +++ b/compiler/rustc_monomorphize/src/lib.rs @@ -22,8 +22,6 @@ mod partitioning; mod polymorphize; mod util; -use collector::should_codegen_locally; - rustc_fluent_macro::fluent_messages! { "../messages.ftl" } fn custom_coerce_unsize_info<'tcx>( @@ -73,7 +71,7 @@ pub fn is_call_from_compiler_builtins_to_upstream_monomorphization<'tcx>( !def_id.is_local() && tcx.is_compiler_builtins(LOCAL_CRATE) && !is_llvm_intrinsic(tcx, def_id) - && !should_codegen_locally(tcx, instance) + && !tcx.should_codegen_locally(instance) } pub fn provide(providers: &mut Providers) { From 9b80250abb8ebf9f49f51593aee6a7308e0ac3b4 Mon Sep 17 00:00:00 2001 From: Mohammad Omidvar Date: Mon, 15 Jul 2024 23:43:52 +0000 Subject: [PATCH 03/29] Move compiler_builtin check to the use case --- .../rustc_codegen_cranelift/src/abi/mod.rs | 2 +- compiler/rustc_codegen_cranelift/src/base.rs | 2 +- compiler/rustc_codegen_cranelift/src/lib.rs | 1 - compiler/rustc_codegen_ssa/src/base.rs | 28 ++++++++++++++++ compiler/rustc_codegen_ssa/src/mir/block.rs | 3 +- compiler/rustc_monomorphize/src/lib.rs | 32 ------------------- 6 files changed, 31 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/abi/mod.rs b/compiler/rustc_codegen_cranelift/src/abi/mod.rs index fa0de6f9de5ea..698981ae1536d 100644 --- a/compiler/rustc_codegen_cranelift/src/abi/mod.rs +++ b/compiler/rustc_codegen_cranelift/src/abi/mod.rs @@ -10,12 +10,12 @@ use std::mem; use cranelift_codegen::ir::{ArgumentPurpose, SigRef}; use cranelift_codegen::isa::CallConv; use cranelift_module::ModuleError; +use rustc_codegen_ssa::base::is_call_from_compiler_builtins_to_upstream_monomorphization; use rustc_codegen_ssa::errors::CompilerBuiltinsCannotCall; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::ty::layout::FnAbiOf; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::TypeVisitableExt; -use rustc_monomorphize::is_call_from_compiler_builtins_to_upstream_monomorphization; use rustc_session::Session; use rustc_span::source_map::Spanned; use rustc_target::abi::call::{Conv, FnAbi, PassMode}; diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index 5adbbb09ac85f..9bc7b57c53745 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -5,13 +5,13 @@ use cranelift_codegen::CodegenError; use cranelift_frontend::{FunctionBuilder, FunctionBuilderContext}; use cranelift_module::ModuleError; use rustc_ast::InlineAsmOptions; +use rustc_codegen_ssa::base::is_call_from_compiler_builtins_to_upstream_monomorphization; use rustc_index::IndexVec; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::ty::adjustment::PointerCoercion; use rustc_middle::ty::layout::FnAbiOf; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::TypeVisitableExt; -use rustc_monomorphize::is_call_from_compiler_builtins_to_upstream_monomorphization; use crate::constant::ConstantCx; use crate::debuginfo::{FunctionDebugContext, TypeDebugContext}; diff --git a/compiler/rustc_codegen_cranelift/src/lib.rs b/compiler/rustc_codegen_cranelift/src/lib.rs index 192e6c91ea38b..8d3d5ac98e1e5 100644 --- a/compiler/rustc_codegen_cranelift/src/lib.rs +++ b/compiler/rustc_codegen_cranelift/src/lib.rs @@ -24,7 +24,6 @@ extern crate rustc_hir; extern crate rustc_incremental; extern crate rustc_index; extern crate rustc_metadata; -extern crate rustc_monomorphize; extern crate rustc_session; extern crate rustc_span; extern crate rustc_target; diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index 137f14fe706cc..399ac4858505e 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -806,6 +806,34 @@ pub fn codegen_crate( ongoing_codegen } +/// Returns whether a call from the current crate to the [`Instance`] would produce a call +/// from `compiler_builtins` to a symbol the linker must resolve. +/// +/// Such calls from `compiler_bultins` are effectively impossible for the linker to handle. Some +/// linkers will optimize such that dead calls to unresolved symbols are not an error, but this is +/// not guaranteed. So we used this function in codegen backends to ensure we do not generate any +/// unlinkable calls. +/// +/// Note that calls to LLVM intrinsics are uniquely okay because they won't make it to the linker. +pub fn is_call_from_compiler_builtins_to_upstream_monomorphization<'tcx>( + tcx: TyCtxt<'tcx>, + instance: Instance<'tcx>, +) -> bool { + fn is_llvm_intrinsic(tcx: TyCtxt<'_>, def_id: DefId) -> bool { + if let Some(name) = tcx.codegen_fn_attrs(def_id).link_name { + name.as_str().starts_with("llvm.") + } else { + false + } + } + + let def_id = instance.def_id(); + !def_id.is_local() + && tcx.is_compiler_builtins(LOCAL_CRATE) + && !is_llvm_intrinsic(tcx, def_id) + && !tcx.should_codegen_locally(instance) +} + impl CrateInfo { pub fn new(tcx: TyCtxt<'_>, target_cpu: String) -> CrateInfo { let crate_types = tcx.crate_types().to_vec(); diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 6a5525dc2b34f..c9c8f02c491bd 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -3,7 +3,7 @@ use super::operand::OperandValue::{Immediate, Pair, Ref, ZeroSized}; use super::place::{PlaceRef, PlaceValue}; use super::{CachedLlbb, FunctionCx, LocalRef}; -use crate::base; +use crate::base::{self, is_call_from_compiler_builtins_to_upstream_monomorphization}; use crate::common::{self, IntPredicate}; use crate::errors::CompilerBuiltinsCannotCall; use crate::meth; @@ -18,7 +18,6 @@ use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, ValidityRequirement}; use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths}; use rustc_middle::ty::{self, Instance, Ty}; use rustc_middle::{bug, span_bug}; -use rustc_monomorphize::is_call_from_compiler_builtins_to_upstream_monomorphization; use rustc_session::config::OptLevel; use rustc_span::{source_map::Spanned, sym, Span}; use rustc_target::abi::call::{ArgAbi, FnAbi, PassMode, Reg}; diff --git a/compiler/rustc_monomorphize/src/lib.rs b/compiler/rustc_monomorphize/src/lib.rs index e5a63a98ff47b..fc6e8e0d14fd2 100644 --- a/compiler/rustc_monomorphize/src/lib.rs +++ b/compiler/rustc_monomorphize/src/lib.rs @@ -8,12 +8,8 @@ use rustc_middle::bug; use rustc_middle::query::TyCtxtAt; use rustc_middle::traits; use rustc_middle::ty::adjustment::CustomCoerceUnsized; -use rustc_middle::ty::Instance; -use rustc_middle::ty::TyCtxt; use rustc_middle::ty::{self, Ty}; use rustc_middle::util::Providers; -use rustc_span::def_id::DefId; -use rustc_span::def_id::LOCAL_CRATE; use rustc_span::ErrorGuaranteed; mod collector; @@ -46,34 +42,6 @@ fn custom_coerce_unsize_info<'tcx>( } } -/// Returns whether a call from the current crate to the [`Instance`] would produce a call -/// from `compiler_builtins` to a symbol the linker must resolve. -/// -/// Such calls from `compiler_bultins` are effectively impossible for the linker to handle. Some -/// linkers will optimize such that dead calls to unresolved symbols are not an error, but this is -/// not guaranteed. So we used this function in codegen backends to ensure we do not generate any -/// unlinkable calls. -/// -/// Note that calls to LLVM intrinsics are uniquely okay because they won't make it to the linker. -pub fn is_call_from_compiler_builtins_to_upstream_monomorphization<'tcx>( - tcx: TyCtxt<'tcx>, - instance: Instance<'tcx>, -) -> bool { - fn is_llvm_intrinsic(tcx: TyCtxt<'_>, def_id: DefId) -> bool { - if let Some(name) = tcx.codegen_fn_attrs(def_id).link_name { - name.as_str().starts_with("llvm.") - } else { - false - } - } - - let def_id = instance.def_id(); - !def_id.is_local() - && tcx.is_compiler_builtins(LOCAL_CRATE) - && !is_llvm_intrinsic(tcx, def_id) - && !tcx.should_codegen_locally(instance) -} - pub fn provide(providers: &mut Providers) { partitioning::provide(providers); polymorphize::provide(providers); From 2b5a9821eb730ab2e294bc2600af8ee580421e27 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Wed, 17 Jul 2024 18:14:54 +0300 Subject: [PATCH 04/29] abstract merge-base commit logic Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/llvm.rs | 33 ++++++++------------ src/bootstrap/src/core/config/config.rs | 36 ++++++++++------------ src/bootstrap/src/utils/helpers.rs | 24 +++++++++++++++ src/tools/build_helper/src/git.rs | 2 +- 4 files changed, 54 insertions(+), 41 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 888290a0479ba..b6b8906746206 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -26,7 +26,6 @@ use crate::{generate_smart_stamp_hash, CLang, GitRepo, Kind}; use crate::utils::exec::command; use build_helper::ci::CiEnv; -use build_helper::git::get_git_merge_base; #[derive(Clone)] pub struct LlvmResult { @@ -154,26 +153,18 @@ pub fn prebuilt_llvm_config(builder: &Builder<'_>, target: TargetSelection) -> L /// This retrieves the LLVM sha we *want* to use, according to git history. pub(crate) fn detect_llvm_sha(config: &Config, is_git: bool) -> String { let llvm_sha = if is_git { - // We proceed in 2 steps. First we get the closest commit that is actually upstream. Then we - // walk back further to the last bors merge commit that actually changed LLVM. The first - // step will fail on CI because only the `auto` branch exists; we just fall back to `HEAD` - // in that case. - let closest_upstream = get_git_merge_base(&config.git_config(), Some(&config.src)) - .unwrap_or_else(|_| "HEAD".into()); - let mut rev_list = helpers::git(Some(&config.src)); - rev_list.args(&[ - PathBuf::from("rev-list"), - format!("--author={}", config.stage0_metadata.config.git_merge_commit_email).into(), - "-n1".into(), - "--first-parent".into(), - closest_upstream.into(), - "--".into(), - config.src.join("src/llvm-project"), - config.src.join("src/bootstrap/download-ci-llvm-stamp"), - // the LLVM shared object file is named `LLVM-12-rust-{version}-nightly` - config.src.join("src/version"), - ]); - output(rev_list.as_command_mut()).trim().to_owned() + helpers::get_closest_merge_base_commit( + Some(&config.src), + &config.git_config(), + &config.stage0_metadata.config.git_merge_commit_email, + &[ + config.src.join("src/llvm-project"), + config.src.join("src/bootstrap/download-ci-llvm-stamp"), + // the LLVM shared object file is named `LLVM-12-rust-{version}-nightly` + config.src.join("src/version"), + ], + ) + .unwrap() } else if let Some(info) = channel::read_commit_info_file(&config.src) { info.sha.trim().to_owned() } else { diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index f96633b059a17..9d5aa795c6c0d 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -20,7 +20,7 @@ use crate::core::build_steps::llvm; use crate::core::config::flags::{Color, Flags, Warnings}; use crate::utils::cache::{Interned, INTERNER}; use crate::utils::channel::{self, GitInfo}; -use crate::utils::helpers::{self, exe, output, t}; +use crate::utils::helpers::{self, exe, get_closest_merge_base_commit, output, t}; use build_helper::exit; use serde::{Deserialize, Deserializer}; use serde_derive::Deserialize; @@ -2471,14 +2471,13 @@ impl Config { // Look for a version to compare to based on the current commit. // Only commits merged by bors will have CI artifacts. - let merge_base = output( - helpers::git(Some(&self.src)) - .arg("rev-list") - .arg(format!("--author={}", self.stage0_metadata.config.git_merge_commit_email)) - .args(["-n1", "--first-parent", "HEAD"]) - .as_command_mut(), - ); - let commit = merge_base.trim_end(); + let commit = get_closest_merge_base_commit( + Some(&self.src), + &self.git_config(), + &self.stage0_metadata.config.git_merge_commit_email, + &[], + ) + .unwrap(); if commit.is_empty() { println!("ERROR: could not find commit hash for downloading rustc"); println!("HELP: maybe your repository history is too shallow?"); @@ -2489,7 +2488,7 @@ impl Config { // Warn if there were changes to the compiler or standard library since the ancestor commit. let has_changes = !t!(helpers::git(Some(&self.src)) - .args(["diff-index", "--quiet", commit]) + .args(["diff-index", "--quiet", &commit]) .arg("--") .args([self.src.join("compiler"), self.src.join("library")]) .as_command_mut() @@ -2565,14 +2564,13 @@ impl Config { ) -> Option { // Look for a version to compare to based on the current commit. // Only commits merged by bors will have CI artifacts. - let merge_base = output( - helpers::git(Some(&self.src)) - .arg("rev-list") - .arg(format!("--author={}", self.stage0_metadata.config.git_merge_commit_email)) - .args(["-n1", "--first-parent", "HEAD"]) - .as_command_mut(), - ); - let commit = merge_base.trim_end(); + let commit = get_closest_merge_base_commit( + Some(&self.src), + &self.git_config(), + &self.stage0_metadata.config.git_merge_commit_email, + &[], + ) + .unwrap(); if commit.is_empty() { println!("error: could not find commit hash for downloading components from CI"); println!("help: maybe your repository history is too shallow?"); @@ -2583,7 +2581,7 @@ impl Config { // Warn if there were changes to the compiler or standard library since the ancestor commit. let mut git = helpers::git(Some(&self.src)); - git.args(["diff-index", "--quiet", commit, "--"]); + git.args(["diff-index", "--quiet", &commit, "--"]); // Handle running from a directory other than the top level let top_level = &self.src; diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index 3c82fa189be36..773a873e47cc0 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/src/bootstrap/src/utils/helpers.rs @@ -3,6 +3,7 @@ //! Simple things like testing the various filesystem operations here and there, //! not a lot of interesting happenings here unfortunately. +use build_helper::git::{get_git_merge_base, output_result, GitConfig}; use build_helper::util::fail; use std::env; use std::ffi::OsStr; @@ -521,3 +522,26 @@ pub fn git(source_dir: Option<&Path>) -> BootstrapCommand { git } + +/// Returns the closest commit available from upstream for the given `author` and `target_paths`. +/// +/// If it fails to find the commit from upstream using `git merge-base`, fallbacks to HEAD. +pub fn get_closest_merge_base_commit( + source_dir: Option<&Path>, + config: &GitConfig<'_>, + author: &str, + target_paths: &[PathBuf], +) -> Result { + let mut git = git(source_dir).capture_stdout(); + + let merge_base = get_git_merge_base(config, source_dir).unwrap_or_else(|_| "HEAD".into()); + + git.arg(Path::new("rev-list")); + git.args([&format!("--author={author}"), "-n1", "--first-parent", &merge_base]); + + if !target_paths.is_empty() { + git.arg("--").args(target_paths); + } + + Ok(output_result(git.as_command_mut())?.trim().to_owned()) +} diff --git a/src/tools/build_helper/src/git.rs b/src/tools/build_helper/src/git.rs index b4522de6897d4..8be38dc855f7a 100644 --- a/src/tools/build_helper/src/git.rs +++ b/src/tools/build_helper/src/git.rs @@ -7,7 +7,7 @@ pub struct GitConfig<'a> { } /// Runs a command and returns the output -fn output_result(cmd: &mut Command) -> Result { +pub fn output_result(cmd: &mut Command) -> Result { let output = match cmd.stderr(Stdio::inherit()).output() { Ok(status) => status, Err(e) => return Err(format!("failed to run command: {:?}: {}", cmd, e)), From 9e354daf7b3ef64740fe2ecf3c5aae9563b68189 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 16 Jul 2024 19:31:23 -0700 Subject: [PATCH 05/29] unix: Unsafe-wrap stack_overflow::signal_handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sometimes a safety comment is a prayer. avoid fuzzy provenance casts after deref. Co-authored-by: Jonas Böttiger --- library/std/src/sys/pal/unix/stack_overflow.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/library/std/src/sys/pal/unix/stack_overflow.rs b/library/std/src/sys/pal/unix/stack_overflow.rs index 2e5bd85327a19..0886624160c08 100644 --- a/library/std/src/sys/pal/unix/stack_overflow.rs +++ b/library/std/src/sys/pal/unix/stack_overflow.rs @@ -86,13 +86,18 @@ mod imp { // out many large systems and all implementations allow returning from a // signal handler to work. For a more detailed explanation see the // comments on #26458. + /// SIGSEGV/SIGBUS entry point + /// # Safety + /// Rust doesn't call this, it *gets called*. + #[forbid(unsafe_op_in_unsafe_fn)] unsafe extern "C" fn signal_handler( signum: libc::c_int, info: *mut libc::siginfo_t, _data: *mut libc::c_void, ) { let (start, end) = GUARD.get(); - let addr = (*info).si_addr() as usize; + // SAFETY: this pointer is provided by the system and will always point to a valid `siginfo_t`. + let addr = unsafe { (*info).si_addr().addr() }; // If the faulting address is within the guard page, then we print a // message saying so and abort. @@ -104,9 +109,11 @@ mod imp { rtabort!("stack overflow"); } else { // Unregister ourselves by reverting back to the default behavior. - let mut action: sigaction = mem::zeroed(); + // SAFETY: assuming all platforms define struct sigaction as "zero-initializable" + let mut action: sigaction = unsafe { mem::zeroed() }; action.sa_sigaction = SIG_DFL; - sigaction(signum, &action, ptr::null_mut()); + // SAFETY: pray this is a well-behaved POSIX implementation of fn sigaction + unsafe { sigaction(signum, &action, ptr::null_mut()) }; // See comment above for why this function returns. } From 357ba1f8ec0326b9f285224fee8a97691485475f Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 16 Jul 2024 20:19:16 -0700 Subject: [PATCH 06/29] unix: lift init of sigaltstack before sigaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is technically "not necessary", as we will "just" segfault instead if we e.g. arrive inside the handler fn with the null altstack. However, it seems incorrect to go about this hoping that segfaulting is okay, seeing as how our purpose here is to mitigate stack overflow problems. Make sure NEED_ALTSTACK syncs with PAGE_SIZE when we do. Co-authored-by: Jonas Böttiger --- .../std/src/sys/pal/unix/stack_overflow.rs | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/library/std/src/sys/pal/unix/stack_overflow.rs b/library/std/src/sys/pal/unix/stack_overflow.rs index 0886624160c08..160e58e16a5c4 100644 --- a/library/std/src/sys/pal/unix/stack_overflow.rs +++ b/library/std/src/sys/pal/unix/stack_overflow.rs @@ -123,28 +123,36 @@ mod imp { static MAIN_ALTSTACK: AtomicPtr = AtomicPtr::new(ptr::null_mut()); static NEED_ALTSTACK: AtomicBool = AtomicBool::new(false); + /// # Safety + /// Must be called only once + #[forbid(unsafe_op_in_unsafe_fn)] pub unsafe fn init() { PAGE_SIZE.store(os::page_size(), Ordering::Relaxed); // Always write to GUARD to ensure the TLS variable is allocated. - let guard = install_main_guard().unwrap_or(0..0); + let guard = unsafe { install_main_guard().unwrap_or(0..0) }; GUARD.set((guard.start, guard.end)); - let mut action: sigaction = mem::zeroed(); + // SAFETY: assuming all platforms define struct sigaction as "zero-initializable" + let mut action: sigaction = unsafe { mem::zeroed() }; for &signal in &[SIGSEGV, SIGBUS] { - sigaction(signal, ptr::null_mut(), &mut action); + // SAFETY: just fetches the current signal handler into action + unsafe { sigaction(signal, ptr::null_mut(), &mut action) }; // Configure our signal handler if one is not already set. if action.sa_sigaction == SIG_DFL { + if !NEED_ALTSTACK.load(Ordering::Relaxed) { + // haven't set up our sigaltstack yet + NEED_ALTSTACK.store(true, Ordering::Release); + let handler = unsafe { make_handler(true) }; + MAIN_ALTSTACK.store(handler.data, Ordering::Relaxed); + mem::forget(handler); + } action.sa_flags = SA_SIGINFO | SA_ONSTACK; action.sa_sigaction = signal_handler as sighandler_t; - sigaction(signal, &action, ptr::null_mut()); - NEED_ALTSTACK.store(true, Ordering::Relaxed); + // SAFETY: only overriding signals if the default is set + unsafe { sigaction(signal, &action, ptr::null_mut()) }; } } - - let handler = make_handler(true); - MAIN_ALTSTACK.store(handler.data, Ordering::Relaxed); - mem::forget(handler); } pub unsafe fn cleanup() { From fa628ceaff4e24fad699cc651cc6e7d521b642a9 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 16 Jul 2024 21:07:44 -0700 Subject: [PATCH 07/29] unix: Unsafe-wrap stack_overflow::cleanup Editorialize on the wisdom of this as we do. --- library/std/src/sys/pal/unix/stack_overflow.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/pal/unix/stack_overflow.rs b/library/std/src/sys/pal/unix/stack_overflow.rs index 160e58e16a5c4..e016f3b3ca4b3 100644 --- a/library/std/src/sys/pal/unix/stack_overflow.rs +++ b/library/std/src/sys/pal/unix/stack_overflow.rs @@ -155,8 +155,13 @@ mod imp { } } + /// # Safety + /// Must be called only once + #[forbid(unsafe_op_in_unsafe_fn)] pub unsafe fn cleanup() { - drop_handler(MAIN_ALTSTACK.load(Ordering::Relaxed)); + // FIXME: I probably cause more bugs than I'm worth! + // see https://github.com/rust-lang/rust/issues/111272 + unsafe { drop_handler(MAIN_ALTSTACK.load(Ordering::Relaxed)) }; } unsafe fn get_stack() -> libc::stack_t { From c1740eee1ea8dc046f9195d7578d3079ef423f7b Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 16 Jul 2024 21:08:06 -0700 Subject: [PATCH 08/29] unix: Unsafe-wrap stack_overflow::{drop,make}_handler Note that current_guard is probably not unsafe for future work. --- .../std/src/sys/pal/unix/stack_overflow.rs | 37 ++++++++++++++----- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/library/std/src/sys/pal/unix/stack_overflow.rs b/library/std/src/sys/pal/unix/stack_overflow.rs index e016f3b3ca4b3..3944e4e1b48a2 100644 --- a/library/std/src/sys/pal/unix/stack_overflow.rs +++ b/library/std/src/sys/pal/unix/stack_overflow.rs @@ -206,6 +206,9 @@ mod imp { libc::stack_t { ss_sp: stackp, ss_flags: 0, ss_size: sigstack_size } } + /// # Safety + /// Mutates the alternate signal stack + #[forbid(unsafe_op_in_unsafe_fn)] pub unsafe fn make_handler(main_thread: bool) -> Handler { if !NEED_ALTSTACK.load(Ordering::Relaxed) { return Handler::null(); @@ -213,27 +216,38 @@ mod imp { if !main_thread { // Always write to GUARD to ensure the TLS variable is allocated. - let guard = current_guard().unwrap_or(0..0); + let guard = unsafe { current_guard() }.unwrap_or(0..0); GUARD.set((guard.start, guard.end)); } - let mut stack = mem::zeroed(); - sigaltstack(ptr::null(), &mut stack); + // SAFETY: assuming stack_t is zero-initializable + let mut stack = unsafe { mem::zeroed() }; + // SAFETY: reads current stack_t into stack + unsafe { sigaltstack(ptr::null(), &mut stack) }; // Configure alternate signal stack, if one is not already set. if stack.ss_flags & SS_DISABLE != 0 { - stack = get_stack(); - sigaltstack(&stack, ptr::null_mut()); + // SAFETY: We warned our caller this would happen! + unsafe { + stack = get_stack(); + sigaltstack(&stack, ptr::null_mut()); + } Handler { data: stack.ss_sp as *mut libc::c_void } } else { Handler::null() } } + /// # Safety + /// Must be called + /// - only with our handler or nullptr + /// - only when done with our altstack + /// This disables the alternate signal stack! + #[forbid(unsafe_op_in_unsafe_fn)] pub unsafe fn drop_handler(data: *mut libc::c_void) { if !data.is_null() { let sigstack_size = sigstack_size(); let page_size = PAGE_SIZE.load(Ordering::Relaxed); - let stack = libc::stack_t { + let disabling_stack = libc::stack_t { ss_sp: ptr::null_mut(), ss_flags: SS_DISABLE, // Workaround for bug in macOS implementation of sigaltstack @@ -242,10 +256,11 @@ mod imp { // both ss_sp and ss_size should be ignored in this case. ss_size: sigstack_size, }; - sigaltstack(&stack, ptr::null_mut()); - // We know from `get_stackp` that the alternate stack we installed is part of a mapping - // that started one page earlier, so walk back a page and unmap from there. - munmap(data.sub(page_size), sigstack_size + page_size); + // SAFETY: we warned the caller this disables the alternate signal stack! + unsafe { sigaltstack(&disabling_stack, ptr::null_mut()) }; + // SAFETY: We know from `get_stackp` that the alternate stack we installed is part of + // a mapping that started one page earlier, so walk back a page and unmap from there. + unsafe { munmap(data.sub(page_size), sigstack_size + page_size) }; } } @@ -446,6 +461,7 @@ mod imp { } #[cfg(any(target_os = "macos", target_os = "openbsd", target_os = "solaris"))] + // FIXME: I am probably not unsafe. unsafe fn current_guard() -> Option> { let stackptr = get_stack_start()?; let stackaddr = stackptr.addr(); @@ -460,6 +476,7 @@ mod imp { target_os = "netbsd", target_os = "l4re" ))] + // FIXME: I am probably not unsafe. unsafe fn current_guard() -> Option> { let mut ret = None; let mut attr: libc::pthread_attr_t = crate::mem::zeroed(); From 529fcbcd6d7c72e942f0e70fefb7db182338ee43 Mon Sep 17 00:00:00 2001 From: Jubilee <46493976+workingjubilee@users.noreply.github.com> Date: Thu, 18 Jul 2024 15:24:40 -0700 Subject: [PATCH 09/29] unix: acquire-load NEED_ALTSTACK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jonas Böttiger --- library/std/src/sys/pal/unix/stack_overflow.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/pal/unix/stack_overflow.rs b/library/std/src/sys/pal/unix/stack_overflow.rs index 3944e4e1b48a2..047edebb8617b 100644 --- a/library/std/src/sys/pal/unix/stack_overflow.rs +++ b/library/std/src/sys/pal/unix/stack_overflow.rs @@ -210,7 +210,7 @@ mod imp { /// Mutates the alternate signal stack #[forbid(unsafe_op_in_unsafe_fn)] pub unsafe fn make_handler(main_thread: bool) -> Handler { - if !NEED_ALTSTACK.load(Ordering::Relaxed) { + if !NEED_ALTSTACK.load(Ordering::Acquire) { return Handler::null(); } From ead6aad53284bce051d861be0d66794c2785c4ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 7 Jul 2024 18:59:01 +0200 Subject: [PATCH 10/29] Make command output capturing more explicit Now there are separate functions for running a command without capturing, running while capturing stdout and running while capturing everything. This should help avoid situations where stdout/stderr is accessed when it was not captured. --- src/bootstrap/src/core/build_steps/compile.rs | 6 +- src/bootstrap/src/core/build_steps/dist.rs | 6 +- src/bootstrap/src/core/build_steps/format.rs | 10 +- src/bootstrap/src/core/build_steps/llvm.rs | 6 +- src/bootstrap/src/core/build_steps/run.rs | 2 +- src/bootstrap/src/core/build_steps/setup.rs | 11 +-- src/bootstrap/src/core/build_steps/suggest.rs | 3 +- .../src/core/build_steps/synthetic_targets.rs | 2 +- src/bootstrap/src/core/build_steps/test.rs | 96 +++++++++---------- src/bootstrap/src/core/build_steps/tool.rs | 2 +- .../src/core/build_steps/toolstate.rs | 9 +- src/bootstrap/src/core/builder.rs | 2 +- src/bootstrap/src/core/metadata.rs | 2 +- src/bootstrap/src/core/sanity.rs | 2 +- src/bootstrap/src/lib.rs | 58 ++++++----- src/bootstrap/src/utils/cc_detect.rs | 6 +- src/bootstrap/src/utils/exec.rs | 33 +++---- src/bootstrap/src/utils/helpers.rs | 6 +- src/bootstrap/src/utils/tarball.rs | 3 +- 19 files changed, 120 insertions(+), 145 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index d4dd3e546ec44..d52d8e76eac80 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -1481,7 +1481,7 @@ pub fn compiler_file( let mut cmd = command(compiler); cmd.args(builder.cflags(target, GitRepo::Rustc, c)); cmd.arg(format!("-print-file-name={file}")); - let out = cmd.capture_stdout().run(builder).stdout(); + let out = cmd.run_capture_stdout(builder).stdout(); PathBuf::from(out.trim()) } @@ -1842,7 +1842,7 @@ impl Step for Assemble { builder.ensure(llvm::Llvm { target: target_compiler.host }); if !builder.config.dry_run() && builder.config.llvm_tools_enabled { let llvm_bin_dir = - command(llvm_config).capture_stdout().arg("--bindir").run(builder).stdout(); + command(llvm_config).arg("--bindir").run_capture_stdout(builder).stdout(); let llvm_bin_dir = Path::new(llvm_bin_dir.trim()); // Since we've already built the LLVM tools, install them to the sysroot. @@ -2168,7 +2168,7 @@ pub fn strip_debug(builder: &Builder<'_>, target: TargetSelection, path: &Path) } let previous_mtime = t!(t!(path.metadata()).modified()); - command("strip").capture().arg("--strip-debug").arg(path).run(builder); + command("strip").arg("--strip-debug").arg(path).run_capture(builder); let file = t!(fs::File::open(path)); diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs index 1e9d2025bc78d..21847beda7828 100644 --- a/src/bootstrap/src/core/build_steps/dist.rs +++ b/src/bootstrap/src/core/build_steps/dist.rs @@ -182,7 +182,7 @@ fn make_win_dist( //Ask gcc where it keeps its stuff let mut cmd = command(builder.cc(target)); cmd.arg("-print-search-dirs"); - let gcc_out = cmd.capture_stdout().run(builder).stdout(); + let gcc_out = cmd.run_capture_stdout(builder).stdout(); let mut bin_path: Vec<_> = env::split_paths(&env::var_os("PATH").unwrap_or_default()).collect(); let mut lib_path = Vec::new(); @@ -1060,7 +1060,7 @@ impl Step for PlainSourceTarball { cmd.arg("--sync").arg(manifest_path); } - let config = cmd.capture().run(builder).stdout(); + let config = cmd.run_capture(builder).stdout(); let cargo_config_dir = plain_dst_src.join(".cargo"); builder.create_dir(&cargo_config_dir); @@ -2068,7 +2068,7 @@ fn maybe_install_llvm( let mut cmd = command(llvm_config); cmd.arg("--libfiles"); builder.verbose(|| println!("running {cmd:?}")); - let files = cmd.capture_stdout().run(builder).stdout(); + let files = cmd.run_capture_stdout(builder).stdout(); let build_llvm_out = &builder.llvm_out(builder.config.build); let target_llvm_out = &builder.llvm_out(target); for file in files.trim_end().split(' ') { diff --git a/src/bootstrap/src/core/build_steps/format.rs b/src/bootstrap/src/core/build_steps/format.rs index d3ac2bae1e875..f254c058b129a 100644 --- a/src/bootstrap/src/core/build_steps/format.rs +++ b/src/bootstrap/src/core/build_steps/format.rs @@ -60,7 +60,7 @@ fn get_rustfmt_version(build: &Builder<'_>) -> Option<(String, PathBuf)> { }); cmd.arg("--version"); - let output = cmd.capture().allow_failure().run(build); + let output = cmd.allow_failure().run_capture(build); if output.is_failure() { return None; } @@ -160,25 +160,23 @@ pub fn format(build: &Builder<'_>, check: bool, all: bool, paths: &[PathBuf]) { } } let git_available = - helpers::git(None).capture().allow_failure().arg("--version").run(build).is_success(); + helpers::git(None).allow_failure().arg("--version").run_capture(build).is_success(); let mut adjective = None; if git_available { let in_working_tree = helpers::git(Some(&build.src)) - .capture() .allow_failure() .arg("rev-parse") .arg("--is-inside-work-tree") - .run(build) + .run_capture(build) .is_success(); if in_working_tree { let untracked_paths_output = helpers::git(Some(&build.src)) - .capture_stdout() .arg("status") .arg("--porcelain") .arg("-z") .arg("--untracked-files=normal") - .run(build) + .run_capture_stdout(build) .stdout(); let untracked_paths: Vec<_> = untracked_paths_output .split_terminator('\0') diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 888290a0479ba..b534b4b39a408 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -480,7 +480,7 @@ impl Step for Llvm { builder.ensure(Llvm { target: builder.config.build }); if !builder.config.dry_run() { let llvm_bindir = - command(&llvm_config).capture_stdout().arg("--bindir").run(builder).stdout(); + command(&llvm_config).arg("--bindir").run_capture_stdout(builder).stdout(); let host_bin = Path::new(llvm_bindir.trim()); cfg.define( "LLVM_TABLEGEN", @@ -531,7 +531,7 @@ impl Step for Llvm { // Helper to find the name of LLVM's shared library on darwin and linux. let find_llvm_lib_name = |extension| { let version = - command(&res.llvm_config).capture_stdout().arg("--version").run(builder).stdout(); + command(&res.llvm_config).arg("--version").run_capture_stdout(builder).stdout(); let major = version.split('.').next().unwrap(); match &llvm_version_suffix { @@ -587,7 +587,7 @@ fn check_llvm_version(builder: &Builder<'_>, llvm_config: &Path) { return; } - let version = command(llvm_config).capture_stdout().arg("--version").run(builder).stdout(); + let version = command(llvm_config).arg("--version").run_capture_stdout(builder).stdout(); let mut parts = version.split('.').take(2).filter_map(|s| s.parse::().ok()); if let (Some(major), Some(_minor)) = (parts.next(), parts.next()) { if major >= 17 { diff --git a/src/bootstrap/src/core/build_steps/run.rs b/src/bootstrap/src/core/build_steps/run.rs index 3a2d3f675228d..b3ddb02801563 100644 --- a/src/bootstrap/src/core/build_steps/run.rs +++ b/src/bootstrap/src/core/build_steps/run.rs @@ -40,7 +40,7 @@ impl Step for BuildManifest { panic!("\n\nfailed to specify `dist.upload-addr` in `config.toml`\n\n") }); - let today = command("date").capture_stdout().arg("+%Y-%m-%d").run(builder).stdout(); + let today = command("date").arg("+%Y-%m-%d").run_capture_stdout(builder).stdout(); cmd.arg(sign); cmd.arg(distdir(builder)); diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs index 226b3729c10ce..7da91b83895b7 100644 --- a/src/bootstrap/src/core/build_steps/setup.rs +++ b/src/bootstrap/src/core/build_steps/setup.rs @@ -275,7 +275,7 @@ impl Step for Link { } fn rustup_installed(builder: &Builder<'_>) -> bool { - command("rustup").capture_stdout().arg("--version").run(builder).is_success() + command("rustup").arg("--version").run_capture_stdout(builder).is_success() } fn stage_dir_exists(stage_path: &str) -> bool { @@ -313,10 +313,9 @@ fn attempt_toolchain_link(builder: &Builder<'_>, stage_path: &str) { fn toolchain_is_linked(builder: &Builder<'_>) -> bool { match command("rustup") - .capture_stdout() .allow_failure() .args(["toolchain", "list"]) - .run(builder) + .run_capture_stdout(builder) .stdout_if_ok() { Some(toolchain_list) => { @@ -341,9 +340,8 @@ fn toolchain_is_linked(builder: &Builder<'_>) -> bool { fn try_link_toolchain(builder: &Builder<'_>, stage_path: &str) -> bool { command("rustup") - .capture_stdout() .args(["toolchain", "link", "stage1", stage_path]) - .run(builder) + .run_capture_stdout(builder) .is_success() } @@ -481,9 +479,8 @@ impl Step for Hook { // install a git hook to automatically run tidy, if they want fn install_git_hook_maybe(builder: &Builder<'_>, config: &Config) -> io::Result<()> { let git = helpers::git(Some(&config.src)) - .capture() .args(["rev-parse", "--git-common-dir"]) - .run(builder) + .run_capture(builder) .stdout(); let git = PathBuf::from(git.trim()); let hooks_dir = git.join("hooks"); diff --git a/src/bootstrap/src/core/build_steps/suggest.rs b/src/bootstrap/src/core/build_steps/suggest.rs index 2d4409d27c65f..0f4765fc12120 100644 --- a/src/bootstrap/src/core/build_steps/suggest.rs +++ b/src/bootstrap/src/core/build_steps/suggest.rs @@ -14,10 +14,9 @@ pub fn suggest(builder: &Builder<'_>, run: bool) { let git_config = builder.config.git_config(); let suggestions = builder .tool_cmd(Tool::SuggestTests) - .capture_stdout() .env("SUGGEST_TESTS_GIT_REPOSITORY", git_config.git_repository) .env("SUGGEST_TESTS_NIGHTLY_BRANCH", git_config.nightly_branch) - .run(builder) + .run_capture_stdout(builder) .stdout(); let suggestions = suggestions diff --git a/src/bootstrap/src/core/build_steps/synthetic_targets.rs b/src/bootstrap/src/core/build_steps/synthetic_targets.rs index 021680302cde6..04297c01d10aa 100644 --- a/src/bootstrap/src/core/build_steps/synthetic_targets.rs +++ b/src/bootstrap/src/core/build_steps/synthetic_targets.rs @@ -64,7 +64,7 @@ fn create_synthetic_target( // we cannot use nightly features. So `RUSTC_BOOTSTRAP` is needed here. cmd.env("RUSTC_BOOTSTRAP", "1"); - let output = cmd.capture().run(builder).stdout(); + let output = cmd.run_capture(builder).stdout(); let mut spec: serde_json::Value = serde_json::from_slice(output.as_bytes()).unwrap(); let spec_map = spec.as_object_mut().unwrap(); diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 3f0cbde64e394..8b6d88cf3f3f8 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -169,7 +169,7 @@ You can skip linkcheck with --skip src/tools/linkchecker" } fn check_if_tidy_is_installed(builder: &Builder<'_>) -> bool { - command("tidy").capture_stdout().allow_failure().arg("--version").run(builder).is_success() + command("tidy").allow_failure().arg("--version").run_capture_stdout(builder).is_success() } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -468,7 +468,7 @@ impl Miri { cargo.arg("--print-sysroot"); builder.verbose(|| println!("running: {cargo:?}")); - let stdout = cargo.capture_stdout().run(builder).stdout(); + let stdout = cargo.run_capture_stdout(builder).stdout(); // Output is "\n". let sysroot = stdout.trim_end(); builder.verbose(|| println!("`cargo miri setup --print-sysroot` said: {sysroot:?}")); @@ -749,7 +749,7 @@ impl Step for Clippy { let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "clippy", host, host); // Clippy reports errors if it blessed the outputs - if cargo.allow_failure().run(builder).is_success() { + if cargo.allow_failure().run(builder) { // The tests succeeded; nothing to do. return; } @@ -904,12 +904,12 @@ fn get_browser_ui_test_version_inner( npm: &Path, global: bool, ) -> Option { - let mut command = command(npm).capture(); + let mut command = command(npm); command.arg("list").arg("--parseable").arg("--long").arg("--depth=0"); if global { command.arg("--global"); } - let lines = command.allow_failure().run(builder).stdout(); + let lines = command.allow_failure().run_capture(builder).stdout(); lines .lines() .find_map(|l| l.split(':').nth(1)?.strip_prefix("browser-ui-test@")) @@ -1846,19 +1846,17 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the let lldb_exe = builder.config.lldb.clone().unwrap_or_else(|| PathBuf::from("lldb")); let lldb_version = command(&lldb_exe) - .capture() .allow_failure() .arg("--version") - .run(builder) + .run_capture(builder) .stdout_if_ok() .and_then(|v| if v.trim().is_empty() { None } else { Some(v) }); if let Some(ref vers) = lldb_version { cmd.arg("--lldb-version").arg(vers); let lldb_python_dir = command(&lldb_exe) .allow_failure() - .capture_stdout() .arg("-P") - .run(builder) + .run_capture_stdout(builder) .stdout_if_ok() .map(|p| p.lines().next().expect("lldb Python dir not found").to_string()); if let Some(ref dir) = lldb_python_dir { @@ -1917,10 +1915,9 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the builder.ensure(llvm::Llvm { target: builder.config.build }); if !builder.config.dry_run() { let llvm_version = - builder.run(command(&llvm_config).capture_stdout().arg("--version")).stdout(); - let llvm_components = builder - .run(command(&llvm_config).capture_stdout().arg("--components")) - .stdout(); + command(&llvm_config).arg("--version").run_capture_stdout(builder).stdout(); + let llvm_components = + command(&llvm_config).arg("--components").run_capture_stdout(builder).stdout(); // Remove trailing newline from llvm-config output. cmd.arg("--llvm-version") .arg(llvm_version.trim()) @@ -1940,7 +1937,7 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the // platform-specific environment variable as a workaround. if !builder.config.dry_run() && suite.ends_with("fulldeps") { let llvm_libdir = - builder.run(command(&llvm_config).capture_stdout().arg("--libdir")).stdout(); + command(&llvm_config).arg("--libdir").run_capture_stdout(builder).stdout(); add_link_lib_path(vec![llvm_libdir.trim().into()], &mut cmd); } @@ -2212,7 +2209,7 @@ impl BookTest { compiler.host, ); let _time = helpers::timeit(builder); - let toolstate = if rustbook_cmd.delay_failure().run(builder).is_success() { + let toolstate = if rustbook_cmd.delay_failure().run(builder) { ToolState::TestPass } else { ToolState::TestFail @@ -2345,7 +2342,7 @@ impl Step for ErrorIndex { let guard = builder.msg(Kind::Test, compiler.stage, "error-index", compiler.host, compiler.host); let _time = helpers::timeit(builder); - tool.capture().run(builder); + tool.run_capture(builder); drop(guard); // The tests themselves need to link to std, so make sure it is // available. @@ -2376,9 +2373,10 @@ fn markdown_test(builder: &Builder<'_>, compiler: Compiler, markdown: &Path) -> cmd = cmd.delay_failure(); if !builder.config.verbose_tests { - cmd = cmd.capture(); + cmd.run_capture(builder).is_success() + } else { + cmd.run(builder) } - cmd.run(builder).is_success() } #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -2404,11 +2402,8 @@ impl Step for RustcGuide { let src = builder.src.join(relative_path); let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook).delay_failure(); rustbook_cmd.arg("linkcheck").arg(&src); - let toolstate = if rustbook_cmd.run(builder).is_success() { - ToolState::TestPass - } else { - ToolState::TestFail - }; + let toolstate = + if rustbook_cmd.run(builder) { ToolState::TestPass } else { ToolState::TestFail }; builder.save_toolstate("rustc-dev-guide", toolstate); } } @@ -2920,7 +2915,7 @@ impl Step for RemoteCopyLibs { let f = t!(f); let name = f.file_name().into_string().unwrap(); if helpers::is_dylib(&name) { - builder.run(command(&tool).arg("push").arg(f.path())); + command(&tool).arg("push").arg(f.path()).run(builder); } } } @@ -2951,21 +2946,21 @@ impl Step for Distcheck { builder.ensure(dist::PlainSourceTarball); builder.ensure(dist::Src); - let mut cmd = command("tar"); - cmd.arg("-xf") + command("tar") + .arg("-xf") .arg(builder.ensure(dist::PlainSourceTarball).tarball()) .arg("--strip-components=1") - .current_dir(&dir); - cmd.run(builder); - builder.run( - command("./configure") - .args(&builder.config.configure_args) - .arg("--enable-vendor") - .current_dir(&dir), - ); - builder.run( - command(helpers::make(&builder.config.build.triple)).arg("check").current_dir(&dir), - ); + .current_dir(&dir) + .run(builder); + command("./configure") + .args(&builder.config.configure_args) + .arg("--enable-vendor") + .current_dir(&dir) + .run(builder); + command(helpers::make(&builder.config.build.triple)) + .arg("check") + .current_dir(&dir) + .run(builder); // Now make sure that rust-src has all of libstd's dependencies builder.info("Distcheck rust-src"); @@ -2973,24 +2968,23 @@ impl Step for Distcheck { let _ = fs::remove_dir_all(&dir); t!(fs::create_dir_all(&dir)); - let mut cmd = command("tar"); - cmd.arg("-xf") + command("tar") + .arg("-xf") .arg(builder.ensure(dist::Src).tarball()) .arg("--strip-components=1") - .current_dir(&dir); - cmd.run(builder); + .current_dir(&dir) + .run(builder); let toml = dir.join("rust-src/lib/rustlib/src/rust/library/std/Cargo.toml"); - builder.run( - command(&builder.initial_cargo) - // Will read the libstd Cargo.toml - // which uses the unstable `public-dependency` feature. - .env("RUSTC_BOOTSTRAP", "1") - .arg("generate-lockfile") - .arg("--manifest-path") - .arg(&toml) - .current_dir(&dir), - ); + command(&builder.initial_cargo) + // Will read the libstd Cargo.toml + // which uses the unstable `public-dependency` feature. + .env("RUSTC_BOOTSTRAP", "1") + .arg("generate-lockfile") + .arg("--manifest-path") + .arg(&toml) + .current_dir(&dir) + .run(builder); } } diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 2f8b41334fc7c..2fada867aff31 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -935,7 +935,7 @@ impl Step for LibcxxVersionTool { } } - let version_output = command(executable).capture_stdout().run(builder).stdout(); + let version_output = command(executable).run_capture_stdout(builder).stdout(); let version_str = version_output.split_once("version:").unwrap().1; let version = version_str.trim().parse::().unwrap(); diff --git a/src/bootstrap/src/core/build_steps/toolstate.rs b/src/bootstrap/src/core/build_steps/toolstate.rs index 2ab0ce7454b17..912cd4b8676f6 100644 --- a/src/bootstrap/src/core/build_steps/toolstate.rs +++ b/src/bootstrap/src/core/build_steps/toolstate.rs @@ -102,12 +102,11 @@ fn print_error(tool: &str, submodule: &str) { fn check_changed_files(builder: &Builder<'_>, toolstates: &HashMap, ToolState>) { // Changed files let output = helpers::git(None) - .capture() .arg("diff") .arg("--name-status") .arg("HEAD") .arg("HEAD^") - .run(builder) + .run_capture(builder) .stdout(); for (tool, submodule) in STABLE_TOOLS.iter().chain(NIGHTLY_TOOLS.iter()) { @@ -391,7 +390,7 @@ fn commit_toolstate_change(builder: &Builder<'_>, current_toolstate: &ToolstateD .arg("-m") .arg(&message) .run(builder); - if !status.is_success() { + if !status { success = true; break; } @@ -403,7 +402,7 @@ fn commit_toolstate_change(builder: &Builder<'_>, current_toolstate: &ToolstateD .arg("master") .run(builder); // If we successfully push, exit. - if status.is_success() { + if status { success = true; break; } @@ -432,7 +431,7 @@ fn commit_toolstate_change(builder: &Builder<'_>, current_toolstate: &ToolstateD /// `publish_toolstate.py` script if the PR passes all tests and is merged to /// master. fn publish_test_results(builder: &Builder<'_>, current_toolstate: &ToolstateData) { - let commit = helpers::git(None).capture().arg("rev-parse").arg("HEAD").run(builder).stdout(); + let commit = helpers::git(None).arg("rev-parse").arg("HEAD").run_capture(builder).stdout(); let toolstate_serialized = t!(serde_json::to_string(¤t_toolstate)); diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index 6d6df650b149b..67ffdc5e6f4f0 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -1951,7 +1951,7 @@ impl<'a> Builder<'a> { if mode == Mode::ToolRustc || mode == Mode::Codegen { if let Some(llvm_config) = self.llvm_config(target) { let llvm_libdir = - command(llvm_config).capture_stdout().arg("--libdir").run(self).stdout(); + command(llvm_config).arg("--libdir").run_capture_stdout(self).stdout(); add_link_lib_path(vec![llvm_libdir.trim().into()], &mut cargo); } } diff --git a/src/bootstrap/src/core/metadata.rs b/src/bootstrap/src/core/metadata.rs index b18da844014be..9b4c85e6d34a4 100644 --- a/src/bootstrap/src/core/metadata.rs +++ b/src/bootstrap/src/core/metadata.rs @@ -81,7 +81,7 @@ fn workspace_members(build: &Build) -> Vec { .arg("--no-deps") .arg("--manifest-path") .arg(build.src.join(manifest_path)); - let metadata_output = cargo.capture_stdout().run_always().run(build).stdout(); + let metadata_output = cargo.run_always().run_capture_stdout(build).stdout(); let Output { packages, .. } = t!(serde_json::from_str(&metadata_output)); packages }; diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs index 2be819d52ea1a..8aa0c43ad6e38 100644 --- a/src/bootstrap/src/core/sanity.rs +++ b/src/bootstrap/src/core/sanity.rs @@ -352,7 +352,7 @@ than building it. // There are three builds of cmake on windows: MSVC, MinGW, and // Cygwin. The Cygwin build does not have generators for Visual // Studio, so detect that here and error. - let out = command("cmake").capture_stdout().arg("--help").run(build).stdout(); + let out = command("cmake").arg("--help").run_capture_stdout(build).stdout(); if !out.contains("Visual Studio") { panic!( " diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index a77c20067e6bc..046c3d413d32b 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -40,7 +40,7 @@ use crate::core::builder::{Builder, Kind}; use crate::core::config::{flags, LldMode}; use crate::core::config::{DryRun, Target}; use crate::core::config::{LlvmLibunwind, TargetSelection}; -use crate::utils::exec::{command, BehaviorOnFailure, BootstrapCommand, CommandOutput}; +use crate::utils::exec::{command, BehaviorOnFailure, BootstrapCommand, CommandOutput, OutputMode}; use crate::utils::helpers::{self, dir_is_empty, exe, libdir, mtime, output, symlink_dir}; mod core; @@ -496,21 +496,21 @@ impl Build { // Therefore, all commands below are marked with `run_always()`, so that they also run in // dry run mode. let submodule_git = || { - let mut cmd = helpers::git(Some(&absolute_path)).capture_stdout(); + let mut cmd = helpers::git(Some(&absolute_path)); cmd.run_always(); cmd }; // Determine commit checked out in submodule. - let checked_out_hash = submodule_git().args(["rev-parse", "HEAD"]).run(self).stdout(); + let checked_out_hash = + submodule_git().args(["rev-parse", "HEAD"]).run_capture_stdout(self).stdout(); let checked_out_hash = checked_out_hash.trim_end(); // Determine commit that the submodule *should* have. let recorded = helpers::git(Some(&self.src)) - .capture_stdout() .run_always() .args(["ls-tree", "HEAD"]) .arg(relative_path) - .run(self) + .run_capture_stdout(self) .stdout(); let actual_hash = recorded .split_whitespace() @@ -534,11 +534,10 @@ impl Build { // Git is buggy and will try to fetch submodules from the tracking branch for *this* repository, // even though that has no relation to the upstream for the submodule. let current_branch = helpers::git(Some(&self.src)) - .capture_stdout() .allow_failure() .run_always() .args(["symbolic-ref", "--short", "HEAD"]) - .run(self) + .run_capture_stdout(self) .stdout_if_ok() .map(|s| s.trim().to_owned()); @@ -557,17 +556,14 @@ impl Build { git.arg(relative_path); git }; - if !update(true).run(self).is_success() { + if !update(true).run(self) { update(false).run(self); } // Save any local changes, but avoid running `git stash pop` if there are none (since it will exit with an error). // diff-index reports the modifications through the exit status - let has_local_modifications = submodule_git() - .allow_failure() - .args(["diff-index", "--quiet", "HEAD"]) - .run(self) - .is_failure(); + let has_local_modifications = + !submodule_git().allow_failure().args(["diff-index", "--quiet", "HEAD"]).run(self); if has_local_modifications { submodule_git().args(["stash", "push"]).run(self); } @@ -588,11 +584,10 @@ impl Build { return; } let output = helpers::git(Some(&self.src)) - .capture() .args(["config", "--file"]) .arg(self.config.src.join(".gitmodules")) .args(["--get-regexp", "path"]) - .run(self) + .run_capture(self) .stdout(); for line in output.lines() { // Look for `submodule.$name.path = $path` @@ -870,14 +865,14 @@ impl Build { if let Some(s) = target_config.and_then(|c| c.llvm_filecheck.as_ref()) { s.to_path_buf() } else if let Some(s) = target_config.and_then(|c| c.llvm_config.as_ref()) { - let llvm_bindir = command(s).capture_stdout().arg("--bindir").run(self).stdout(); + let llvm_bindir = command(s).arg("--bindir").run_capture_stdout(self).stdout(); let filecheck = Path::new(llvm_bindir.trim()).join(exe("FileCheck", target)); if filecheck.exists() { filecheck } else { // On Fedora the system LLVM installs FileCheck in the // llvm subdirectory of the libdir. - let llvm_libdir = command(s).capture_stdout().arg("--libdir").run(self).stdout(); + let llvm_libdir = command(s).arg("--libdir").run_capture_stdout(self).stdout(); let lib_filecheck = Path::new(llvm_libdir.trim()).join("llvm").join(exe("FileCheck", target)); if lib_filecheck.exists() { @@ -944,7 +939,12 @@ impl Build { /// Execute a command and return its output. /// This method should be used for all command executions in bootstrap. #[track_caller] - fn run(&self, command: &mut BootstrapCommand) -> CommandOutput { + fn run( + &self, + command: &mut BootstrapCommand, + stdout: OutputMode, + stderr: OutputMode, + ) -> CommandOutput { command.mark_as_executed(); if self.config.dry_run() && !command.run_always { return CommandOutput::default(); @@ -957,12 +957,11 @@ impl Build { println!("running: {command:?} (created at {created_at}, executed at {executed_at})") }); - let stdout = command.stdout.stdio(); - command.as_command_mut().stdout(stdout); - let stderr = command.stderr.stdio(); - command.as_command_mut().stderr(stderr); + let cmd = command.as_command_mut(); + cmd.stdout(stdout.stdio()); + cmd.stderr(stderr.stdio()); - let output = command.as_command_mut().output(); + let output = cmd.output(); use std::fmt::Write; @@ -988,10 +987,10 @@ Executed at: {executed_at}"#, // If the output mode is OutputMode::Capture, we can now print the output. // If it is OutputMode::Print, then the output has already been printed to // stdout/stderr, and we thus don't have anything captured to print anyway. - if command.stdout.captures() { + if stdout.captures() { writeln!(message, "\nSTDOUT ----\n{}", output.stdout().trim()).unwrap(); } - if command.stderr.captures() { + if stderr.captures() { writeln!(message, "\nSTDERR ----\n{}", output.stderr().trim()).unwrap(); } output @@ -1516,7 +1515,6 @@ Executed at: {executed_at}"#, // That's our beta number! // (Note that we use a `..` range, not the `...` symmetric difference.) helpers::git(Some(&self.src)) - .capture() .arg("rev-list") .arg("--count") .arg("--merges") @@ -1524,7 +1522,7 @@ Executed at: {executed_at}"#, "refs/remotes/origin/{}..HEAD", self.config.stage0_metadata.config.nightly_branch )) - .run(self) + .run_capture(self) .stdout() }); let n = count.trim().parse().unwrap(); @@ -1959,21 +1957,19 @@ pub fn generate_smart_stamp_hash( additional_input: &str, ) -> String { let diff = helpers::git(Some(dir)) - .capture_stdout() .allow_failure() .arg("diff") - .run(builder) + .run_capture_stdout(builder) .stdout_if_ok() .unwrap_or_default(); let status = helpers::git(Some(dir)) - .capture_stdout() .allow_failure() .arg("status") .arg("--porcelain") .arg("-z") .arg("--untracked-files=normal") - .run(builder) + .run_capture_stdout(builder) .stdout_if_ok() .unwrap_or_default(); diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index d6fa7fc0bbebd..831364b5b823d 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/src/bootstrap/src/utils/cc_detect.rs @@ -182,15 +182,15 @@ fn default_compiler( return None; } - let cmd = BootstrapCommand::from(c.to_command()); - let output = cmd.capture_stdout().arg("--version").run(build).stdout(); + let mut cmd = BootstrapCommand::from(c.to_command()); + let output = cmd.arg("--version").run_capture_stdout(build).stdout(); let i = output.find(" 4.")?; match output[i + 3..].chars().next().unwrap() { '0'..='6' => {} _ => return None, } let alternative = format!("e{gnu_compiler}"); - if command(&alternative).capture().run(build).is_success() { + if command(&alternative).run_capture(build).is_success() { Some(PathBuf::from(alternative)) } else { None diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index a60c0084f3d7f..22619e674f9a1 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -59,8 +59,6 @@ impl OutputMode { pub struct BootstrapCommand { command: Command, pub failure_behavior: BehaviorOnFailure, - pub stdout: OutputMode, - pub stderr: OutputMode, // Run the command even during dry run pub run_always: bool, // This field makes sure that each command is executed (or disarmed) before it is dropped, @@ -135,22 +133,23 @@ impl BootstrapCommand { self } - /// Capture all output of the command, do not print it. - #[must_use] - pub fn capture(self) -> Self { - Self { stdout: OutputMode::Capture, stderr: OutputMode::Capture, ..self } + /// Run the command, while printing stdout and stderr. + /// Returns true if the command has succeeded. + #[track_caller] + pub fn run(&mut self, builder: &Build) -> bool { + builder.run(self, OutputMode::Print, OutputMode::Print).is_success() } - /// Capture stdout of the command, do not print it. - #[must_use] - pub fn capture_stdout(self) -> Self { - Self { stdout: OutputMode::Capture, ..self } + /// Run the command, while capturing and returning all its output. + #[track_caller] + pub fn run_capture(&mut self, builder: &Build) -> CommandOutput { + builder.run(self, OutputMode::Capture, OutputMode::Capture) } - /// Run the command, returning its output. + /// Run the command, while capturing and returning stdout, and printing stderr. #[track_caller] - pub fn run(&mut self, builder: &Build) -> CommandOutput { - builder.run(self) + pub fn run_capture_stdout(&mut self, builder: &Build) -> CommandOutput { + builder.run(self, OutputMode::Capture, OutputMode::Print) } /// Provides access to the stdlib Command inside. @@ -189,11 +188,7 @@ impl BootstrapCommand { impl Debug for BootstrapCommand { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!(f, "{:?}", self.command)?; - write!( - f, - " (failure_mode={:?}, stdout_mode={:?}, stderr_mode={:?})", - self.failure_behavior, self.stdout, self.stderr - ) + write!(f, " (failure_mode={:?})", self.failure_behavior) } } @@ -205,8 +200,6 @@ impl From for BootstrapCommand { Self { command, failure_behavior: BehaviorOnFailure::Exit, - stdout: OutputMode::Print, - stderr: OutputMode::Print, run_always: false, drop_bomb: DropBomb::arm(program), } diff --git a/src/bootstrap/src/utils/helpers.rs b/src/bootstrap/src/utils/helpers.rs index 3c82fa189be36..51aad5acda474 100644 --- a/src/bootstrap/src/utils/helpers.rs +++ b/src/bootstrap/src/utils/helpers.rs @@ -344,7 +344,7 @@ pub fn get_clang_cl_resource_dir(builder: &Builder<'_>, clang_cl_path: &str) -> let mut builtins_locator = command(clang_cl_path); builtins_locator.args(["/clang:-print-libgcc-file-name", "/clang:--rtlib=compiler-rt"]); - let clang_rt_builtins = builtins_locator.capture_stdout().run(builder).stdout(); + let clang_rt_builtins = builtins_locator.run_capture_stdout(builder).stdout(); let clang_rt_builtins = Path::new(clang_rt_builtins.trim()); assert!( clang_rt_builtins.exists(), @@ -369,9 +369,9 @@ fn lld_flag_no_threads(builder: &Builder<'_>, lld_mode: LldMode, is_windows: boo let (windows_flag, other_flag) = LLD_NO_THREADS.get_or_init(|| { let newer_version = match lld_mode { LldMode::External => { - let mut cmd = command("lld").capture_stdout(); + let mut cmd = command("lld"); cmd.arg("-flavor").arg("ld").arg("--version"); - let out = cmd.run(builder).stdout(); + let out = cmd.run_capture_stdout(builder).stdout(); match (out.find(char::is_numeric), out.find('.')) { (Some(b), Some(e)) => out.as_str()[b..e].parse::().ok().unwrap_or(14) > 10, _ => true, diff --git a/src/bootstrap/src/utils/tarball.rs b/src/bootstrap/src/utils/tarball.rs index 9378c35127f26..817ac3eb769c3 100644 --- a/src/bootstrap/src/utils/tarball.rs +++ b/src/bootstrap/src/utils/tarball.rs @@ -370,11 +370,10 @@ impl<'a> Tarball<'a> { if self.builder.rust_info().is_managed_git_subrepository() { // %ct means committer date let timestamp = helpers::git(Some(&self.builder.src)) - .capture_stdout() .arg("log") .arg("-1") .arg("--format=%ct") - .run(self.builder) + .run_capture_stdout(self.builder) .stdout(); cmd.args(["--override-file-mtime", timestamp.trim()]); } From 1984a463df694e40b3415d7beb50f414a3a78f49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sun, 7 Jul 2024 19:04:27 +0200 Subject: [PATCH 11/29] Make it easier to detect when bootstrap tries to read uncaptured stdout/stderr If e.g. only stdout is captured, but the caller tries to read stderr, previously they would get back an empty string. Now the code will explicitly panic when accessing an uncaptured output stream. --- src/bootstrap/src/lib.rs | 6 +++-- src/bootstrap/src/utils/exec.rs | 46 ++++++++++++++++++++------------- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 046c3d413d32b..f361b69f3b6a9 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -968,7 +968,9 @@ impl Build { let mut message = String::new(); let output: CommandOutput = match output { // Command has succeeded - Ok(output) if output.status.success() => output.into(), + Ok(output) if output.status.success() => { + CommandOutput::from_output(output, stdout, stderr) + } // Command has started, but then it failed Ok(output) => { writeln!( @@ -982,7 +984,7 @@ Executed at: {executed_at}"#, ) .unwrap(); - let output: CommandOutput = output.into(); + let output: CommandOutput = CommandOutput::from_output(output, stdout, stderr); // If the output mode is OutputMode::Capture, we can now print the output. // If it is OutputMode::Print, then the output has already been printed to diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index 22619e674f9a1..d4ae8e26aaae6 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -223,17 +223,31 @@ pub fn command>(program: S) -> BootstrapCommand { } /// Represents the output of an executed process. -#[allow(unused)] pub struct CommandOutput { status: CommandStatus, - stdout: Vec, - stderr: Vec, + stdout: Option>, + stderr: Option>, } impl CommandOutput { #[must_use] pub fn did_not_start() -> Self { - Self { status: CommandStatus::DidNotStart, stdout: vec![], stderr: vec![] } + Self { status: CommandStatus::DidNotStart, stdout: None, stderr: None } + } + + #[must_use] + pub fn from_output(output: Output, stdout: OutputMode, stderr: OutputMode) -> Self { + Self { + status: CommandStatus::Finished(output.status), + stdout: match stdout { + OutputMode::Print => None, + OutputMode::Capture => Some(output.stdout), + }, + stderr: match stderr { + OutputMode::Print => None, + OutputMode::Capture => Some(output.stderr), + }, + } } #[must_use] @@ -259,7 +273,10 @@ impl CommandOutput { #[must_use] pub fn stdout(&self) -> String { - String::from_utf8(self.stdout.clone()).expect("Cannot parse process stdout as UTF-8") + String::from_utf8( + self.stdout.clone().expect("Accessing stdout of a command that did not capture stdout"), + ) + .expect("Cannot parse process stdout as UTF-8") } #[must_use] @@ -269,7 +286,10 @@ impl CommandOutput { #[must_use] pub fn stderr(&self) -> String { - String::from_utf8(self.stderr.clone()).expect("Cannot parse process stderr as UTF-8") + String::from_utf8( + self.stderr.clone().expect("Accessing stderr of a command that did not capture stderr"), + ) + .expect("Cannot parse process stderr as UTF-8") } } @@ -277,18 +297,8 @@ impl Default for CommandOutput { fn default() -> Self { Self { status: CommandStatus::Finished(ExitStatus::default()), - stdout: vec![], - stderr: vec![], - } - } -} - -impl From for CommandOutput { - fn from(output: Output) -> Self { - Self { - status: CommandStatus::Finished(output.status), - stdout: output.stdout, - stderr: output.stderr, + stdout: Some(vec![]), + stderr: Some(vec![]), } } } From 36ebf7a3211d0cb327c01357545346d2a97d54cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Fri, 19 Jul 2024 11:49:26 +0000 Subject: [PATCH 12/29] run_make_support: skip rustfmt for lib.rs to preserve use ordering --- src/tools/run-make-support/src/lib.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index e6a45f57de608..b85191970de62 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -3,6 +3,10 @@ //! notably is built via cargo: this means that if your test wants some non-trivial utility, such //! as `object` or `wasmparser`, they can be re-exported and be made available through this library. +// We want to control use declaration ordering and spacing (and preserve use group comments), so +// skip rustfmt on this file. +#![cfg_attr(rustfmt, rustfmt::skip)] + mod command; mod macros; mod util; @@ -18,6 +22,8 @@ pub mod scoped_run; pub mod string; pub mod targets; +// Internally we call our fs-related support module as `fs`, but re-export its content as `rfs` +// to tests to avoid colliding with commonly used `use std::fs;`. mod fs; /// [`std::fs`] wrappers and assorted filesystem-related helpers. Public to tests as `rfs` to not be From 5b1860a24e78fbe2d906574141e904c58c61a4ec Mon Sep 17 00:00:00 2001 From: Oneirical Date: Fri, 12 Jul 2024 14:33:16 -0400 Subject: [PATCH 13/29] rewrite return-non-c-like-enum-from-c to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - .../return-non-c-like-enum-from-c/Makefile | 6 ------ .../return-non-c-like-enum-from-c/rmake.rs | 17 +++++++++++++++++ 3 files changed, 17 insertions(+), 7 deletions(-) delete mode 100644 tests/run-make/return-non-c-like-enum-from-c/Makefile create mode 100644 tests/run-make/return-non-c-like-enum-from-c/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index bd25a6c144e64..07fd5d3e9b838 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -66,7 +66,6 @@ run-make/no-alloc-shim/Makefile run-make/no-builtins-attribute/Makefile run-make/no-duplicate-libs/Makefile run-make/panic-abort-eh_frame/Makefile -run-make/pass-non-c-like-enum-to-c/Makefile run-make/pdb-buildinfo-cl-cmd/Makefile run-make/pgo-gen-lto/Makefile run-make/pgo-gen-no-imp-symbols/Makefile diff --git a/tests/run-make/return-non-c-like-enum-from-c/Makefile b/tests/run-make/return-non-c-like-enum-from-c/Makefile deleted file mode 100644 index bd441d321bfab..0000000000000 --- a/tests/run-make/return-non-c-like-enum-from-c/Makefile +++ /dev/null @@ -1,6 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,test) - $(RUSTC) nonclike.rs -L$(TMPDIR) -ltest - $(call RUN,nonclike) diff --git a/tests/run-make/return-non-c-like-enum-from-c/rmake.rs b/tests/run-make/return-non-c-like-enum-from-c/rmake.rs new file mode 100644 index 0000000000000..f24bd6d5fc763 --- /dev/null +++ b/tests/run-make/return-non-c-like-enum-from-c/rmake.rs @@ -0,0 +1,17 @@ +// A reversed version of the `return-non-c-like-enum` test, though +// this time, the C code is the library, and the Rust code compiles +// into the executable. Once again, enum variants should be treated +// like an union of structs, which should prevent segfaults or +// unexpected results. +// See https://github.com/rust-lang/rust/issues/68190 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("test"); + rustc().input("nonclike.rs").arg("-ltest").run(); + run("nonclike"); +} From 5e55f07cc0d843dff89a1b29fb15b66e99391627 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Fri, 12 Jul 2024 14:57:05 -0400 Subject: [PATCH 14/29] rewrite and rename issue-28595 to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/issue-28595/Makefile | 7 ------- .../{issue-28595 => native-lib-load-order}/a.c | 0 .../{issue-28595 => native-lib-load-order}/a.rs | 0 .../{issue-28595 => native-lib-load-order}/b.c | 0 .../{issue-28595 => native-lib-load-order}/b.rs | 0 tests/run-make/native-lib-load-order/rmake.rs | 16 ++++++++++++++++ 7 files changed, 16 insertions(+), 8 deletions(-) delete mode 100644 tests/run-make/issue-28595/Makefile rename tests/run-make/{issue-28595 => native-lib-load-order}/a.c (100%) rename tests/run-make/{issue-28595 => native-lib-load-order}/a.rs (100%) rename tests/run-make/{issue-28595 => native-lib-load-order}/b.c (100%) rename tests/run-make/{issue-28595 => native-lib-load-order}/b.rs (100%) create mode 100644 tests/run-make/native-lib-load-order/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 07fd5d3e9b838..d935bb10d779c 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -37,7 +37,6 @@ run-make/interdependent-c-libraries/Makefile run-make/issue-107094/Makefile run-make/issue-14698/Makefile run-make/issue-15460/Makefile -run-make/issue-28595/Makefile run-make/issue-33329/Makefile run-make/issue-35164/Makefile run-make/issue-36710/Makefile diff --git a/tests/run-make/issue-28595/Makefile b/tests/run-make/issue-28595/Makefile deleted file mode 100644 index 258f9788aafa7..0000000000000 --- a/tests/run-make/issue-28595/Makefile +++ /dev/null @@ -1,7 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,a) $(call NATIVE_STATICLIB,b) - $(RUSTC) a.rs - $(RUSTC) b.rs - $(call RUN,b) diff --git a/tests/run-make/issue-28595/a.c b/tests/run-make/native-lib-load-order/a.c similarity index 100% rename from tests/run-make/issue-28595/a.c rename to tests/run-make/native-lib-load-order/a.c diff --git a/tests/run-make/issue-28595/a.rs b/tests/run-make/native-lib-load-order/a.rs similarity index 100% rename from tests/run-make/issue-28595/a.rs rename to tests/run-make/native-lib-load-order/a.rs diff --git a/tests/run-make/issue-28595/b.c b/tests/run-make/native-lib-load-order/b.c similarity index 100% rename from tests/run-make/issue-28595/b.c rename to tests/run-make/native-lib-load-order/b.c diff --git a/tests/run-make/issue-28595/b.rs b/tests/run-make/native-lib-load-order/b.rs similarity index 100% rename from tests/run-make/issue-28595/b.rs rename to tests/run-make/native-lib-load-order/b.rs diff --git a/tests/run-make/native-lib-load-order/rmake.rs b/tests/run-make/native-lib-load-order/rmake.rs new file mode 100644 index 0000000000000..ffe20a64168f8 --- /dev/null +++ b/tests/run-make/native-lib-load-order/rmake.rs @@ -0,0 +1,16 @@ +// An old compiler bug from 2015 caused native libraries to be loaded in the +// wrong order, causing `b` to be loaded before `a` in this test. If the compilation +// is successful, the libraries were loaded in the correct order. + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("a"); + build_native_static_lib("b"); + rustc().input("a.rs").run(); + rustc().input("b.rs").run(); + run("b"); +} From 1ae1ab8215c3ba2c9c0066d1aa13a033f0f94d5e Mon Sep 17 00:00:00 2001 From: Oneirical Date: Fri, 12 Jul 2024 15:14:12 -0400 Subject: [PATCH 15/29] rewrite c-static-dylib to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/c-static-dylib/Makefile | 13 ------------ tests/run-make/c-static-dylib/rmake.rs | 20 +++++++++++++++++++ 3 files changed, 20 insertions(+), 14 deletions(-) delete mode 100644 tests/run-make/c-static-dylib/Makefile create mode 100644 tests/run-make/c-static-dylib/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index d935bb10d779c..4b5ab3b063f58 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -1,7 +1,6 @@ run-make/branch-protection-check-IBT/Makefile run-make/c-dynamic-dylib/Makefile run-make/c-dynamic-rlib/Makefile -run-make/c-static-dylib/Makefile run-make/c-static-rlib/Makefile run-make/c-unwind-abi-catch-lib-panic/Makefile run-make/c-unwind-abi-catch-panic/Makefile diff --git a/tests/run-make/c-static-dylib/Makefile b/tests/run-make/c-static-dylib/Makefile deleted file mode 100644 index 05da1743c83ba..0000000000000 --- a/tests/run-make/c-static-dylib/Makefile +++ /dev/null @@ -1,13 +0,0 @@ -# This test checks that static Rust linking with C does not encounter any errors, with dynamic dependencies given preference over static. -# See https://github.com/rust-lang/rust/issues/10434 - -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,cfoo) - $(RUSTC) foo.rs -C prefer-dynamic - $(RUSTC) bar.rs - rm $(call NATIVE_STATICLIB,cfoo) - $(call RUN,bar) - $(call REMOVE_DYLIBS,foo) - $(call FAIL,bar) diff --git a/tests/run-make/c-static-dylib/rmake.rs b/tests/run-make/c-static-dylib/rmake.rs new file mode 100644 index 0000000000000..32edcd93feff3 --- /dev/null +++ b/tests/run-make/c-static-dylib/rmake.rs @@ -0,0 +1,20 @@ +// This test checks that static Rust linking with C does not encounter any errors, +// with dynamic dependencies given preference over static. +// See https://github.com/rust-lang/rust/issues/10434 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{ + build_native_static_lib, dynamic_lib_name, fs_wrapper, run, run_fail, rustc, static_lib_name, +}; + +fn main() { + build_native_static_lib("cfoo"); + rustc().input("foo.rs").arg("-Cprefer-dynamic").run(); + rustc().input("bar.rs").run(); + fs_wrapper::remove_file(static_lib_name("cfoo")); + run("bar"); + fs_wrapper::remove_file(dynamic_lib_name("foo")); + run_fail("bar"); +} From 1a4fba5eb089d0c5413ed615c6e276eb76969535 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Fri, 12 Jul 2024 15:16:53 -0400 Subject: [PATCH 16/29] rewrite c-static-rlib to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/c-static-rlib/Makefile | 12 ------------ tests/run-make/c-static-rlib/rmake.rs | 19 +++++++++++++++++++ 3 files changed, 19 insertions(+), 13 deletions(-) delete mode 100644 tests/run-make/c-static-rlib/Makefile create mode 100644 tests/run-make/c-static-rlib/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 4b5ab3b063f58..0defdb373634c 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -1,7 +1,6 @@ run-make/branch-protection-check-IBT/Makefile run-make/c-dynamic-dylib/Makefile run-make/c-dynamic-rlib/Makefile -run-make/c-static-rlib/Makefile run-make/c-unwind-abi-catch-lib-panic/Makefile run-make/c-unwind-abi-catch-panic/Makefile run-make/cat-and-grep-sanity-check/Makefile diff --git a/tests/run-make/c-static-rlib/Makefile b/tests/run-make/c-static-rlib/Makefile deleted file mode 100644 index 298e432cdb8ad..0000000000000 --- a/tests/run-make/c-static-rlib/Makefile +++ /dev/null @@ -1,12 +0,0 @@ -# This test checks that static Rust linking with C does not encounter any errors, with static dependencies given preference over dynamic. (This is the default behaviour.) -# See https://github.com/rust-lang/rust/issues/10434 - -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,cfoo) - $(RUSTC) foo.rs - $(RUSTC) bar.rs - $(call REMOVE_RLIBS,foo) - rm $(call NATIVE_STATICLIB,cfoo) - $(call RUN,bar) diff --git a/tests/run-make/c-static-rlib/rmake.rs b/tests/run-make/c-static-rlib/rmake.rs new file mode 100644 index 0000000000000..3d582dca98e12 --- /dev/null +++ b/tests/run-make/c-static-rlib/rmake.rs @@ -0,0 +1,19 @@ +// This test checks that static Rust linking with C does not encounter any errors, +// with static dependencies given preference over dynamic. (This is the default behaviour.) +// See https://github.com/rust-lang/rust/issues/10434 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{ + build_native_static_lib, fs_wrapper, run, rust_lib_name, rustc, static_lib_name, +}; + +fn main() { + build_native_static_lib("cfoo"); + rustc().input("foo.rs").run(); + rustc().input("bar.rs").run(); + fs_wrapper::remove_file(rust_lib_name("foo")); + fs_wrapper::remove_file(static_lib_name("cfoo")); + run("bar"); +} From 8a09f2231d91fad0ff09ec4d4b3a89ff0376f860 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Fri, 12 Jul 2024 15:24:18 -0400 Subject: [PATCH 17/29] rewrite extern-fn-generic to rmake --- src/tools/tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/extern-fn-generic/Makefile | 7 ------- tests/run-make/extern-fn-generic/rmake.rs | 15 +++++++++++++++ 3 files changed, 15 insertions(+), 8 deletions(-) delete mode 100644 tests/run-make/extern-fn-generic/Makefile create mode 100644 tests/run-make/extern-fn-generic/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 0defdb373634c..ef18b6f16a270 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -20,7 +20,6 @@ run-make/emit-to-stdout/Makefile run-make/export-executable-symbols/Makefile run-make/extern-diff-internal-name/Makefile run-make/extern-flag-disambiguates/Makefile -run-make/extern-fn-generic/Makefile run-make/extern-fn-reachable/Makefile run-make/extern-fn-with-union/Makefile run-make/extern-multiple-copies/Makefile diff --git a/tests/run-make/extern-fn-generic/Makefile b/tests/run-make/extern-fn-generic/Makefile deleted file mode 100644 index 7dceea6cb887c..0000000000000 --- a/tests/run-make/extern-fn-generic/Makefile +++ /dev/null @@ -1,7 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,test) - $(RUSTC) testcrate.rs - $(RUSTC) test.rs - $(call RUN,test) || exit 1 diff --git a/tests/run-make/extern-fn-generic/rmake.rs b/tests/run-make/extern-fn-generic/rmake.rs new file mode 100644 index 0000000000000..3dee1e20544e2 --- /dev/null +++ b/tests/run-make/extern-fn-generic/rmake.rs @@ -0,0 +1,15 @@ +// Generic types in foreign-function interfaces were introduced in #15831 - this +// test simply runs a Rust program containing generics that is also reliant on +// a C library, and checks that compilation and execution are successful. +// See https://github.com/rust-lang/rust/pull/15831 +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("test"); + rustc().input("testcrate.rs").run(); + rustc().input("test.rs").run(); + run("test"); +} From 01c7118fa99a0c1f2df47b25c63fae6f62ef6ed1 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Fri, 12 Jul 2024 15:32:46 -0400 Subject: [PATCH 18/29] rewrite extern-fn-with-union to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/extern-fn-generic/rmake.rs | 1 + tests/run-make/extern-fn-with-union/Makefile | 7 ------- tests/run-make/extern-fn-with-union/rmake.rs | 16 ++++++++++++++++ 4 files changed, 17 insertions(+), 8 deletions(-) delete mode 100644 tests/run-make/extern-fn-with-union/Makefile create mode 100644 tests/run-make/extern-fn-with-union/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index ef18b6f16a270..421097a4bdf8c 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -21,7 +21,6 @@ run-make/export-executable-symbols/Makefile run-make/extern-diff-internal-name/Makefile run-make/extern-flag-disambiguates/Makefile run-make/extern-fn-reachable/Makefile -run-make/extern-fn-with-union/Makefile run-make/extern-multiple-copies/Makefile run-make/extern-multiple-copies2/Makefile run-make/fmt-write-bloat/Makefile diff --git a/tests/run-make/extern-fn-generic/rmake.rs b/tests/run-make/extern-fn-generic/rmake.rs index 3dee1e20544e2..05de839a1b0d8 100644 --- a/tests/run-make/extern-fn-generic/rmake.rs +++ b/tests/run-make/extern-fn-generic/rmake.rs @@ -2,6 +2,7 @@ // test simply runs a Rust program containing generics that is also reliant on // a C library, and checks that compilation and execution are successful. // See https://github.com/rust-lang/rust/pull/15831 + //@ ignore-cross-compile // Reason: the compiled binary is executed diff --git a/tests/run-make/extern-fn-with-union/Makefile b/tests/run-make/extern-fn-with-union/Makefile deleted file mode 100644 index e6c8c9936791b..0000000000000 --- a/tests/run-make/extern-fn-with-union/Makefile +++ /dev/null @@ -1,7 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,ctest) - $(RUSTC) testcrate.rs - $(RUSTC) test.rs - $(call RUN,test) || exit 1 diff --git a/tests/run-make/extern-fn-with-union/rmake.rs b/tests/run-make/extern-fn-with-union/rmake.rs new file mode 100644 index 0000000000000..200602eabeb5c --- /dev/null +++ b/tests/run-make/extern-fn-with-union/rmake.rs @@ -0,0 +1,16 @@ +// If an external function from foreign-function interface was called upon, +// its attributes would only be passed to LLVM if and only if it was called in the same crate. +// This caused passing around unions to be incorrect. +// See https://github.com/rust-lang/rust/pull/14191 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("ctest"); + rustc().input("testcrate.rs").run(); + rustc().input("test.rs").run(); + run("test"); +} From db8398db6f79ad70ce4598372bf807c68b62ea13 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Fri, 12 Jul 2024 15:49:26 -0400 Subject: [PATCH 19/29] rewrite lto-no-link-whole-rlib to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/lto-no-link-whole-rlib/Makefile | 9 --------- tests/run-make/lto-no-link-whole-rlib/rmake.rs | 18 ++++++++++++++++++ 3 files changed, 18 insertions(+), 10 deletions(-) delete mode 100644 tests/run-make/lto-no-link-whole-rlib/Makefile create mode 100644 tests/run-make/lto-no-link-whole-rlib/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 421097a4bdf8c..3f5de1f7c1528 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -52,7 +52,6 @@ run-make/linkage-attr-on-static/Makefile run-make/long-linker-command-lines-cmd-exe/Makefile run-make/long-linker-command-lines/Makefile run-make/lto-linkage-used-attr/Makefile -run-make/lto-no-link-whole-rlib/Makefile run-make/macos-deployment-target/Makefile run-make/min-global-align/Makefile run-make/native-link-modifier-bundle/Makefile diff --git a/tests/run-make/lto-no-link-whole-rlib/Makefile b/tests/run-make/lto-no-link-whole-rlib/Makefile deleted file mode 100644 index 3e82322e72d34..0000000000000 --- a/tests/run-make/lto-no-link-whole-rlib/Makefile +++ /dev/null @@ -1,9 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,foo) $(call NATIVE_STATICLIB,bar) - $(RUSTC) lib1.rs - $(RUSTC) lib2.rs - $(RUSTC) main.rs -Clto - $(call RUN,main) - diff --git a/tests/run-make/lto-no-link-whole-rlib/rmake.rs b/tests/run-make/lto-no-link-whole-rlib/rmake.rs new file mode 100644 index 0000000000000..8cd653d5f0866 --- /dev/null +++ b/tests/run-make/lto-no-link-whole-rlib/rmake.rs @@ -0,0 +1,18 @@ +// In order to improve linking performance, entire rlibs will only be linked if a dylib is being +// created. Otherwise, an executable will only link one rlib as usual. Linking will fail in this +// test should this optimization be reverted. +// See https://github.com/rust-lang/rust/pull/31460 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("foo"); + build_native_static_lib("bar"); + rustc().input("lib1.rs").run(); + rustc().input("lib2.rs").run(); + rustc().input("main.rs").arg("-Clto").run(); + run("main"); +} From 1f976b43da767ae3926df75ccfda279159038bcb Mon Sep 17 00:00:00 2001 From: Oneirical Date: Fri, 12 Jul 2024 15:58:06 -0400 Subject: [PATCH 20/29] rewrite linkage-attr-on-static to rmake --- src/tools/tidy/src/allowed_run_make_makefiles.txt | 2 -- tests/run-make/linkage-attr-on-static/Makefile | 6 ------ tests/run-make/linkage-attr-on-static/rmake.rs | 15 +++++++++++++++ tests/run-make/pass-non-c-like-enum-to-c/Makefile | 6 ------ 4 files changed, 15 insertions(+), 14 deletions(-) delete mode 100644 tests/run-make/linkage-attr-on-static/Makefile create mode 100644 tests/run-make/linkage-attr-on-static/rmake.rs delete mode 100644 tests/run-make/pass-non-c-like-enum-to-c/Makefile diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 3f5de1f7c1528..6a987c184df3a 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -48,7 +48,6 @@ run-make/libtest-junit/Makefile run-make/libtest-thread-limit/Makefile run-make/link-cfg/Makefile run-make/link-framework/Makefile -run-make/linkage-attr-on-static/Makefile run-make/long-linker-command-lines-cmd-exe/Makefile run-make/long-linker-command-lines/Makefile run-make/lto-linkage-used-attr/Makefile @@ -76,7 +75,6 @@ run-make/redundant-libs/Makefile run-make/remap-path-prefix-dwarf/Makefile run-make/reproducible-build-2/Makefile run-make/reproducible-build/Makefile -run-make/return-non-c-like-enum-from-c/Makefile run-make/rlib-format-packed-bundled-libs-2/Makefile run-make/rlib-format-packed-bundled-libs-3/Makefile run-make/rlib-format-packed-bundled-libs/Makefile diff --git a/tests/run-make/linkage-attr-on-static/Makefile b/tests/run-make/linkage-attr-on-static/Makefile deleted file mode 100644 index ef50a7ef9f13c..0000000000000 --- a/tests/run-make/linkage-attr-on-static/Makefile +++ /dev/null @@ -1,6 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,foo) - $(RUSTC) bar.rs - $(call RUN,bar) || exit 1 diff --git a/tests/run-make/linkage-attr-on-static/rmake.rs b/tests/run-make/linkage-attr-on-static/rmake.rs new file mode 100644 index 0000000000000..cd85542e9587d --- /dev/null +++ b/tests/run-make/linkage-attr-on-static/rmake.rs @@ -0,0 +1,15 @@ +// #[linkage] is a useful attribute which can be applied to statics to allow +// external linkage, something which was not possible before #18890. This test +// checks that using this new feature results in successful compilation and execution. +// See https://github.com/rust-lang/rust/pull/18890 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("foo"); + rustc().input("bar.rs").run(); + run("bar"); +} diff --git a/tests/run-make/pass-non-c-like-enum-to-c/Makefile b/tests/run-make/pass-non-c-like-enum-to-c/Makefile deleted file mode 100644 index bd441d321bfab..0000000000000 --- a/tests/run-make/pass-non-c-like-enum-to-c/Makefile +++ /dev/null @@ -1,6 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: $(call NATIVE_STATICLIB,test) - $(RUSTC) nonclike.rs -L$(TMPDIR) -ltest - $(call RUN,nonclike) From fdc8d62c962c401a096a9e4beb63a3b421570471 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Wed, 17 Jul 2024 15:23:55 -0400 Subject: [PATCH 21/29] rewrite pass-non-c-like-enum-to-c to rmake --- tests/run-make/c-static-dylib/rmake.rs | 6 +++--- tests/run-make/c-static-rlib/rmake.rs | 8 +++----- .../pass-non-c-like-enum-to-c/rmake.rs | 19 +++++++++++++++++++ 3 files changed, 25 insertions(+), 8 deletions(-) create mode 100644 tests/run-make/pass-non-c-like-enum-to-c/rmake.rs diff --git a/tests/run-make/c-static-dylib/rmake.rs b/tests/run-make/c-static-dylib/rmake.rs index 32edcd93feff3..12ec06c8199d6 100644 --- a/tests/run-make/c-static-dylib/rmake.rs +++ b/tests/run-make/c-static-dylib/rmake.rs @@ -6,15 +6,15 @@ // Reason: the compiled binary is executed use run_make_support::{ - build_native_static_lib, dynamic_lib_name, fs_wrapper, run, run_fail, rustc, static_lib_name, + build_native_static_lib, dynamic_lib_name, rfs, run, run_fail, rustc, static_lib_name, }; fn main() { build_native_static_lib("cfoo"); rustc().input("foo.rs").arg("-Cprefer-dynamic").run(); rustc().input("bar.rs").run(); - fs_wrapper::remove_file(static_lib_name("cfoo")); + rfs::remove_file(static_lib_name("cfoo")); run("bar"); - fs_wrapper::remove_file(dynamic_lib_name("foo")); + rfs::remove_file(dynamic_lib_name("foo")); run_fail("bar"); } diff --git a/tests/run-make/c-static-rlib/rmake.rs b/tests/run-make/c-static-rlib/rmake.rs index 3d582dca98e12..447e29a14f657 100644 --- a/tests/run-make/c-static-rlib/rmake.rs +++ b/tests/run-make/c-static-rlib/rmake.rs @@ -5,15 +5,13 @@ //@ ignore-cross-compile // Reason: the compiled binary is executed -use run_make_support::{ - build_native_static_lib, fs_wrapper, run, rust_lib_name, rustc, static_lib_name, -}; +use run_make_support::{build_native_static_lib, rfs, run, rust_lib_name, rustc, static_lib_name}; fn main() { build_native_static_lib("cfoo"); rustc().input("foo.rs").run(); rustc().input("bar.rs").run(); - fs_wrapper::remove_file(rust_lib_name("foo")); - fs_wrapper::remove_file(static_lib_name("cfoo")); + rfs::remove_file(rust_lib_name("foo")); + rfs::remove_file(static_lib_name("cfoo")); run("bar"); } diff --git a/tests/run-make/pass-non-c-like-enum-to-c/rmake.rs b/tests/run-make/pass-non-c-like-enum-to-c/rmake.rs new file mode 100644 index 0000000000000..c706e82f4a0cc --- /dev/null +++ b/tests/run-make/pass-non-c-like-enum-to-c/rmake.rs @@ -0,0 +1,19 @@ +// Similar to the `return-non-c-like-enum-from-c` test, where +// the C code is the library, and the Rust code compiles +// into the executable. Once again, enum variants should be treated +// like an union of structs, which should prevent segfaults or +// unexpected results. The only difference with the aforementioned +// test is that the structs are passed into C directly through the +// `tt_add` and `t_add` function calls. +// See https://github.com/rust-lang/rust/issues/68190 + +//@ ignore-cross-compile +// Reason: the compiled binary is executed + +use run_make_support::{build_native_static_lib, run, rustc}; + +fn main() { + build_native_static_lib("test"); + rustc().input("nonclike.rs").arg("-ltest").run(); + run("nonclike"); +} From 1d83da88473e72717c089a389adcea1c53d4c9f8 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Wed, 17 Jul 2024 12:57:44 -0700 Subject: [PATCH 22/29] kmc-solid: forbid(unsafe_op_in_unsafe_fn) --- library/std/src/os/solid/io.rs | 1 - library/std/src/os/solid/mod.rs | 1 + library/std/src/sys/pal/solid/mod.rs | 2 +- library/std/src/sys/sync/mutex/itron.rs | 1 + library/std/src/sys/sync/rwlock/solid.rs | 1 + 5 files changed, 4 insertions(+), 2 deletions(-) diff --git a/library/std/src/os/solid/io.rs b/library/std/src/os/solid/io.rs index 19b4fe22093c3..e75bcf74e5cd3 100644 --- a/library/std/src/os/solid/io.rs +++ b/library/std/src/os/solid/io.rs @@ -44,7 +44,6 @@ //! //! [`BorrowedFd<'a>`]: crate::os::solid::io::BorrowedFd -#![deny(unsafe_op_in_unsafe_fn)] #![unstable(feature = "solid_ext", issue = "none")] use crate::fmt; diff --git a/library/std/src/os/solid/mod.rs b/library/std/src/os/solid/mod.rs index 0bb83c73ddf7c..75824048e2498 100644 --- a/library/std/src/os/solid/mod.rs +++ b/library/std/src/os/solid/mod.rs @@ -1,4 +1,5 @@ #![stable(feature = "rust1", since = "1.0.0")] +#![forbid(unsafe_op_in_unsafe_fn)] pub mod ffi; pub mod io; diff --git a/library/std/src/sys/pal/solid/mod.rs b/library/std/src/sys/pal/solid/mod.rs index 9a7741ddda71e..0b158fb63df70 100644 --- a/library/std/src/sys/pal/solid/mod.rs +++ b/library/std/src/sys/pal/solid/mod.rs @@ -1,6 +1,6 @@ #![allow(dead_code)] #![allow(missing_docs, nonstandard_style)] -#![deny(unsafe_op_in_unsafe_fn)] +#![forbid(unsafe_op_in_unsafe_fn)] pub mod abi; diff --git a/library/std/src/sys/sync/mutex/itron.rs b/library/std/src/sys/sync/mutex/itron.rs index 4ba32a8fbcd69..b29c7e1d03444 100644 --- a/library/std/src/sys/sync/mutex/itron.rs +++ b/library/std/src/sys/sync/mutex/itron.rs @@ -1,5 +1,6 @@ //! Mutex implementation backed by μITRON mutexes. Assumes `acre_mtx` and //! `TA_INHERIT` are available. +#![forbid(unsafe_op_in_unsafe_fn)] use crate::sys::pal::itron::{ abi, diff --git a/library/std/src/sys/sync/rwlock/solid.rs b/library/std/src/sys/sync/rwlock/solid.rs index 7558eee8edd33..a8fef685ceb27 100644 --- a/library/std/src/sys/sync/rwlock/solid.rs +++ b/library/std/src/sys/sync/rwlock/solid.rs @@ -1,4 +1,5 @@ //! A readers-writer lock implementation backed by the SOLID kernel extension. +#![forbid(unsafe_op_in_unsafe_fn)] use crate::sys::pal::{ abi, From e9b3e9c7f44949abd71ac05ea9d46b3c418888e5 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Wed, 17 Jul 2024 13:10:12 -0700 Subject: [PATCH 23/29] std: forbid unwrapped unsafe in unsupported_backslash --- library/std/src/sys/path/unsupported_backslash.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/std/src/sys/path/unsupported_backslash.rs b/library/std/src/sys/path/unsupported_backslash.rs index 7045c9be25b08..855f443678c6c 100644 --- a/library/std/src/sys/path/unsupported_backslash.rs +++ b/library/std/src/sys/path/unsupported_backslash.rs @@ -1,3 +1,4 @@ +#![forbid(unsafe_op_in_unsafe_fn)] use crate::ffi::OsStr; use crate::io; use crate::path::{Path, PathBuf, Prefix}; From 3b7e4bc77a83c1101e8799739504b0a53f0d0619 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 18 Jul 2024 20:27:15 +1000 Subject: [PATCH 24/29] Split out `remove_never_subcandidates` --- .../rustc_mir_build/src/build/matches/mod.rs | 84 +++++++++++-------- 1 file changed, 47 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index b531a392efac9..6bd2c308128ad 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1706,6 +1706,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } self.merge_trivial_subcandidates(candidate); + self.remove_never_subcandidates(candidate); if !candidate.match_pairs.is_empty() { let or_span = candidate.or_span.unwrap_or(candidate.extra_data.span); @@ -1753,44 +1754,53 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let can_merge = candidate.subcandidates.iter().all(|subcandidate| { subcandidate.subcandidates.is_empty() && subcandidate.extra_data.is_empty() }); - if can_merge { - let mut last_otherwise = None; - let any_matches = self.cfg.start_new_block(); - let or_span = candidate.or_span.take().unwrap(); - let source_info = self.source_info(or_span); - if candidate.false_edge_start_block.is_none() { - candidate.false_edge_start_block = - candidate.subcandidates[0].false_edge_start_block; - } - for subcandidate in mem::take(&mut candidate.subcandidates) { - let or_block = subcandidate.pre_binding_block.unwrap(); - self.cfg.goto(or_block, source_info, any_matches); - last_otherwise = subcandidate.otherwise_block; - } - candidate.pre_binding_block = Some(any_matches); - assert!(last_otherwise.is_some()); - candidate.otherwise_block = last_otherwise; - } else { - // Never subcandidates may have a set of bindings inconsistent with their siblings, - // which would break later code. So we filter them out. Note that we can't filter out - // top-level candidates this way. - candidate.subcandidates.retain_mut(|candidate| { - if candidate.extra_data.is_never { - candidate.visit_leaves(|subcandidate| { - let block = subcandidate.pre_binding_block.unwrap(); - // That block is already unreachable but needs a terminator to make the MIR well-formed. - let source_info = self.source_info(subcandidate.extra_data.span); - self.cfg.terminate(block, source_info, TerminatorKind::Unreachable); - }); - false - } else { - true - } - }); - if candidate.subcandidates.is_empty() { - // If `candidate` has become a leaf candidate, ensure it has a `pre_binding_block`. - candidate.pre_binding_block = Some(self.cfg.start_new_block()); + if !can_merge { + return; + } + + let mut last_otherwise = None; + let any_matches = self.cfg.start_new_block(); + let or_span = candidate.or_span.take().unwrap(); + let source_info = self.source_info(or_span); + + if candidate.false_edge_start_block.is_none() { + candidate.false_edge_start_block = candidate.subcandidates[0].false_edge_start_block; + } + + for subcandidate in mem::take(&mut candidate.subcandidates) { + let or_block = subcandidate.pre_binding_block.unwrap(); + self.cfg.goto(or_block, source_info, any_matches); + last_otherwise = subcandidate.otherwise_block; + } + candidate.pre_binding_block = Some(any_matches); + assert!(last_otherwise.is_some()); + candidate.otherwise_block = last_otherwise; + } + + /// Never subcandidates may have a set of bindings inconsistent with their siblings, + /// which would break later code. So we filter them out. Note that we can't filter out + /// top-level candidates this way. + fn remove_never_subcandidates(&mut self, candidate: &mut Candidate<'_, 'tcx>) { + if candidate.subcandidates.is_empty() { + return; + } + + candidate.subcandidates.retain_mut(|candidate| { + if candidate.extra_data.is_never { + candidate.visit_leaves(|subcandidate| { + let block = subcandidate.pre_binding_block.unwrap(); + // That block is already unreachable but needs a terminator to make the MIR well-formed. + let source_info = self.source_info(subcandidate.extra_data.span); + self.cfg.terminate(block, source_info, TerminatorKind::Unreachable); + }); + false + } else { + true } + }); + if candidate.subcandidates.is_empty() { + // If `candidate` has become a leaf candidate, ensure it has a `pre_binding_block`. + candidate.pre_binding_block = Some(self.cfg.start_new_block()); } } From e091c356fa0e47251f6c7d11cbc7043213df48c6 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 18 Jul 2024 20:58:52 +1000 Subject: [PATCH 25/29] Improve `merge_trivial_subcandidates` --- .../rustc_mir_build/src/build/matches/mod.rs | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 6bd2c308128ad..d18e59759e203 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1744,8 +1744,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// Try to merge all of the subcandidates of the given candidate into one. This avoids /// exponentially large CFGs in cases like `(1 | 2, 3 | 4, ...)`. The candidate should have been /// expanded with `create_or_subcandidates`. + /// + /// Note that this takes place _after_ the subcandidates have participated + /// in match tree lowering. fn merge_trivial_subcandidates(&mut self, candidate: &mut Candidate<'_, 'tcx>) { - if candidate.subcandidates.is_empty() || candidate.has_guard { + assert!(!candidate.subcandidates.is_empty()); + if candidate.has_guard { // FIXME(or_patterns; matthewjasper) Don't give up if we have a guard. return; } @@ -1759,7 +1763,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } let mut last_otherwise = None; - let any_matches = self.cfg.start_new_block(); + let shared_pre_binding_block = self.cfg.start_new_block(); + // This candidate is about to become a leaf, so unset `or_span`. let or_span = candidate.or_span.take().unwrap(); let source_info = self.source_info(or_span); @@ -1767,12 +1772,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate.false_edge_start_block = candidate.subcandidates[0].false_edge_start_block; } + // Remove the (known-trivial) subcandidates from the candidate tree, + // so that they aren't visible after match tree lowering, and wire them + // all to join up at a single shared pre-binding block. + // (Note that the subcandidates have already had their part of the match + // tree lowered by this point, which is why we can add a goto to them.) for subcandidate in mem::take(&mut candidate.subcandidates) { - let or_block = subcandidate.pre_binding_block.unwrap(); - self.cfg.goto(or_block, source_info, any_matches); + let subcandidate_block = subcandidate.pre_binding_block.unwrap(); + self.cfg.goto(subcandidate_block, source_info, shared_pre_binding_block); last_otherwise = subcandidate.otherwise_block; } - candidate.pre_binding_block = Some(any_matches); + candidate.pre_binding_block = Some(shared_pre_binding_block); assert!(last_otherwise.is_some()); candidate.otherwise_block = last_otherwise; } From e60c5c1a77a4bc6a2540437969ec0dc6e213d712 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 18 Jul 2024 22:11:09 +1000 Subject: [PATCH 26/29] Split out `test_remaining_match_pairs_after_or` Only the last candidate can possibly have more match pairs, so this can be separate from the main or-candidate postprocessing loop. --- .../rustc_mir_build/src/build/matches/mod.rs | 101 ++++++++++-------- 1 file changed, 58 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index d18e59759e203..8bba932908514 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1598,6 +1598,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { for subcandidate in candidate.subcandidates.iter_mut() { expanded_candidates.push(subcandidate); } + // Note that the subcandidates have been added to `expanded_candidates`, + // but `candidate` itself has not. If the last candidate has more match pairs, + // they are handled separately by `test_remaining_match_pairs_after_or`. } else { // A candidate that doesn't start with an or-pattern has nothing to // expand, so it is included in the post-expansion list as-is. @@ -1613,12 +1616,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { expanded_candidates.as_mut_slice(), ); - // Simplify subcandidates and process any leftover match pairs. - for candidate in candidates_to_expand { + // Postprocess subcandidates, and process any leftover match pairs. + // (Only the last candidate can possibly have more match pairs.) + debug_assert!({ + let mut all_except_last = candidates_to_expand.iter().rev().skip(1); + all_except_last.all(|candidate| candidate.match_pairs.is_empty()) + }); + for candidate in candidates_to_expand.iter_mut() { if !candidate.subcandidates.is_empty() { - self.finalize_or_candidate(span, scrutinee_span, candidate); + self.finalize_or_candidate(candidate); } } + if let Some(last_candidate) = candidates_to_expand.last_mut() { + self.test_remaining_match_pairs_after_or(span, scrutinee_span, last_candidate); + } remainder_start.and(remaining_candidates) } @@ -1642,8 +1653,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate.subcandidates[0].false_edge_start_block = candidate.false_edge_start_block; } - /// Simplify subcandidates and process any leftover match pairs. The candidate should have been - /// expanded with `create_or_subcandidates`. + /// Simplify subcandidates and remove `is_never` subcandidates. + /// The candidate should have been expanded with `create_or_subcandidates`. /// /// Given a pattern `(P | Q, R | S)` we (in principle) generate a CFG like /// so: @@ -1695,50 +1706,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// | /// ... /// ``` - fn finalize_or_candidate( - &mut self, - span: Span, - scrutinee_span: Span, - candidate: &mut Candidate<'_, 'tcx>, - ) { + fn finalize_or_candidate(&mut self, candidate: &mut Candidate<'_, 'tcx>) { if candidate.subcandidates.is_empty() { return; } self.merge_trivial_subcandidates(candidate); self.remove_never_subcandidates(candidate); - - if !candidate.match_pairs.is_empty() { - let or_span = candidate.or_span.unwrap_or(candidate.extra_data.span); - let source_info = self.source_info(or_span); - // If more match pairs remain, test them after each subcandidate. - // We could add them to the or-candidates before the call to `test_or_pattern` but this - // would make it impossible to detect simplifiable or-patterns. That would guarantee - // exponentially large CFGs for cases like `(1 | 2, 3 | 4, ...)`. - let mut last_otherwise = None; - candidate.visit_leaves(|leaf_candidate| { - last_otherwise = leaf_candidate.otherwise_block; - }); - let remaining_match_pairs = mem::take(&mut candidate.match_pairs); - candidate.visit_leaves(|leaf_candidate| { - assert!(leaf_candidate.match_pairs.is_empty()); - leaf_candidate.match_pairs.extend(remaining_match_pairs.iter().cloned()); - let or_start = leaf_candidate.pre_binding_block.unwrap(); - let otherwise = - self.match_candidates(span, scrutinee_span, or_start, &mut [leaf_candidate]); - // In a case like `(P | Q, R | S)`, if `P` succeeds and `R | S` fails, we know `(Q, - // R | S)` will fail too. If there is no guard, we skip testing of `Q` by branching - // directly to `last_otherwise`. If there is a guard, - // `leaf_candidate.otherwise_block` can be reached by guard failure as well, so we - // can't skip `Q`. - let or_otherwise = if leaf_candidate.has_guard { - leaf_candidate.otherwise_block.unwrap() - } else { - last_otherwise.unwrap() - }; - self.cfg.goto(otherwise, source_info, or_otherwise); - }); - } } /// Try to merge all of the subcandidates of the given candidate into one. This avoids @@ -1814,6 +1788,47 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } + /// If more match pairs remain, test them after each subcandidate. + /// We could have added them to the or-candidates during or-pattern expansion, but that + /// would make it impossible to detect simplifiable or-patterns. That would guarantee + /// exponentially large CFGs for cases like `(1 | 2, 3 | 4, ...)`. + fn test_remaining_match_pairs_after_or( + &mut self, + span: Span, + scrutinee_span: Span, + candidate: &mut Candidate<'_, 'tcx>, + ) { + if candidate.match_pairs.is_empty() { + return; + } + + let or_span = candidate.or_span.unwrap_or(candidate.extra_data.span); + let source_info = self.source_info(or_span); + let mut last_otherwise = None; + candidate.visit_leaves(|leaf_candidate| { + last_otherwise = leaf_candidate.otherwise_block; + }); + let remaining_match_pairs = mem::take(&mut candidate.match_pairs); + candidate.visit_leaves(|leaf_candidate| { + assert!(leaf_candidate.match_pairs.is_empty()); + leaf_candidate.match_pairs.extend(remaining_match_pairs.iter().cloned()); + let or_start = leaf_candidate.pre_binding_block.unwrap(); + let otherwise = + self.match_candidates(span, scrutinee_span, or_start, &mut [leaf_candidate]); + // In a case like `(P | Q, R | S)`, if `P` succeeds and `R | S` fails, we know `(Q, + // R | S)` will fail too. If there is no guard, we skip testing of `Q` by branching + // directly to `last_otherwise`. If there is a guard, + // `leaf_candidate.otherwise_block` can be reached by guard failure as well, so we + // can't skip `Q`. + let or_otherwise = if leaf_candidate.has_guard { + leaf_candidate.otherwise_block.unwrap() + } else { + last_otherwise.unwrap() + }; + self.cfg.goto(otherwise, source_info, or_otherwise); + }); + } + /// Pick a test to run. Which test doesn't matter as long as it is guaranteed to fully match at /// least one match pair. We currently simply pick the test corresponding to the first match /// pair of the first candidate in the list. From 886668cc2ea22a3d52f775961bac06d9499d9e26 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 18 Jul 2024 22:34:49 +1000 Subject: [PATCH 27/29] Improve `test_remaining_match_pairs_after_or` --- compiler/rustc_mir_build/src/build/matches/mod.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 8bba932908514..d8db08bfb8af5 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1808,10 +1808,23 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate.visit_leaves(|leaf_candidate| { last_otherwise = leaf_candidate.otherwise_block; }); + let remaining_match_pairs = mem::take(&mut candidate.match_pairs); + // We're testing match pairs that remained after an `Or`, so the remaining + // pairs should all be `Or` too, due to the sorting invariant. + debug_assert!( + remaining_match_pairs + .iter() + .all(|match_pair| matches!(match_pair.test_case, TestCase::Or { .. })) + ); + candidate.visit_leaves(|leaf_candidate| { + // At this point the leaf's own match pairs have all been lowered + // and removed, so `extend` and assignment are equivalent, + // but extending can also recycle any existing vector capacity. assert!(leaf_candidate.match_pairs.is_empty()); leaf_candidate.match_pairs.extend(remaining_match_pairs.iter().cloned()); + let or_start = leaf_candidate.pre_binding_block.unwrap(); let otherwise = self.match_candidates(span, scrutinee_span, or_start, &mut [leaf_candidate]); From 239037ecde3d884ad09bfb95c950021f3bc78da1 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 18 Jul 2024 22:51:17 +1000 Subject: [PATCH 28/29] Inline `finalize_or_candidate` --- .../rustc_mir_build/src/build/matches/mod.rs | 24 ++++++------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index d8db08bfb8af5..db7aed4306b20 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1624,7 +1624,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }); for candidate in candidates_to_expand.iter_mut() { if !candidate.subcandidates.is_empty() { - self.finalize_or_candidate(candidate); + self.merge_trivial_subcandidates(candidate); + self.remove_never_subcandidates(candidate); } } if let Some(last_candidate) = candidates_to_expand.last_mut() { @@ -1635,8 +1636,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } /// Given a match-pair that corresponds to an or-pattern, expand each subpattern into a new - /// subcandidate. Any candidate that has been expanded that way should be passed to - /// `finalize_or_candidate` after its subcandidates have been processed. + /// subcandidate. Any candidate that has been expanded this way should also be postprocessed + /// at the end of [`Self::expand_and_match_or_candidates`]. fn create_or_subcandidates<'pat>( &mut self, candidate: &mut Candidate<'pat, 'tcx>, @@ -1653,8 +1654,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate.subcandidates[0].false_edge_start_block = candidate.false_edge_start_block; } - /// Simplify subcandidates and remove `is_never` subcandidates. - /// The candidate should have been expanded with `create_or_subcandidates`. + /// Try to merge all of the subcandidates of the given candidate into one. This avoids + /// exponentially large CFGs in cases like `(1 | 2, 3 | 4, ...)`. The candidate should have been + /// expanded with `create_or_subcandidates`. /// /// Given a pattern `(P | Q, R | S)` we (in principle) generate a CFG like /// so: @@ -1706,18 +1708,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// | /// ... /// ``` - fn finalize_or_candidate(&mut self, candidate: &mut Candidate<'_, 'tcx>) { - if candidate.subcandidates.is_empty() { - return; - } - - self.merge_trivial_subcandidates(candidate); - self.remove_never_subcandidates(candidate); - } - - /// Try to merge all of the subcandidates of the given candidate into one. This avoids - /// exponentially large CFGs in cases like `(1 | 2, 3 | 4, ...)`. The candidate should have been - /// expanded with `create_or_subcandidates`. /// /// Note that this takes place _after_ the subcandidates have participated /// in match tree lowering. From 0636293fb740c96e7a52cf53452b728b1cf9e834 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Sun, 7 Jul 2024 19:02:32 +0300 Subject: [PATCH 29/29] use precompiled rustdoc with CI rustc When CI rustc is enabled and rustdoc sources are unchanged, we can use the precompiled rustdoc from the CI rustc's sysroot. This speeds up bootstrapping quite a lot by avoiding unnecessary rustdoc compilation. Signed-off-by: onur-ozkan --- src/bootstrap/src/core/build_steps/tool.rs | 59 +++++++++++++++++++--- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 7bc410b9e8878..459e59e1b3446 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -9,7 +9,7 @@ use crate::core::builder::{Builder, Cargo as CargoCommand, RunConfig, ShouldRun, use crate::core::config::TargetSelection; use crate::utils::channel::GitInfo; use crate::utils::exec::{command, BootstrapCommand}; -use crate::utils::helpers::{add_dylib_path, exe, t}; +use crate::utils::helpers::{add_dylib_path, exe, get_closest_merge_base_commit, git, t}; use crate::Compiler; use crate::Mode; use crate::{gha, Kind}; @@ -554,6 +554,57 @@ impl Step for Rustdoc { } let target = target_compiler.host; + let bin_rustdoc = || { + let sysroot = builder.sysroot(target_compiler); + let bindir = sysroot.join("bin"); + t!(fs::create_dir_all(&bindir)); + let bin_rustdoc = bindir.join(exe("rustdoc", target_compiler.host)); + let _ = fs::remove_file(&bin_rustdoc); + bin_rustdoc + }; + + // If CI rustc is enabled and we haven't modified the rustdoc sources, + // use the precompiled rustdoc from CI rustc's sysroot to speed up bootstrapping. + if builder.download_rustc() + && target_compiler.stage > 0 + && builder.rust_info().is_managed_git_subrepository() + { + let commit = get_closest_merge_base_commit( + Some(&builder.config.src), + &builder.config.git_config(), + &builder.config.stage0_metadata.config.git_merge_commit_email, + &[], + ) + .unwrap(); + + let librustdoc_src = builder.config.src.join("src/librustdoc"); + let rustdoc_src = builder.config.src.join("src/tools/rustdoc"); + + // FIXME: The change detection logic here is quite similar to `Config::download_ci_rustc_commit`. + // It would be better to unify them. + let has_changes = !git(Some(&builder.config.src)) + .allow_failure() + .run_always() + .args(["diff-index", "--quiet", &commit]) + .arg("--") + .arg(librustdoc_src) + .arg(rustdoc_src) + .run(builder) + .is_success(); + + if !has_changes { + let precompiled_rustdoc = builder + .config + .ci_rustc_dir() + .join("bin") + .join(exe("rustdoc", target_compiler.host)); + + let bin_rustdoc = bin_rustdoc(); + builder.copy_link(&precompiled_rustdoc, &bin_rustdoc); + return bin_rustdoc; + } + } + let build_compiler = if builder.download_rustc() && target_compiler.stage == 1 { // We already have the stage 1 compiler, we don't need to cut the stage. builder.compiler(target_compiler.stage, builder.config.build) @@ -614,11 +665,7 @@ impl Step for Rustdoc { // don't create a stage0-sysroot/bin directory. if target_compiler.stage > 0 { - let sysroot = builder.sysroot(target_compiler); - let bindir = sysroot.join("bin"); - t!(fs::create_dir_all(&bindir)); - let bin_rustdoc = bindir.join(exe("rustdoc", target_compiler.host)); - let _ = fs::remove_file(&bin_rustdoc); + let bin_rustdoc = bin_rustdoc(); builder.copy_link(&tool_rustdoc, &bin_rustdoc); bin_rustdoc } else {