Skip to content

Reduce special casing for the panic runtime #140809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/back/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1871,6 +1871,7 @@ pub(crate) fn linked_symbols(
for_each_exported_symbols_include_dep(tcx, crate_type, |symbol, info, cnum| {
if info.level.is_below_threshold(export_threshold) && !tcx.is_compiler_builtins(cnum)
|| info.used
|| info.rustc_std_internal_symbol
{
symbols.push((
symbol_export::linking_symbol_name_for_instance_in_crate(
Expand Down
17 changes: 17 additions & 0 deletions compiler/rustc_codegen_ssa/src/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ fn reachable_non_generics_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> DefIdMap<S
used: codegen_attrs.flags.contains(CodegenFnAttrFlags::USED)
|| codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER)
|| used,
rustc_std_internal_symbol: codegen_attrs
.flags
.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL),
};
(def_id.to_def_id(), info)
})
Expand All @@ -143,6 +146,7 @@ fn reachable_non_generics_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> DefIdMap<S
level: SymbolExportLevel::C,
kind: SymbolExportKind::Data,
used: false,
rustc_std_internal_symbol: false,
},
);
}
Expand Down Expand Up @@ -191,6 +195,7 @@ fn exported_symbols_provider_local<'tcx>(
level: info.level,
kind: SymbolExportKind::Text,
used: info.used,
rustc_std_internal_symbol: info.rustc_std_internal_symbol,
},
)
})
Expand All @@ -207,6 +212,7 @@ fn exported_symbols_provider_local<'tcx>(
level: SymbolExportLevel::C,
kind: SymbolExportKind::Text,
used: false,
rustc_std_internal_symbol: false,
},
));
}
Expand All @@ -229,6 +235,7 @@ fn exported_symbols_provider_local<'tcx>(
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Text,
used: false,
rustc_std_internal_symbol: true,
},
));
}
Expand All @@ -243,6 +250,7 @@ fn exported_symbols_provider_local<'tcx>(
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Data,
used: false,
rustc_std_internal_symbol: true,
},
))
}
Expand All @@ -262,6 +270,7 @@ fn exported_symbols_provider_local<'tcx>(
level: SymbolExportLevel::C,
kind: SymbolExportKind::Data,
used: false,
rustc_std_internal_symbol: false,
},
)
}));
Expand All @@ -287,6 +296,7 @@ fn exported_symbols_provider_local<'tcx>(
level: SymbolExportLevel::C,
kind: SymbolExportKind::Data,
used: false,
rustc_std_internal_symbol: false,
},
)
}));
Expand All @@ -304,6 +314,7 @@ fn exported_symbols_provider_local<'tcx>(
level: SymbolExportLevel::C,
kind: SymbolExportKind::Data,
used: true,
rustc_std_internal_symbol: false,
},
));
}
Expand Down Expand Up @@ -379,6 +390,8 @@ fn exported_symbols_provider_local<'tcx>(
}
}

// Note: These all set rustc_std_internal_symbol to false as generic functions must not
// be marked with this attribute and we are only handling generic functions here.
match *mono_item {
MonoItem::Fn(Instance { def: InstanceKind::Item(def), args }) => {
let has_generics = args.non_erasable_generics().next().is_some();
Expand All @@ -394,6 +407,7 @@ fn exported_symbols_provider_local<'tcx>(
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Text,
used: false,
rustc_std_internal_symbol: false,
},
));
}
Expand All @@ -416,6 +430,7 @@ fn exported_symbols_provider_local<'tcx>(
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Text,
used: false,
rustc_std_internal_symbol: false,
},
));
}
Expand All @@ -432,6 +447,7 @@ fn exported_symbols_provider_local<'tcx>(
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Text,
used: false,
rustc_std_internal_symbol: false,
},
));
}
Expand All @@ -442,6 +458,7 @@ fn exported_symbols_provider_local<'tcx>(
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Text,
used: false,
rustc_std_internal_symbol: false,
},
));
}
Expand Down
22 changes: 1 addition & 21 deletions compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::time::{Duration, Instant};
use itertools::Itertools;
use rustc_abi::FIRST_VARIANT;
use rustc_ast as ast;
use rustc_ast::expand::allocator::{ALLOCATOR_METHODS, AllocatorKind, global_fn_name};
use rustc_ast::expand::allocator::AllocatorKind;
use rustc_attr_parsing::OptimizeAttr;
use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
use rustc_data_structures::profiling::{get_resident_set_size, print_time_passes_entry};
Expand Down Expand Up @@ -1091,26 +1091,6 @@ impl CrateInfo {
.collect::<Vec<_>>();
symbols.sort_unstable_by(|a, b| a.0.cmp(&b.0));
linked_symbols.extend(symbols);
if tcx.allocator_kind(()).is_some() {
// At least one crate needs a global allocator. This crate may be placed
// after the crate that defines it in the linker order, in which case some
// linkers return an error. By adding the global allocator shim methods to
// the linked_symbols list, linking the generated symbols.o will ensure that
// circular dependencies involving the global allocator don't lead to linker
// errors.
linked_symbols.extend(ALLOCATOR_METHODS.iter().map(|method| {
(
add_prefix(
mangle_internal_symbol(
tcx,
global_fn_name(method.name).as_str(),
),
SymbolExportKind::Text,
),
SymbolExportKind::Text,
)
}));
}
});
}

