Skip to content

Commit

Permalink
Auto merge of rust-lang#120303 - fmease:rollup-to11ql5, r=fmease
Browse files Browse the repository at this point in the history
Rollup of 10 pull requests

Successful merges:

 - rust-lang#114764 ([style edition 2024] Combine all delimited exprs as last argument)
 - rust-lang#118326 (Add `NonZero*::count_ones`)
 - rust-lang#118636 (Add the unstable option  to reduce the binary size of dynamic library…)
 - rust-lang#119460 (coverage: Never emit improperly-ordered coverage regions)
 - rust-lang#119616 (Add a new `wasm32-wasi-preview2` target)
 - rust-lang#120185 (coverage: Don't instrument `#[automatically_derived]` functions)
 - rust-lang#120265 (Remove no-system-llvm)
 - rust-lang#120284 (privacy: Refactor top-level visiting in `TypePrivacyVisitor`)
 - rust-lang#120285 (Remove extra # from url in suggestion)
 - rust-lang#120299 (Add mw to review rotation and add some owner assignments)

Failed merges:

 - rust-lang#120124 (Split assembly tests for ELF and MachO)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Jan 24, 2024
2 parents cd6d8f2 + f79440e commit fe158e6
Show file tree
Hide file tree
Showing 79 changed files with 824 additions and 457 deletions.
46 changes: 45 additions & 1 deletion compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ fn make_code_region(
start_line = source_map.doctest_offset_line(&file.name, start_line);
end_line = source_map.doctest_offset_line(&file.name, end_line);

Some(CodeRegion {
check_code_region(CodeRegion {
file_name,
start_line: start_line as u32,
start_col: start_col as u32,
Expand All @@ -338,6 +338,39 @@ fn make_code_region(
})
}

/// If `llvm-cov` sees a code region that is improperly ordered (end < start),
/// it will immediately exit with a fatal error. To prevent that from happening,
/// discard regions that are improperly ordered, or might be interpreted in a
/// way that makes them improperly ordered.
fn check_code_region(code_region: CodeRegion) -> Option<CodeRegion> {
let CodeRegion { file_name: _, start_line, start_col, end_line, end_col } = code_region;

// Line/column coordinates are supposed to be 1-based. If we ever emit
// coordinates of 0, `llvm-cov` might misinterpret them.
let all_nonzero = [start_line, start_col, end_line, end_col].into_iter().all(|x| x != 0);
// Coverage mappings use the high bit of `end_col` to indicate that a
// region is actually a "gap" region, so make sure it's unset.
let end_col_has_high_bit_unset = (end_col & (1 << 31)) == 0;
// If a region is improperly ordered (end < start), `llvm-cov` will exit
// with a fatal error, which is inconvenient for users and hard to debug.
let is_ordered = (start_line, start_col) <= (end_line, end_col);

if all_nonzero && end_col_has_high_bit_unset && is_ordered {
Some(code_region)
} else {
debug!(
?code_region,
?all_nonzero,
?end_col_has_high_bit_unset,
?is_ordered,
"Skipping code region that would be misinterpreted or rejected by LLVM"
);
// If this happens in a debug build, ICE to make it easier to notice.
debug_assert!(false, "Improper code region: {code_region:?}");
None
}
}

fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
// Only instrument functions, methods, and closures (not constants since they are evaluated
// at compile time by Miri).
Expand All @@ -351,7 +384,18 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
return false;
}

// Don't instrument functions with `#[automatically_derived]` on their
// enclosing impl block, on the assumption that most users won't care about
// coverage for derived impls.
if let Some(impl_of) = tcx.impl_of_method(def_id.to_def_id())
&& tcx.is_automatically_derived(impl_of)
{
trace!("InstrumentCoverage skipped for {def_id:?} (automatically derived)");
return false;
}

if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::NO_COVERAGE) {
trace!("InstrumentCoverage skipped for {def_id:?} (`#[coverage(off)]`)");
return false;
}

Expand Down
90 changes: 27 additions & 63 deletions compiler/rustc_privacy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId, CRATE_DEF_ID};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{AssocItemKind, ForeignItemKind, ItemId, PatKind};
use rustc_middle::bug;
use rustc_middle::hir::nested_filter;
use rustc_middle::middle::privacy::{EffectiveVisibilities, EffectiveVisibility, Level};
use rustc_middle::query::Providers;
use rustc_middle::ty::GenericArgs;
use rustc_middle::ty::{self, Const, GenericParamDefKind};
use rustc_middle::ty::{TraitRef, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor};
use rustc_middle::{bug, span_bug};
use rustc_session::lint;
use rustc_span::hygiene::Transparency;
use rustc_span::symbol::{kw, sym, Ident};
Expand Down Expand Up @@ -1064,29 +1064,22 @@ impl<'tcx> Visitor<'tcx> for NamePrivacyVisitor<'tcx> {