Expand Down
91 changes: 9 additions & 82 deletions compiler/rustc_metadata/src/creader.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Validates all used crates and extern libraries and loads their metadata

use std::error::Error;
use std::ops::Fn;
use std::path::Path;
use std::str::FromStr;
use std::time::Duration;
Expand Down Expand Up @@ -275,12 +274,6 @@ impl CStore {
.filter_map(|(cnum, data)| data.as_deref().map(|data| (cnum, data)))
}

fn iter_crate_data_mut(&mut self) -> impl Iterator<Item = (CrateNum, &mut CrateMetadata)> {
self.metas
.iter_enumerated_mut()
.filter_map(|(cnum, data)| data.as_deref_mut().map(|data| (cnum, data)))
}

fn push_dependencies_in_postorder(&self, deps: &mut Vec<CrateNum>, cnum: CrateNum) {
if !deps.contains(&cnum) {
let data = self.get_crate_data(cnum);
Expand All @@ -306,12 +299,6 @@ impl CStore {
deps
}

fn crate_dependencies_in_reverse_postorder(&self, cnum: CrateNum) -> Vec<CrateNum> {
let mut deps = self.crate_dependencies_in_postorder(cnum);
deps.reverse();
deps
}

pub(crate) fn injected_panic_runtime(&self) -> Option<CrateNum> {
self.injected_panic_runtime
}
Expand Down Expand Up @@ -943,47 +930,27 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
// If we need a panic runtime, we try to find an existing one here. At
// the same time we perform some general validation of the DAG we've got
// going such as ensuring everything has a compatible panic strategy.
//
// The logic for finding the panic runtime here is pretty much the same
// as the allocator case with the only addition that the panic strategy
// compilation mode also comes into play.
let desired_strategy = self.sess.panic_strategy();
let mut runtime_found = false;
let mut needs_panic_runtime = attr::contains_name(&krate.attrs, sym::needs_panic_runtime);

let mut panic_runtimes = Vec::new();
for (cnum, data) in self.cstore.iter_crate_data() {
needs_panic_runtime = needs_panic_runtime || data.needs_panic_runtime();
if data.is_panic_runtime() {
// Inject a dependency from all #![needs_panic_runtime] to this
// #![panic_runtime] crate.
panic_runtimes.push(cnum);
runtime_found = runtime_found || data.dep_kind() == CrateDepKind::Explicit;
}
}
for cnum in panic_runtimes {
self.inject_dependency_if(cnum, "a panic runtime", &|data| data.needs_panic_runtime());
for (_cnum, data) in self.cstore.iter_crate_data() {
needs_panic_runtime |= data.needs_panic_runtime();
}

// If an explicitly linked and matching panic runtime was found, or if
// we just don't need one at all, then we're done here and there's
// nothing else to do.
if !needs_panic_runtime || runtime_found {
// If we just don't need a panic runtime at all, then we're done here
// and there's nothing else to do.
if !needs_panic_runtime {
return;
}

// By this point we know that we (a) need a panic runtime and (b) no
// panic runtime was explicitly linked. Here we just load an appropriate
// default runtime for our panic strategy and then inject the
// dependencies.
// By this point we know that we need a panic runtime. Here we just load
// an appropriate default runtime for our panic strategy.
//
// We may resolve to an already loaded crate (as the crate may not have
// been explicitly linked prior to this) and we may re-inject
// dependencies again, but both of those situations are fine.
// been explicitly linked prior to this), but this is fine.
//
// Also note that we have yet to perform validation of the crate graph
// in terms of everyone has a compatible panic runtime format, that's
// performed later as part of the `dependency_format` module.
let desired_strategy = self.sess.panic_strategy();
let name = match desired_strategy {
PanicStrategy::Unwind => sym::panic_unwind,
PanicStrategy::Abort => sym::panic_abort,
Expand All @@ -1008,7 +975,6 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
}

self.cstore.injected_panic_runtime = Some(cnum);
self.inject_dependency_if(cnum, "a panic runtime", &|data| data.needs_panic_runtime());
}

fn inject_profiler_runtime(&mut self) {
Expand Down Expand Up @@ -1189,45 +1155,6 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
}
}

fn inject_dependency_if(
&mut self,
krate: CrateNum,
what: &str,
needs_dep: &dyn Fn(&CrateMetadata) -> bool,
) {
// Don't perform this validation if the session has errors, as one of
// those errors may indicate a circular dependency which could cause
// this to stack overflow.
if self.dcx().has_errors().is_some() {
return;
}

// Before we inject any dependencies, make sure we don't inject a
// circular dependency by validating that this crate doesn't
// transitively depend on any crates satisfying `needs_dep`.
for dep in self.cstore.crate_dependencies_in_reverse_postorder(krate) {
let data = self.cstore.get_crate_data(dep);
if needs_dep(&data) {
self.dcx().emit_err(errors::NoTransitiveNeedsDep {
crate_name: self.cstore.get_crate_data(krate).name(),
needs_crate_name: what,
deps_crate_name: data.name(),
});
}
}

// All crates satisfying `needs_dep` do not explicitly depend on the
// crate provided for this compile, but in order for this compilation to
// be successfully linked we need to inject a dependency (to order the
// crates on the command line correctly).
for (cnum, data) in self.cstore.iter_crate_data_mut() {
if needs_dep(data) {
info!("injecting a dep from {} to {}", cnum, krate);
data.add_dependency(krate);
}
}
}

fn report_unused_deps(&mut self, krate: &ast::Crate) {
// Make a point span rather than covering the whole file
let span = krate.spans.inner_span.shrink_to_lo();
Expand Down
18 changes: 9 additions & 9 deletions compiler/rustc_metadata/src/dependency_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,15 +370,15 @@ fn attempt_static(tcx: TyCtxt<'_>, unavailable: &mut Vec<CrateNum>) -> Option<De
Some(ret)
}

// Given a list of how to link upstream dependencies so far, ensure that an
// injected dependency is activated. This will not do anything if one was
// transitively included already (e.g., via a dylib or explicitly so).
//
// If an injected dependency was not found then we're guaranteed the
// metadata::creader module has injected that dependency (not listed as
// a required dependency) in one of the session's field. If this field is not
// set then this compilation doesn't actually need the dependency and we can
// also skip this step entirely.
/// Given a list of how to link upstream dependencies so far, ensure that an
/// injected dependency is activated. This will not do anything if one was
/// transitively included already (e.g., via a dylib or explicitly so).
///
/// If an injected dependency was not found then we're guaranteed the
/// metadata::creader module has injected that dependency (not listed as
/// a required dependency) in one of the session's field. If this field is not
/// set then this compilation doesn't actually need the dependency and we can
/// also skip this step entirely.
fn activate_injected_dep(
injected: Option<CrateNum>,
list: &mut DependencyList,
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1895,10 +1895,6 @@ impl CrateMetadata {
self.dependencies.iter().copied()
}

pub(crate) fn add_dependency(&mut self, cnum: CrateNum) {
self.dependencies.push(cnum);
}

pub(crate) fn target_modifiers(&self) -> TargetModifiers {
self.root.decode_target_modifiers(&self.blob).collect()
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/middle/exported_symbols.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ pub enum SymbolExportKind {
pub struct SymbolExportInfo {
pub level: SymbolExportLevel,
pub kind: SymbolExportKind,
/// Was the symbol marked as `#[used(compiler)]` or `#[used(linker)]`?
pub used: bool,
/// Was the symbol marked as `#[rustc_std_internal_symbol]`?
pub rustc_std_internal_symbol: bool,
}

#[derive(Eq, PartialEq, Debug, Copy, Clone, TyEncodable, TyDecodable, HashStable)]
Expand Down
2 changes: 1 addition & 1 deletion library/std/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ backtrace = [
'miniz_oxide/rustc-dep-of-std',
]

panic-unwind = ["panic_unwind"]
panic-unwind = ["dep:panic_unwind"]
compiler-builtins-c = ["alloc/compiler-builtins-c"]
compiler-builtins-mem = ["alloc/compiler-builtins-mem"]
compiler-builtins-no-asm = ["alloc/compiler-builtins-no-asm"]
Expand Down
2 changes: 1 addition & 1 deletion library/sysroot/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ debug_typeid = ["std/debug_typeid"]
llvm-libunwind = ["std/llvm-libunwind"]
system-llvm-libunwind = ["std/system-llvm-libunwind"]
optimize_for_size = ["std/optimize_for_size"]
panic-unwind = ["std/panic_unwind"]
panic-unwind = ["std/panic-unwind"]
panic_immediate_abort = ["std/panic_immediate_abort"]
profiler = ["dep:profiler_builtins"]
std_detect_file_io = ["std/std_detect_file_io"]
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/cargo-miri/src/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub fn setup(
SysrootConfig::NoStd
} else {
SysrootConfig::WithStd {
std_features: ["panic_unwind", "backtrace"].into_iter().map(Into::into).collect(),
std_features: ["panic-unwind", "backtrace"].into_iter().map(Into::into).collect(),
}
};
let cargo_cmd = {
Expand Down
1 change: 1 addition & 0 deletions src/tools/miri/src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ impl rustc_driver::Callbacks for MiriBeRustCompilerCalls {
level: SymbolExportLevel::C,
kind: SymbolExportKind::Text,
used: false,
rustc_std_internal_symbol: false,
},
))
} else {
Expand Down
8 changes: 0 additions & 8 deletions tests/ui/panic-runtime/auxiliary/depends.rs

This file was deleted.

Loading
Loading