struct TypePrivacyVisitor<'tcx> {
tcx: TyCtxt<'tcx>,
module_def_id: LocalModDefId,
maybe_typeck_results: Option<&'tcx ty::TypeckResults<'tcx>>,
current_item: LocalDefId,
span: Span,
}

impl<'tcx> TypePrivacyVisitor<'tcx> {
/// Gets the type-checking results for the current body.
/// As this will ICE if called outside bodies, only call when working with
/// `Expr` or `Pat` nodes (they are guaranteed to be found only in bodies).
#[track_caller]
fn typeck_results(&self) -> &'tcx ty::TypeckResults<'tcx> {
self.maybe_typeck_results
.expect("`TypePrivacyVisitor::typeck_results` called outside of body")
}

fn item_is_accessible(&self, did: DefId) -> bool {
self.tcx.visibility(did).is_accessible_from(self.current_item, self.tcx)
self.tcx.visibility(did).is_accessible_from(self.module_def_id, self.tcx)
}

// Take node-id of an expression or pattern and check its type for privacy.
fn check_expr_pat_type(&mut self, id: hir::HirId, span: Span) -> bool {
self.span = span;
let typeck_results = self.typeck_results();
let typeck_results = self
.maybe_typeck_results
.unwrap_or_else(|| span_bug!(span, "`hir::Expr` or `hir::Pat` outside of a body"));
let result: ControlFlow<()> = try {
self.visit(typeck_results.node_type(id))?;
self.visit(typeck_results.node_args(id))?;
Expand All @@ -1107,35 +1100,13 @@ impl<'tcx> TypePrivacyVisitor<'tcx> {
}

impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> {
type NestedFilter = nested_filter::All;

/// We want to visit items in the context of their containing
/// module and so forth, so supply a crate for doing a deep walk.
fn nested_visit_map(&mut self) -> Self::Map {
self.tcx.hir()
}

fn visit_mod(&mut self, _m: &'tcx hir::Mod<'tcx>, _s: Span, _n: hir::HirId) {
// Don't visit nested modules, since we run a separate visitor walk
// for each module in `effective_visibilities`
}

fn visit_nested_body(&mut self, body: hir::BodyId) {
fn visit_nested_body(&mut self, body_id: hir::BodyId) {
let old_maybe_typeck_results =
self.maybe_typeck_results.replace(self.tcx.typeck_body(body));
let body = self.tcx.hir().body(body);
self.visit_body(body);
self.maybe_typeck_results.replace(self.tcx.typeck_body(body_id));
self.visit_body(self.tcx.hir().body(body_id));
self.maybe_typeck_results = old_maybe_typeck_results;
}

fn visit_generic_arg(&mut self, generic_arg: &'tcx hir::GenericArg<'tcx>) {
match generic_arg {
hir::GenericArg::Type(t) => self.visit_ty(t),
hir::GenericArg::Infer(inf) => self.visit_infer(inf),
hir::GenericArg::Lifetime(_) | hir::GenericArg::Const(_) => {}
}
}

fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'tcx>) {
self.span = hir_ty.span;
if let Some(typeck_results) = self.maybe_typeck_results {
Expand Down Expand Up @@ -1163,19 +1134,19 @@ impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> {
return;
}
} else {
// We don't do anything for const infers here.
// FIXME: check types of const infers here.
}
} else {
bug!("visit_infer without typeck_results");
span_bug!(self.span, "`hir::InferArg` outside of a body");
}
intravisit::walk_inf(self, inf);
}

fn visit_trait_ref(&mut self, trait_ref: &'tcx hir::TraitRef<'tcx>) {
self.span = trait_ref.path.span;
if self.maybe_typeck_results.is_none() {
// Avoid calling `hir_trait_to_predicates` in bodies, it will ICE.
// The traits' privacy in bodies is already checked as a part of trait object types.
if self.maybe_typeck_results.is_some() {
// Privacy of traits in bodies is checked as a part of trait object types.
} else {
let bounds = rustc_hir_analysis::hir_trait_to_predicates(
self.tcx,
trait_ref,
Expand Down Expand Up @@ -1223,7 +1194,10 @@ impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> {
hir::ExprKind::MethodCall(segment, ..) => {
// Method calls have to be checked specially.
self.span = segment.ident.span;
if let Some(def_id) = self.typeck_results().type_dependent_def_id(expr.hir_id) {
let typeck_results = self
.maybe_typeck_results
.unwrap_or_else(|| span_bug!(self.span, "`hir::Expr` outside of a body"));
if let Some(def_id) = typeck_results.type_dependent_def_id(expr.hir_id) {
if self.visit(self.tcx.type_of(def_id).instantiate_identity()).is_break() {
return;
}
Expand Down Expand Up @@ -1251,9 +1225,13 @@ impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> {
Res::Def(kind, def_id) => Some((kind, def_id)),
_ => None,
},
hir::QPath::TypeRelative(..) | hir::QPath::LangItem(..) => self
.maybe_typeck_results
.and_then(|typeck_results| typeck_results.type_dependent_def(id)),
hir::QPath::TypeRelative(..) | hir::QPath::LangItem(..) => {
match self.maybe_typeck_results {
Some(typeck_results) => typeck_results.type_dependent_def(id),
// FIXME: Check type-relative associated types in signatures.
None => None,
}
}
};
let def = def.filter(|(kind, _)| {
matches!(
Expand Down Expand Up @@ -1307,15 +1285,6 @@ impl<'tcx> Visitor<'tcx> for TypePrivacyVisitor<'tcx> {

intravisit::walk_local(self, local);
}

// Check types in item interfaces.
fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) {
let orig_current_item = mem::replace(&mut self.current_item, item.owner_id.def_id);
let old_maybe_typeck_results = self.maybe_typeck_results.take();
intravisit::walk_item(self, item);
self.maybe_typeck_results = old_maybe_typeck_results;
self.current_item = orig_current_item;
}
}

impl<'tcx> DefIdVisitor<'tcx> for TypePrivacyVisitor<'tcx> {
Expand Down Expand Up @@ -1785,13 +1754,8 @@ fn check_mod_privacy(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) {

// Check privacy of explicitly written types and traits as well as
// inferred types of expressions and patterns.
let mut visitor = TypePrivacyVisitor {
tcx,
maybe_typeck_results: None,
current_item: module_def_id.to_local_def_id(),
span,
};
intravisit::walk_mod(&mut visitor, module, hir_id);
let mut visitor = TypePrivacyVisitor { tcx, module_def_id, maybe_typeck_results: None, span };
tcx.hir().visit_item_likes_in_module(module_def_id, &mut visitor);
}

fn effective_visibilities(tcx: TyCtxt<'_>, (): ()) -> &EffectiveVisibilities {
Expand Down
15 changes: 15 additions & 0 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ impl SwitchWithOptPath {
pub enum SymbolManglingVersion {
Legacy,
V0,
Hashed,
}

#[derive(Clone, Copy, Debug, PartialEq, Hash)]
Expand Down Expand Up @@ -2692,6 +2693,7 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M
match cg.symbol_mangling_version {
// Stable values:
None | Some(SymbolManglingVersion::V0) => {}

// Unstable values:
Some(SymbolManglingVersion::Legacy) => {
if !unstable_opts.unstable_options {
Expand All @@ -2700,6 +2702,13 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M
);
}
}
Some(SymbolManglingVersion::Hashed) => {
if !unstable_opts.unstable_options {
early_dcx.early_fatal(
"`-C symbol-mangling-version=hashed` requires `-Z unstable-options`",
);
}
}
}

// Check for unstable values of `-C instrument-coverage`.
Expand Down Expand Up @@ -2741,6 +2750,12 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M
);
}
Some(SymbolManglingVersion::V0) => {}
Some(SymbolManglingVersion::Hashed) => {
early_dcx.early_warn(
"-C instrument-coverage requires symbol mangling version `v0`, \
but `-C symbol-mangling-version=hashed` was specified",
);
}
}
}

Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,8 @@ mod desc {
pub const parse_switch_with_opt_path: &str =
"an optional path to the profiling data output directory";
pub const parse_merge_functions: &str = "one of: `disabled`, `trampolines`, or `aliases`";
pub const parse_symbol_mangling_version: &str = "either `legacy` or `v0` (RFC 2603)";
pub const parse_symbol_mangling_version: &str =
"one of: `legacy`, `v0` (RFC 2603), or `hashed`";
pub const parse_src_file_hash: &str = "either `md5` or `sha1`";
pub const parse_relocation_model: &str =
"one of supported relocation models (`rustc --print relocation-models`)";
Expand Down Expand Up @@ -1180,6 +1181,7 @@ mod parse {
*slot = match v {
Some("legacy") => Some(SymbolManglingVersion::Legacy),
Some("v0") => Some(SymbolManglingVersion::V0),
Some("hashed") => Some(SymbolManglingVersion::Hashed),
_ => return false,
};
true
Expand Down Expand Up @@ -1504,7 +1506,7 @@ options! {
"tell the linker which information to strip (`none` (default), `debuginfo` or `symbols`)"),
symbol_mangling_version: Option<SymbolManglingVersion> = (None,
parse_symbol_mangling_version, [TRACKED],
"which mangling version to use for symbol names ('legacy' (default) or 'v0')"),
"which mangling version to use for symbol names ('legacy' (default), 'v0', or 'hashed')"),
target_cpu: Option<String> = (None, parse_opt_string, [TRACKED],
"select target processor (`rustc --print target-cpus` for details)"),
target_feature: String = (String::new(), parse_target_feature, [TRACKED],
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1793,6 +1793,7 @@ symbols! {
warn,
wasm_abi,
wasm_import_module,
wasm_preview2,
wasm_target_feature,
while_let,
windows,
Expand Down
43 changes: 43 additions & 0 deletions compiler/rustc_symbol_mangling/src/hashed.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
use crate::v0;
use rustc_data_structures::stable_hasher::{Hash64, HashStable, StableHasher};
use rustc_hir::def_id::CrateNum;
use rustc_middle::ty::{Instance, TyCtxt};

use std::fmt::Write;

pub(super) fn mangle<'tcx>(
tcx: TyCtxt<'tcx>,
instance: Instance<'tcx>,
instantiating_crate: Option<CrateNum>,
full_mangling_name: impl FnOnce() -> String,
) -> String {
// The symbol of a generic function may be scattered in multiple downstream dylibs.
// If the symbol of a generic function still contains `crate name`, hash conflicts between the
// generic funcion and other symbols of the same `crate` cannot be detected in time during
// construction. This symbol conflict is left over until it occurs during run time.
// In this case, `instantiating-crate name` is used to replace `crate name` can completely
// eliminate the risk of the preceding potential hash conflict.
let crate_num =
if let Some(krate) = instantiating_crate { krate } else { instance.def_id().krate };

let mut symbol = "_RNxC".to_string();
v0::push_ident(tcx.crate_name(crate_num).as_str(), &mut symbol);

let hash = tcx.with_stable_hashing_context(|mut hcx| {
let mut hasher = StableHasher::new();
full_mangling_name().hash_stable(&mut hcx, &mut hasher);
hasher.finish::<Hash64>().as_u64()
});

push_hash64(hash, &mut symbol);

symbol
}

// The hash is encoded based on `base-62` and the final terminator `_` is removed because it does
// not help prevent hash collisions
fn push_hash64(hash: u64, output: &mut String) {
let hash = v0::encode_integer_62(hash);
let hash_len = hash.len();
let _ = write!(output, "{hash_len}H{}", &hash[..hash_len - 1]);
}
4 changes: 4 additions & 0 deletions compiler/rustc_symbol_mangling/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ use rustc_middle::query::Providers;
use rustc_middle::ty::{self, Instance, TyCtxt};
use rustc_session::config::SymbolManglingVersion;

mod hashed;
mod legacy;
mod v0;

Expand Down Expand Up @@ -265,6 +266,9 @@ fn compute_symbol_name<'tcx>(
let symbol = match mangling_version {
SymbolManglingVersion::Legacy => legacy::mangle(tcx, instance, instantiating_crate),
SymbolManglingVersion::V0 => v0::mangle(tcx, instance, instantiating_crate),
SymbolManglingVersion::Hashed => hashed::mangle(tcx, instance, instantiating_crate, || {
v0::mangle(tcx, instance, instantiating_crate)
}),
};

debug_assert!(
Expand Down
Loading

0 comments on commit fe158e6

Please sign in to comment.