diff options
75 files changed, 1719 insertions, 809 deletions
diff --git a/Cargo.lock b/Cargo.lock index 34fc0860a4b..8135f56e8de 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2394,6 +2394,7 @@ dependencies = [ "regex", "rustc_version", "serde", + "serde_json", "smallvec", "tempfile", "tikv-jemalloc-sys", diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index 5dd6882b025..f8ecff69a76 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -1202,9 +1202,10 @@ macro_rules! common_visitor_and_walkers { let TyPat { id, kind, span, tokens: _ } = tp; try_visit!(visit_id(vis, id)); match kind { - TyPatKind::Range(start, end, _include_end) => { + TyPatKind::Range(start, end, Spanned { span, node: _include_end }) => { visit_opt!(vis, visit_anon_const, start); visit_opt!(vis, visit_anon_const, end); + try_visit!(visit_span(vis, span)); } TyPatKind::Or(variants) => walk_list!(vis, visit_ty_pat, variants), TyPatKind::Err(_) => {} @@ -1523,16 +1524,26 @@ macro_rules! common_visitor_and_walkers { } pub fn walk_inline_asm<$($lt,)? V: $Visitor$(<$lt>)?>(vis: &mut V, asm: &$($lt)? $($mut)? InlineAsm) -> V::Result { - // FIXME: Visit spans inside all this currently ignored stuff. let InlineAsm { asm_macro: _, - template: _, - template_strs: _, + template, + template_strs, operands, - clobber_abis: _, + clobber_abis, options: _, - line_spans: _, + line_spans, } = asm; + for piece in template { + match piece { + InlineAsmTemplatePiece::String(_str) => {} + InlineAsmTemplatePiece::Placeholder { operand_idx: _, modifier: _, span } => { + try_visit!(visit_span(vis, span)); + } + } + } + for (_s1, _s2, span) in template_strs { + try_visit!(visit_span(vis, span)); + } for (op, span) in operands { match op { InlineAsmOperand::In { expr, reg: _ } @@ -1553,6 +1564,12 @@ macro_rules! common_visitor_and_walkers { } try_visit!(visit_span(vis, span)); } + for (_s1, span) in clobber_abis { + try_visit!(visit_span(vis, span)) + } + for span in line_spans { + try_visit!(visit_span(vis, span)) + } V::Result::output() } @@ -1565,9 +1582,9 @@ macro_rules! common_visitor_and_walkers { vis.visit_path(path) } - // FIXME: visit the template exhaustively. pub fn walk_format_args<$($lt,)? V: $Visitor$(<$lt>)?>(vis: &mut V, fmt: &$($lt)? $($mut)? FormatArgs) -> V::Result { - let FormatArgs { span, template: _, arguments, uncooked_fmt_str: _, is_source_literal: _ } = fmt; + let FormatArgs { span, template, arguments, uncooked_fmt_str: _, is_source_literal: _ } = fmt; + let args = $(${ignore($mut)} arguments.all_args_mut())? $(${ignore($lt)} arguments.all_args())? ; for FormatArgument { kind, expr } in args { match kind { @@ -1578,9 +1595,58 @@ macro_rules! common_visitor_and_walkers { } try_visit!(vis.visit_expr(expr)); } + for piece in template { + match piece { + FormatArgsPiece::Literal(_symbol) => {} + FormatArgsPiece::Placeholder(placeholder) => try_visit!(walk_format_placeholder(vis, placeholder)), + } + } visit_span(vis, span) } + fn walk_format_placeholder<$($lt,)? V: $Visitor$(<$lt>)?>( + vis: &mut V, + placeholder: &$($lt)? $($mut)? FormatPlaceholder, + ) -> V::Result { + let FormatPlaceholder { argument, span, format_options, format_trait: _ } = placeholder; + if let Some(span) = span { + try_visit!(visit_span(vis, span)); + } + let FormatArgPosition { span, index: _, kind: _ } = argument; + if let Some(span) = span { + try_visit!(visit_span(vis, span)); + } + let FormatOptions { + width, + precision, + alignment: _, + fill: _, + sign: _, + alternate: _, + zero_pad: _, + debug_hex: _, + } = format_options; + match width { + None => {} + Some(FormatCount::Literal(_)) => {} + Some(FormatCount::Argument(FormatArgPosition { span, index: _, kind: _ })) => { + if let Some(span) = span { + try_visit!(visit_span(vis, span)); + } + } + } + match precision { + None => {} + Some(FormatCount::Literal(_)) => {} + Some(FormatCount::Argument(FormatArgPosition { span, index: _, kind: _ })) => { + if let Some(span) = span { + try_visit!(visit_span(vis, span)); + } + } + } + V::Result::output() + } + pub fn walk_expr<$($lt,)? V: $Visitor$(<$lt>)?>(vis: &mut V, expression: &$($lt)? $($mut)? Expr) -> V::Result { let Expr { id, kind, span, attrs, tokens: _ } = expression; try_visit!(visit_id(vis, id)); @@ -1601,7 +1667,7 @@ macro_rules! common_visitor_and_walkers { try_visit!(visit_expr_fields(vis, fields)); match rest { StructRest::Base(expr) => try_visit!(vis.visit_expr(expr)), - StructRest::Rest(_span) => {} + StructRest::Rest(span) => try_visit!(visit_span(vis, span)), StructRest::None => {} } } @@ -1688,7 +1754,8 @@ macro_rules! common_visitor_and_walkers { visit_opt!(vis, visit_label, opt_label); try_visit!(vis.visit_block(block)); } - ExprKind::Gen(_capt, body, _kind, decl_span) => { + ExprKind::Gen(capture_clause, body, _kind, decl_span) => { + try_visit!(vis.visit_capture_by(capture_clause)); try_visit!(vis.visit_block(body)); try_visit!(visit_span(vis, decl_span)); } @@ -1705,9 +1772,10 @@ macro_rules! common_visitor_and_walkers { try_visit!(vis.visit_expr(rhs)); try_visit!(visit_span(vis, span)); } - ExprKind::AssignOp(_op, left_expression, right_expression) => { + ExprKind::AssignOp(Spanned { span, node: _ }, left_expression, right_expression) => { try_visit!(vis.visit_expr(left_expression)); try_visit!(vis.visit_expr(right_expression)); + try_visit!(visit_span(vis, span)); } ExprKind::Field(subexpression, ident) => { try_visit!(vis.visit_expr(subexpression)); diff --git a/compiler/rustc_attr_data_structures/src/attributes.rs b/compiler/rustc_attr_data_structures/src/attributes.rs index b5934f4e36e..6af15da7d08 100644 --- a/compiler/rustc_attr_data_structures/src/attributes.rs +++ b/compiler/rustc_attr_data_structures/src/attributes.rs @@ -250,6 +250,13 @@ pub enum AttributeKind { span: Span, }, + /// Represents `#[ignore]` + Ignore { + span: Span, + /// ignore can optionally have a reason: `#[ignore = "reason this is ignored"]` + reason: Option<Symbol>, + }, + /// Represents `#[inline]` and `#[rustc_force_inline]`. Inline(InlineAttr, Span), diff --git a/compiler/rustc_attr_data_structures/src/encode_cross_crate.rs b/compiler/rustc_attr_data_structures/src/encode_cross_crate.rs index 02e95ddcb6f..8ebd38a6ba7 100644 --- a/compiler/rustc_attr_data_structures/src/encode_cross_crate.rs +++ b/compiler/rustc_attr_data_structures/src/encode_cross_crate.rs @@ -26,6 +26,7 @@ impl AttributeKind { Deprecation { .. } => Yes, DocComment { .. } => Yes, ExportName { .. } => Yes, + Ignore { .. } => No, Inline(..) => No, LinkName { .. } => Yes, LinkSection { .. } => No, diff --git a/compiler/rustc_attr_parsing/src/attributes/mod.rs b/compiler/rustc_attr_parsing/src/attributes/mod.rs index f5ac3890a46..55fbb825466 100644 --- a/compiler/rustc_attr_parsing/src/attributes/mod.rs +++ b/compiler/rustc_attr_parsing/src/attributes/mod.rs @@ -41,6 +41,7 @@ pub(crate) mod repr; pub(crate) mod rustc_internal; pub(crate) mod semantics; pub(crate) mod stability; +pub(crate) mod test_attrs; pub(crate) mod traits; pub(crate) mod transparency; pub(crate) mod util; diff --git a/compiler/rustc_attr_parsing/src/attributes/test_attrs.rs b/compiler/rustc_attr_parsing/src/attributes/test_attrs.rs new file mode 100644 index 00000000000..cea3ee52ff4 --- /dev/null +++ b/compiler/rustc_attr_parsing/src/attributes/test_attrs.rs @@ -0,0 +1,46 @@ +use rustc_attr_data_structures::AttributeKind; +use rustc_attr_data_structures::lints::AttributeLintKind; +use rustc_feature::{AttributeTemplate, template}; +use rustc_span::{Symbol, sym}; + +use crate::attributes::{AttributeOrder, OnDuplicate, SingleAttributeParser}; +use crate::context::{AcceptContext, Stage}; +use crate::parser::ArgParser; + +pub(crate) struct IgnoreParser; + +impl<S: Stage> SingleAttributeParser<S> for IgnoreParser { + const PATH: &[Symbol] = &[sym::ignore]; + const ATTRIBUTE_ORDER: AttributeOrder = AttributeOrder::KeepLast; + const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::Warn; + const TEMPLATE: AttributeTemplate = template!(Word, NameValueStr: "reason"); + + fn convert(cx: &mut AcceptContext<'_, '_, S>, args: &ArgParser<'_>) -> Option<AttributeKind> { + Some(AttributeKind::Ignore { + span: cx.attr_span, + reason: match args { + ArgParser::NoArgs => None, + ArgParser::NameValue(name_value) => { + let Some(str_value) = name_value.value_as_str() else { + let suggestions = <Self as SingleAttributeParser<S>>::TEMPLATE + .suggestions(false, "ignore"); + let span = cx.attr_span; + cx.emit_lint( + AttributeLintKind::IllFormedAttributeInput { suggestions }, + span, + ); + return None; + }; + Some(str_value) + } + ArgParser::List(_) => { + let suggestions = + <Self as SingleAttributeParser<S>>::TEMPLATE.suggestions(false, "ignore"); + let span = cx.attr_span; + cx.emit_lint(AttributeLintKind::IllFormedAttributeInput { suggestions }, span); + return None; + } + }, + }) + } +} diff --git a/compiler/rustc_attr_parsing/src/context.rs b/compiler/rustc_attr_parsing/src/context.rs index 265e1bb6a8c..2a01ee58493 100644 --- a/compiler/rustc_attr_parsing/src/context.rs +++ b/compiler/rustc_attr_parsing/src/context.rs @@ -37,6 +37,7 @@ use crate::attributes::semantics::MayDangleParser; use crate::attributes::stability::{ BodyStabilityParser, ConstStabilityIndirectParser, ConstStabilityParser, StabilityParser, }; +use crate::attributes::test_attrs::IgnoreParser; use crate::attributes::traits::SkipDuringMethodDispatchParser; use crate::attributes::transparency::TransparencyParser; use crate::attributes::{AttributeParser as _, Combine, Single, WithoutArgs}; @@ -126,6 +127,7 @@ attribute_parsers!( // tidy-alphabetical-start Single<DeprecationParser>, Single<ExportNameParser>, + Single<IgnoreParser>, Single<InlineParser>, Single<LinkNameParser>, Single<LinkSectionParser>, diff --git a/compiler/rustc_const_eval/src/const_eval/mod.rs b/compiler/rustc_const_eval/src/const_eval/mod.rs index d95d552d7d5..0082f90f3b8 100644 --- a/compiler/rustc_const_eval/src/const_eval/mod.rs +++ b/compiler/rustc_const_eval/src/const_eval/mod.rs @@ -1,9 +1,9 @@ // Not in interpret to make sure we do not use private implementation details use rustc_abi::{FieldIdx, VariantIdx}; -use rustc_middle::query::Key; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_middle::{bug, mir}; +use rustc_span::DUMMY_SP; use tracing::instrument; use crate::interpret::InterpCx; @@ -71,8 +71,7 @@ pub fn tag_for_variant_provider<'tcx>( let (ty, variant_index) = key.value; assert!(ty.is_enum()); - let ecx = - InterpCx::new(tcx, ty.default_span(tcx), key.typing_env, crate::const_eval::DummyMachine); + let ecx = InterpCx::new(tcx, DUMMY_SP, key.typing_env, crate::const_eval::DummyMachine); let layout = ecx.layout_of(ty).unwrap(); ecx.tag_for_variant(layout, variant_index).unwrap().map(|(tag, _tag_field)| tag) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index ca6405ea209..559a771931e 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1303,6 +1303,7 @@ impl AttributeExt for Attribute { Attribute::Parsed(AttributeKind::Deprecation { span, .. }) => *span, Attribute::Parsed(AttributeKind::DocComment { span, .. }) => *span, Attribute::Parsed(AttributeKind::MayDangle(span)) => *span, + Attribute::Parsed(AttributeKind::Ignore { span, .. }) => *span, a => panic!("can't get the span of an arbitrary parsed attribute: {a:?}"), } } diff --git a/compiler/rustc_hir_analysis/src/check/region.rs b/compiler/rustc_hir_analysis/src/check/region.rs index c458878da15..288105dfba7 100644 --- a/compiler/rustc_hir_analysis/src/check/region.rs +++ b/compiler/rustc_hir_analysis/src/check/region.rs @@ -15,7 +15,6 @@ use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::{Arm, Block, Expr, LetStmt, Pat, PatKind, Stmt}; use rustc_index::Idx; -use rustc_middle::bug; use rustc_middle::middle::region::*; use rustc_middle::ty::TyCtxt; use rustc_session::lint; @@ -34,14 +33,6 @@ struct Context { struct ScopeResolutionVisitor<'tcx> { tcx: TyCtxt<'tcx>, - // The number of expressions and patterns visited in the current body. - expr_and_pat_count: usize, - // When this is `true`, we record the `Scopes` we encounter - // when processing a Yield expression. This allows us to fix - // up their indices. - pessimistic_yield: bool, - // Stores scopes when `pessimistic_yield` is `true`. - fixup_scopes: Vec<Scope>, // The generated scope tree. scope_tree: ScopeTree, @@ -199,19 +190,14 @@ fn resolve_arm<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, arm: &'tcx hir: visitor.cx = prev_cx; } +#[tracing::instrument(level = "debug", skip(visitor))] fn resolve_pat<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, pat: &'tcx hir::Pat<'tcx>) { // If this is a binding then record the lifetime of that binding. if let PatKind::Binding(..) = pat.kind { record_var_lifetime(visitor, pat.hir_id.local_id); } - debug!("resolve_pat - pre-increment {} pat = {:?}", visitor.expr_and_pat_count, pat); - intravisit::walk_pat(visitor, pat); - - visitor.expr_and_pat_count += 1; - - debug!("resolve_pat - post-increment {} pat = {:?}", visitor.expr_and_pat_count, pat); } fn resolve_stmt<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, stmt: &'tcx hir::Stmt<'tcx>) { @@ -243,68 +229,15 @@ fn resolve_stmt<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, stmt: &'tcx hi } } +#[tracing::instrument(level = "debug", skip(visitor))] fn resolve_expr<'tcx>( visitor: &mut ScopeResolutionVisitor<'tcx>, expr: &'tcx hir::Expr<'tcx>, terminating: bool, ) { - debug!("resolve_expr - pre-increment {} expr = {:?}", visitor.expr_and_pat_count, expr); - let prev_cx = visitor.cx; visitor.enter_node_scope_with_dtor(expr.hir_id.local_id, terminating); - let prev_pessimistic = visitor.pessimistic_yield; - - // Ordinarily, we can rely on the visit order of HIR intravisit - // to correspond to the actual execution order of statements. - // However, there's a weird corner case with compound assignment - // operators (e.g. `a += b`). The evaluation order depends on whether - // or not the operator is overloaded (e.g. whether or not a trait - // like AddAssign is implemented). - - // For primitive types (which, despite having a trait impl, don't actually - // end up calling it), the evaluation order is right-to-left. For example, - // the following code snippet: - // - // let y = &mut 0; - // *{println!("LHS!"); y} += {println!("RHS!"); 1}; - // - // will print: - // - // RHS! - // LHS! - // - // However, if the operator is used on a non-primitive type, - // the evaluation order will be left-to-right, since the operator - // actually get desugared to a method call. For example, this - // nearly identical code snippet: - // - // let y = &mut String::new(); - // *{println!("LHS String"); y} += {println!("RHS String"); "hi"}; - // - // will print: - // LHS String - // RHS String - // - // To determine the actual execution order, we need to perform - // trait resolution. Unfortunately, we need to be able to compute - // yield_in_scope before type checking is even done, as it gets - // used by AST borrowcheck. - // - // Fortunately, we don't need to know the actual execution order. - // It suffices to know the 'worst case' order with respect to yields. - // Specifically, we need to know the highest 'expr_and_pat_count' - // that we could assign to the yield expression. To do this, - // we pick the greater of the two values from the left-hand - // and right-hand expressions. This makes us overly conservative - // about what types could possibly live across yield points, - // but we will never fail to detect that a type does actually - // live across a yield point. The latter part is critical - - // we're already overly conservative about what types will live - // across yield points, as the generated MIR will determine - // when things are actually live. However, for typecheck to work - // properly, we can't miss any types. - match expr.kind { // Conditional or repeating scopes are always terminating // scopes, meaning that temporaries cannot outlive them. @@ -360,55 +293,42 @@ fn resolve_expr<'tcx>( let body = visitor.tcx.hir_body(body); visitor.visit_body(body); } + // Ordinarily, we can rely on the visit order of HIR intravisit + // to correspond to the actual execution order of statements. + // However, there's a weird corner case with compound assignment + // operators (e.g. `a += b`). The evaluation order depends on whether + // or not the operator is overloaded (e.g. whether or not a trait + // like AddAssign is implemented). + // + // For primitive types (which, despite having a trait impl, don't actually + // end up calling it), the evaluation order is right-to-left. For example, + // the following code snippet: + // + // let y = &mut 0; + // *{println!("LHS!"); y} += {println!("RHS!"); 1}; + // + // will print: + // + // RHS! + // LHS! + // + // However, if the operator is used on a non-primitive type, + // the evaluation order will be left-to-right, since the operator + // actually get desugared to a method call. For example, this + // nearly identical code snippet: + // + // let y = &mut String::new(); + // *{println!("LHS String"); y} += {println!("RHS String"); "hi"}; + // + // will print: + // LHS String + // RHS String + // + // To determine the actual execution order, we need to perform + // trait resolution. Fortunately, we don't need to know the actual execution order. hir::ExprKind::AssignOp(_, left_expr, right_expr) => { - debug!( - "resolve_expr - enabling pessimistic_yield, was previously {}", - prev_pessimistic - ); - - let start_point = visitor.fixup_scopes.len(); - visitor.pessimistic_yield = true; - - // If the actual execution order turns out to be right-to-left, - // then we're fine. However, if the actual execution order is left-to-right, - // then we'll assign too low a count to any `yield` expressions - // we encounter in 'right_expression' - they should really occur after all of the - // expressions in 'left_expression'. visitor.visit_expr(right_expr); - visitor.pessimistic_yield = prev_pessimistic; - - debug!("resolve_expr - restoring pessimistic_yield to {}", prev_pessimistic); visitor.visit_expr(left_expr); - debug!("resolve_expr - fixing up counts to {}", visitor.expr_and_pat_count); - - // Remove and process any scopes pushed by the visitor - let target_scopes = visitor.fixup_scopes.drain(start_point..); - - for scope in target_scopes { - let yield_data = - visitor.scope_tree.yield_in_scope.get_mut(&scope).unwrap().last_mut().unwrap(); - let count = yield_data.expr_and_pat_count; - let span = yield_data.span; - - // expr_and_pat_count never decreases. Since we recorded counts in yield_in_scope - // before walking the left-hand side, it should be impossible for the recorded - // count to be greater than the left-hand side count. - if count > visitor.expr_and_pat_count { - bug!( - "Encountered greater count {} at span {:?} - expected no greater than {}", - count, - span, - visitor.expr_and_pat_count - ); - } - let new_count = visitor.expr_and_pat_count; - debug!( - "resolve_expr - increasing count for scope {:?} from {} to {} at span {:?}", - scope, count, new_count, span - ); - - yield_data.expr_and_pat_count = new_count; - } } hir::ExprKind::If(cond, then, Some(otherwise)) => { @@ -453,43 +373,6 @@ fn resolve_expr<'tcx>( _ => intravisit::walk_expr(visitor, expr), } - visitor.expr_and_pat_count += 1; - - debug!("resolve_expr post-increment {}, expr = {:?}", visitor.expr_and_pat_count, expr); - - if let hir::ExprKind::Yield(_, source) = &expr.kind { - // Mark this expr's scope and all parent scopes as containing `yield`. - let mut scope = Scope { local_id: expr.hir_id.local_id, data: ScopeData::Node }; - loop { - let data = YieldData { - span: expr.span, - expr_and_pat_count: visitor.expr_and_pat_count, - source: *source, - }; - match visitor.scope_tree.yield_in_scope.get_mut(&scope) { - Some(yields) => yields.push(data), - None => { - visitor.scope_tree.yield_in_scope.insert(scope, vec![data]); - } - } - - if visitor.pessimistic_yield { - debug!("resolve_expr in pessimistic_yield - marking scope {:?} for fixup", scope); - visitor.fixup_scopes.push(scope); - } - - // Keep traversing up while we can. - match visitor.scope_tree.parent_map.get(&scope) { - // Don't cross from closure bodies to their parent. - Some(&superscope) => match superscope.data { - ScopeData::CallSite => break, - _ => scope = superscope, - }, - None => break, - } - } - } - visitor.cx = prev_cx; } @@ -612,8 +495,8 @@ fn resolve_local<'tcx>( } } - // Make sure we visit the initializer first, so expr_and_pat_count remains correct. - // The correct order, as shared between coroutine_interior, drop_ranges and intravisitor, + // Make sure we visit the initializer first. + // The correct order, as shared between drop_ranges and intravisitor, // is to walk initializer, followed by pattern bindings, finally followed by the `else` block. if let Some(expr) = init { visitor.visit_expr(expr); @@ -798,16 +681,7 @@ impl<'tcx> ScopeResolutionVisitor<'tcx> { } fn enter_body(&mut self, hir_id: hir::HirId, f: impl FnOnce(&mut Self)) { - // Save all state that is specific to the outer function - // body. These will be restored once down below, once we've - // visited the body. - let outer_ec = mem::replace(&mut self.expr_and_pat_count, 0); let outer_cx = self.cx; - // The 'pessimistic yield' flag is set to true when we are - // processing a `+=` statement and have to make pessimistic - // control flow assumptions. This doesn't apply to nested - // bodies within the `+=` statements. See #69307. - let outer_pessimistic_yield = mem::replace(&mut self.pessimistic_yield, false); self.enter_scope(Scope { local_id: hir_id.local_id, data: ScopeData::CallSite }); self.enter_scope(Scope { local_id: hir_id.local_id, data: ScopeData::Arguments }); @@ -815,9 +689,7 @@ impl<'tcx> ScopeResolutionVisitor<'tcx> { f(self); // Restore context we had at the start. - self.expr_and_pat_count = outer_ec; self.cx = outer_cx; - self.pessimistic_yield = outer_pessimistic_yield; } } @@ -919,10 +791,7 @@ pub(crate) fn region_scope_tree(tcx: TyCtxt<'_>, def_id: DefId) -> &ScopeTree { let mut visitor = ScopeResolutionVisitor { tcx, scope_tree: ScopeTree::default(), - expr_and_pat_count: 0, cx: Context { parent: None, var_parent: None }, - pessimistic_yield: false, - fixup_scopes: vec![], extended_super_lets: Default::default(), }; diff --git a/compiler/rustc_lint/src/map_unit_fn.rs b/compiler/rustc_lint/src/map_unit_fn.rs index 3b27e456136..af509cb786d 100644 --- a/compiler/rustc_lint/src/map_unit_fn.rs +++ b/compiler/rustc_lint/src/map_unit_fn.rs @@ -1,5 +1,4 @@ use rustc_hir::{Expr, ExprKind, HirId, Stmt, StmtKind}; -use rustc_middle::query::Key; use rustc_middle::ty::{self, Ty}; use rustc_session::{declare_lint, declare_lint_pass}; @@ -69,7 +68,7 @@ impl<'tcx> LateLintPass<'tcx> for MapUnitFn { .span_of_impl(*id) .unwrap_or(default_span), argument_label: args[0].span, - map_label: arg_ty.default_span(cx.tcx), + map_label: span, suggestion: path.ident.span, replace: "for_each".to_string(), }, @@ -88,7 +87,7 @@ impl<'tcx> LateLintPass<'tcx> for MapUnitFn { .span_of_impl(*id) .unwrap_or(default_span), argument_label: args[0].span, - map_label: arg_ty.default_span(cx.tcx), + map_label: span, suggestion: path.ident.span, replace: "for_each".to_string(), }, diff --git a/compiler/rustc_middle/src/middle/region.rs b/compiler/rustc_middle/src/middle/region.rs index 92eab59dd02..0f5b63f5c1d 100644 --- a/compiler/rustc_middle/src/middle/region.rs +++ b/compiler/rustc_middle/src/middle/region.rs @@ -7,7 +7,6 @@ //! [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/borrow_check.html use std::fmt; -use std::ops::Deref; use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::unord::UnordMap; @@ -228,82 +227,6 @@ pub struct ScopeTree { /// This information is used later for linting to identify locals and /// temporary values that will receive backwards-incompatible drop orders. pub backwards_incompatible_scope: UnordMap<hir::ItemLocalId, Scope>, - - /// If there are any `yield` nested within a scope, this map - /// stores the `Span` of the last one and its index in the - /// postorder of the Visitor traversal on the HIR. - /// - /// HIR Visitor postorder indexes might seem like a peculiar - /// thing to care about. but it turns out that HIR bindings - /// and the temporary results of HIR expressions are never - /// storage-live at the end of HIR nodes with postorder indexes - /// lower than theirs, and therefore don't need to be suspended - /// at yield-points at these indexes. - /// - /// For an example, suppose we have some code such as: - /// ```rust,ignore (example) - /// foo(f(), yield y, bar(g())) - /// ``` - /// - /// With the HIR tree (calls numbered for expository purposes) - /// - /// ```text - /// Call#0(foo, [Call#1(f), Yield(y), Call#2(bar, Call#3(g))]) - /// ``` - /// - /// Obviously, the result of `f()` was created before the yield - /// (and therefore needs to be kept valid over the yield) while - /// the result of `g()` occurs after the yield (and therefore - /// doesn't). If we want to infer that, we can look at the - /// postorder traversal: - /// ```plain,ignore - /// `foo` `f` Call#1 `y` Yield `bar` `g` Call#3 Call#2 Call#0 - /// ``` - /// - /// In which we can easily see that `Call#1` occurs before the yield, - /// and `Call#3` after it. - /// - /// To see that this method works, consider: - /// - /// Let `D` be our binding/temporary and `U` be our other HIR node, with - /// `HIR-postorder(U) < HIR-postorder(D)`. Suppose, as in our example, - /// U is the yield and D is one of the calls. - /// Let's show that `D` is storage-dead at `U`. - /// - /// Remember that storage-live/storage-dead refers to the state of - /// the *storage*, and does not consider moves/drop flags. - /// - /// Then: - /// - /// 1. From the ordering guarantee of HIR visitors (see - /// `rustc_hir::intravisit`), `D` does not dominate `U`. - /// - /// 2. Therefore, `D` is *potentially* storage-dead at `U` (because - /// we might visit `U` without ever getting to `D`). - /// - /// 3. However, we guarantee that at each HIR point, each - /// binding/temporary is always either always storage-live - /// or always storage-dead. This is what is being guaranteed - /// by `terminating_scopes` including all blocks where the - /// count of executions is not guaranteed. - /// - /// 4. By `2.` and `3.`, `D` is *statically* storage-dead at `U`, - /// QED. - /// - /// This property ought to not on (3) in an essential way -- it - /// is probably still correct even if we have "unrestricted" terminating - /// scopes. However, why use the complicated proof when a simple one - /// works? - /// - /// A subtle thing: `box` expressions, such as `box (&x, yield 2, &y)`. It - /// might seem that a `box` expression creates a `Box<T>` temporary - /// when it *starts* executing, at `HIR-preorder(BOX-EXPR)`. That might - /// be true in the MIR desugaring, but it is not important in the semantics. - /// - /// The reason is that semantically, until the `box` expression returns, - /// the values are still owned by their containing expressions. So - /// we'll see that `&x`. - pub yield_in_scope: UnordMap<Scope, Vec<YieldData>>, } /// See the `rvalue_candidates` field for more information on rvalue @@ -316,15 +239,6 @@ pub struct RvalueCandidate { pub lifetime: Option<Scope>, } -#[derive(Debug, Copy, Clone, HashStable)] -pub struct YieldData { - /// The `Span` of the yield. - pub span: Span, - /// The number of expressions and patterns appearing before the `yield` in the body, plus one. - pub expr_and_pat_count: usize, - pub source: hir::YieldSource, -} - impl ScopeTree { pub fn record_scope_parent(&mut self, child: Scope, parent: Option<Scope>) { debug!("{:?}.parent = {:?}", child, parent); @@ -380,10 +294,4 @@ impl ScopeTree { true } - - /// Checks whether the given scope contains a `yield`. If so, - /// returns `Some(YieldData)`. If not, returns `None`. - pub fn yield_in_scope(&self, scope: Scope) -> Option<&[YieldData]> { - self.yield_in_scope.get(&scope).map(Deref::deref) - } } diff --git a/compiler/rustc_mir_build/src/builder/expr/as_rvalue.rs b/compiler/rustc_mir_build/src/builder/expr/as_rvalue.rs index 975226bb642..daf8fa5f19e 100644 --- a/compiler/rustc_mir_build/src/builder/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/builder/expr/as_rvalue.rs @@ -171,9 +171,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.diverge_from(block); block = success; - // The `Box<T>` temporary created here is not a part of the HIR, - // and therefore is not considered during coroutine auto-trait - // determination. See the comment about `box` at `yield_in_scope`. let result = this.local_decls.push(LocalDecl::new(expr.ty, expr_span)); this.cfg .push(block, Statement::new(source_info, StatementKind::StorageLive(result))); diff --git a/compiler/rustc_parse/src/validate_attr.rs b/compiler/rustc_parse/src/validate_attr.rs index 27355a422d1..8fdc06ee463 100644 --- a/compiler/rustc_parse/src/validate_attr.rs +++ b/compiler/rustc_parse/src/validate_attr.rs @@ -304,6 +304,7 @@ fn emit_malformed_attribute( | sym::naked | sym::no_mangle | sym::non_exhaustive + | sym::ignore | sym::must_use | sym::track_caller | sym::link_name @@ -319,8 +320,7 @@ fn emit_malformed_attribute( // Some of previously accepted forms were used in practice, // report them as warnings for now. - let should_warn = - |name| matches!(name, sym::doc | sym::ignore | sym::link | sym::test | sym::bench); + let should_warn = |name| matches!(name, sym::doc | sym::link | sym::test | sym::bench); let error_msg = format!("malformed `{name}` attribute input"); let mut suggestions = vec![]; diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index c5ced406414..18b3ab12e2d 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -215,6 +215,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> { Attribute::Parsed(AttributeKind::MayDangle(attr_span)) => { self.check_may_dangle(hir_id, *attr_span) } + Attribute::Parsed(AttributeKind::Ignore { span, .. }) => { + self.check_generic_attr(hir_id, sym::ignore, *span, target, Target::Fn) + } Attribute::Parsed(AttributeKind::MustUse { span, .. }) => { self.check_must_use(hir_id, *span, target) } @@ -303,7 +306,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } [sym::path, ..] => self.check_generic_attr_unparsed(hir_id, attr, target, Target::Mod), [sym::macro_export, ..] => self.check_macro_export(hir_id, attr, target), - [sym::ignore, ..] | [sym::should_panic, ..] => { + [sym::should_panic, ..] => { self.check_generic_attr_unparsed(hir_id, attr, target, Target::Fn) } [sym::automatically_derived, ..] => { diff --git a/src/doc/rustc/src/platform-support/armv5te-unknown-linux-gnueabi.md b/src/doc/rustc/src/platform-support/armv5te-unknown-linux-gnueabi.md index 1baf1049994..0aebbc34d40 100644 --- a/src/doc/rustc/src/platform-support/armv5te-unknown-linux-gnueabi.md +++ b/src/doc/rustc/src/platform-support/armv5te-unknown-linux-gnueabi.md @@ -7,7 +7,7 @@ floating-point units. ## Target maintainers -[@koalatux](https://github.com/koalatux) +There are currently no formally documented target maintainers. ## Requirements diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index 9dbf51e9796..11c0f08debe 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -13,35 +13,82 @@ defaults: shell: bash jobs: - build: + test: + name: test (${{ matrix.host_target }}) strategy: fail-fast: false matrix: include: - - os: ubuntu-latest - host_target: x86_64-unknown-linux-gnu - - os: macos-14 - host_target: aarch64-apple-darwin - - os: windows-latest - host_target: i686-pc-windows-msvc + - host_target: x86_64-unknown-linux-gnu + os: ubuntu-latest + - host_target: i686-unknown-linux-gnu + os: ubuntu-latest + multiarch: i386 + gcc_cross: i686-linux-gnu + - host_target: aarch64-unknown-linux-gnu + os: ubuntu-24.04-arm + - host_target: armv7-unknown-linux-gnueabihf + os: ubuntu-24.04-arm + multiarch: armhf + gcc_cross: arm-linux-gnueabihf + - host_target: riscv64gc-unknown-linux-gnu + os: ubuntu-latest + multiarch: riscv64 + gcc_cross: riscv64-linux-gnu + qemu: true + - host_target: s390x-unknown-linux-gnu + os: ubuntu-latest + multiarch: s390x + gcc_cross: s390x-linux-gnu + qemu: true + - host_target: aarch64-apple-darwin + os: macos-latest + - host_target: i686-pc-windows-msvc + os: windows-latest runs-on: ${{ matrix.os }} env: HOST_TARGET: ${{ matrix.host_target }} steps: - uses: actions/checkout@v4 + - name: install qemu + if: ${{ matrix.qemu }} + run: sudo apt install qemu-user qemu-user-binfmt + - name: install multiarch + if: ${{ matrix.multiarch != '' }} + run: | + # s390x, ppc64el need Ubuntu Ports to be in the mirror list + sudo bash -c "echo 'https://ports.ubuntu.com/ priority:4' >> /etc/apt/apt-mirrors.txt" + # Add architecture + sudo dpkg --add-architecture ${{ matrix.multiarch }} + sudo apt update + # Install needed packages + sudo apt install $(echo "libatomic1: zlib1g-dev:" | sed 's/:/:${{ matrix.multiarch }}/g') - uses: ./.github/workflows/setup with: toolchain_flags: "--host ${{ matrix.host_target }}" - # The `style` job only runs on Linux; this makes sure the Windows-host-specific - # code is also covered by clippy. - - name: Check clippy - if: ${{ matrix.os == 'windows-latest' }} - run: ./miri clippy -- -D warnings + # We set up the cross-compiler *after* the basic setup as setting CC would otherwise + # cause confusion. + - name: install gcc-cross + if: ${{ matrix.gcc_cross != '' }} + run: | + sudo apt install gcc-${{ matrix.gcc_cross }} + echo "Setting environment variables:" + echo "CC_${{ matrix.host_target }}=${{ matrix.gcc_cross }}-gcc" | tee -a $GITHUB_ENV + TARGET_UPPERCASE=$(echo ${{ matrix.host_target }} | tr '[:lower:]-' '[:upper:]_') + echo "CARGO_TARGET_${TARGET_UPPERCASE}_LINKER=${{ matrix.gcc_cross }}-gcc" | tee -a $GITHUB_ENV - - name: Test Miri + # The main test job! We don't run this in qemu as that is quite slow, + # so those targets only get the clippy check below. + - name: test Miri + if: ${{ !matrix.qemu }} run: ./ci/ci.sh + # The `style` job only runs on Linux; this makes sure the host-specific + # code is also covered by clippy. + - name: clippy + run: ./miri clippy -- -D warnings + style: name: style checks runs-on: ubuntu-latest @@ -51,8 +98,6 @@ jobs: - name: rustfmt run: ./miri fmt --check - - name: clippy - run: ./miri clippy -- -D warnings - name: clippy (no features) run: ./miri clippy --no-default-features -- -D warnings - name: clippy (all features) @@ -73,7 +118,7 @@ jobs: # ALL THE PREVIOUS JOBS NEED TO BE ADDED TO THE `needs` SECTION OF THIS JOB! # And they should be added below in `cron-fail-notify` as well. conclusion: - needs: [build, style, coverage] + needs: [test, style, coverage] # We need to ensure this job does *not* get skipped if its dependencies fail, # because a skipped job is considered a success by GitHub. So we have to # overwrite `if:`. We use `!cancelled()` to ensure the job does still not get run @@ -135,7 +180,7 @@ jobs: cron-fail-notify: name: cronjob failure notification runs-on: ubuntu-latest - needs: [build, style, coverage] + needs: [test, style, coverage] if: ${{ github.event_name == 'schedule' && failure() }} steps: # Send a Zulip notification diff --git a/src/tools/miri/.github/workflows/setup/action.yml b/src/tools/miri/.github/workflows/setup/action.yml index 146b432171e..9110e6947f4 100644 --- a/src/tools/miri/.github/workflows/setup/action.yml +++ b/src/tools/miri/.github/workflows/setup/action.yml @@ -2,6 +2,7 @@ name: "Miri CI setup" description: "Sets up Miri CI" inputs: toolchain_flags: + description: extra flags to pass to rustup-toolchain-install-master required: false default: '' runs: @@ -31,18 +32,15 @@ runs: ~/.cargo/bin ~/.cargo/.crates.toml ~/.cargo/.crates2.json - key: cargo-${{ runner.os }}-${{ hashFiles('**/Cargo.lock', '.github/workflows/**/*.yml') }} - restore-keys: cargo-${{ runner.os }} + # Bump the version when something here changes that needs a cache reset. + key: cargo-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('**/Cargo.lock') }}-v1 + restore-keys: cargo-${{ runner.os }}-${{ runner.arch }} - - name: Install rustup-toolchain-install-master + - name: Install the tools we need if: steps.cache.outputs.cache-hit != 'true' run: cargo install -f rustup-toolchain-install-master hyperfine shell: bash - - name: Install nightly toolchain - run: rustup toolchain install nightly --profile minimal - shell: bash - - name: Install "master" toolchain run: | if [[ ${{ github.event_name }} == 'schedule' ]]; then diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index 739f0702252..fef7f807e93 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -158,6 +158,15 @@ compiler that has `debug=true` set in `bootstrap.toml`. You can set `MIRI_BACKTRACE=1` to get a backtrace of where an evaluation error was originally raised. +#### Tracing + +You can generate a Chrome trace file from a Miri execution by passing `--features=tracing` during the +build and then setting `MIRI_TRACING=1` when running Miri. This will generate a `.json` file that +you can visualize in [Perfetto](https://ui.perfetto.dev/). For example: + +```sh +MIRI_TRACING=1 ./miri run --features=tracing tests/pass/hello.rs +``` ### UI testing diff --git a/src/tools/miri/Cargo.lock b/src/tools/miri/Cargo.lock index d3123caaa47..aa6f059cec2 100644 --- a/src/tools/miri/Cargo.lock +++ b/src/tools/miri/Cargo.lock @@ -627,6 +627,7 @@ dependencies = [ "regex", "rustc_version", "serde", + "serde_json", "smallvec", "tempfile", "tikv-jemalloc-sys", diff --git a/src/tools/miri/Cargo.toml b/src/tools/miri/Cargo.toml index 9391a6ffee3..75476d7923c 100644 --- a/src/tools/miri/Cargo.toml +++ b/src/tools/miri/Cargo.toml @@ -27,6 +27,7 @@ chrono = { version = "0.4.38", default-features = false } chrono-tz = "0.10" directories = "6" bitflags = "2.6" +serde_json = { version = "1.0", optional = true } # Copied from `compiler/rustc/Cargo.toml`. # But only for some targets, it fails for others. Rustc configures this in its CI, but we can't @@ -67,6 +68,7 @@ default = ["stack-cache"] genmc = [] stack-cache = [] stack-cache-consistency-check = ["stack-cache"] +tracing = ["serde_json"] [lints.rust.unexpected_cfgs] level = "warn" diff --git a/src/tools/miri/README.md b/src/tools/miri/README.md index e609fb9b6f9..b05acff72b5 100644 --- a/src/tools/miri/README.md +++ b/src/tools/miri/README.md @@ -11,12 +11,12 @@ instance: * Not sufficiently aligned memory accesses and references * Violation of basic type invariants (a `bool` that is not 0 or 1, for example, or an invalid enum discriminant) +* Data races and emulation of *some* weak memory effects, i.e., + atomic reads can return outdated values * **Experimental**: Violations of the [Stacked Borrows] rules governing aliasing for reference types * **Experimental**: Violations of the [Tree Borrows] aliasing rules, as an optional alternative to [Stacked Borrows] -* **Experimental**: Data races and emulation of weak memory effects, i.e., - atomic reads can return outdated values. On top of that, Miri will also tell you about memory leaks: when there is memory still allocated at the end of the execution, and that memory is not reachable diff --git a/src/tools/miri/ci/ci.sh b/src/tools/miri/ci/ci.sh index 8941af681a4..5767d178279 100755 --- a/src/tools/miri/ci/ci.sh +++ b/src/tools/miri/ci/ci.sh @@ -143,11 +143,36 @@ case $HOST_TARGET in GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests # Extra tier 1 MANY_SEEDS=64 TEST_TARGET=i686-unknown-linux-gnu run_tests - MANY_SEEDS=64 TEST_TARGET=aarch64-unknown-linux-gnu run_tests MANY_SEEDS=64 TEST_TARGET=x86_64-apple-darwin run_tests MANY_SEEDS=64 TEST_TARGET=x86_64-pc-windows-gnu run_tests + ;; + i686-unknown-linux-gnu) + # Host + # Without GC_STRESS as this is a slow runner. + MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests + # Partially supported targets (tier 2) + BASIC="empty_main integer heap_alloc libc-mem vec string btreemap" # ensures we have the basics: pre-main code, system allocator + UNIX="hello panic/panic panic/unwind concurrency/simple atomic libc-mem libc-misc libc-random env num_cpus" # the things that are very similar across all Unixes, and hence easily supported there + TEST_TARGET=aarch64-linux-android run_tests_minimal $BASIC $UNIX time hashmap random thread sync concurrency epoll eventfd + TEST_TARGET=wasm32-wasip2 run_tests_minimal $BASIC wasm + TEST_TARGET=wasm32-unknown-unknown run_tests_minimal no_std empty_main wasm # this target doesn't really have std + TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std + ;; + aarch64-unknown-linux-gnu) + # Host + GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests # Extra tier 1 candidate MANY_SEEDS=64 TEST_TARGET=aarch64-pc-windows-msvc run_tests + # Extra tier 2 + MANY_SEEDS=16 TEST_TARGET=arm-unknown-linux-gnueabi run_tests # 32bit ARM + MANY_SEEDS=16 TEST_TARGET=aarch64-pc-windows-gnullvm run_tests # gnullvm ABI + MANY_SEEDS=16 TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture of choice + # Custom target JSON file + TEST_TARGET=tests/x86_64-unknown-kernel.json MIRI_NO_STD=1 run_tests_minimal no_std + ;; + armv7-unknown-linux-gnueabihf) + # Host + GC_STRESS=1 MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 CARGO_MIRI_ENV=1 run_tests ;; aarch64-apple-darwin) # Host @@ -155,30 +180,16 @@ case $HOST_TARGET in # Extra tier 1 MANY_SEEDS=64 TEST_TARGET=i686-pc-windows-gnu run_tests MANY_SEEDS=64 TEST_TARGET=x86_64-pc-windows-msvc CARGO_MIRI_ENV=1 run_tests - # Extra tier 2 - MANY_SEEDS=16 TEST_TARGET=arm-unknown-linux-gnueabi run_tests # 32bit ARM - MANY_SEEDS=16 TEST_TARGET=aarch64-pc-windows-gnullvm run_tests # gnullvm ABI - MANY_SEEDS=16 TEST_TARGET=s390x-unknown-linux-gnu run_tests # big-endian architecture of choice # Not officially supported tier 2 MANY_SEEDS=16 TEST_TARGET=mips-unknown-linux-gnu run_tests # a 32bit big-endian target, and also a target without 64bit atomics MANY_SEEDS=16 TEST_TARGET=x86_64-unknown-illumos run_tests MANY_SEEDS=16 TEST_TARGET=x86_64-pc-solaris run_tests MANY_SEEDS=16 TEST_TARGET=x86_64-unknown-freebsd run_tests MANY_SEEDS=16 TEST_TARGET=i686-unknown-freebsd run_tests - # Partially supported targets (tier 2) - BASIC="empty_main integer heap_alloc libc-mem vec string btreemap" # ensures we have the basics: pre-main code, system allocator - UNIX="hello panic/panic panic/unwind concurrency/simple atomic libc-mem libc-misc libc-random env num_cpus" # the things that are very similar across all Unixes, and hence easily supported there - TEST_TARGET=aarch64-linux-android run_tests_minimal $BASIC $UNIX time hashmap random thread sync concurrency epoll eventfd - TEST_TARGET=wasm32-wasip2 run_tests_minimal $BASIC wasm - TEST_TARGET=wasm32-unknown-unknown run_tests_minimal no_std empty_main wasm # this target doesn't really have std - TEST_TARGET=thumbv7em-none-eabihf run_tests_minimal no_std - # Custom target JSON file - TEST_TARGET=tests/x86_64-unknown-kernel.json MIRI_NO_STD=1 run_tests_minimal no_std ;; i686-pc-windows-msvc) # Host - # Without GC_STRESS and with reduced many-seeds count as this is the slowest runner. - # (The macOS runner checks windows-msvc with full many-seeds count.) + # Without GC_STRESS as this is the slowest runner. MIR_OPT=1 MANY_SEEDS=64 TEST_BENCH=1 run_tests # Extra tier 1 # We really want to ensure a Linux target works on a Windows host, diff --git a/src/tools/miri/etc/rust_analyzer_helix.toml b/src/tools/miri/etc/rust_analyzer_helix.toml index 62db463a191..9bfb09120d8 100644 --- a/src/tools/miri/etc/rust_analyzer_helix.toml +++ b/src/tools/miri/etc/rust_analyzer_helix.toml @@ -9,23 +9,21 @@ linkedProjects = [ ] [language-server.rust-analyzer.config.check] -invocationLocation = "root" invocationStrategy = "once" overrideCommand = [ - "env", - "MIRI_AUTO_OPS=no", "./miri", "clippy", # make this `check` when working with a locally built rustc "--message-format=json", ] +[language-server.rust-analyzer.config.cargo.extraEnv] +MIRI_AUTO_OPS = "no" +MIRI_IN_RA = "1" + # Contrary to what the name suggests, this also affects proc macros. -[language-server.rust-analyzer.config.buildScripts] -invocationLocation = "root" +[language-server.rust-analyzer.config.cargo.buildScripts] invocationStrategy = "once" overrideCommand = [ - "env", - "MIRI_AUTO_OPS=no", "./miri", "check", "--message-format=json", diff --git a/src/tools/miri/miri b/src/tools/miri/miri index b1b146d7990..549998ae44a 100755 --- a/src/tools/miri/miri +++ b/src/tools/miri/miri @@ -2,20 +2,24 @@ set -e # We want to call the binary directly, so we need to know where it ends up. ROOT_DIR="$(dirname "$0")" -MIRI_SCRIPT_TARGET_DIR="$ROOT_DIR"/miri-script/target -TOOLCHAIN="+nightly" +TARGET_DIR="$ROOT_DIR"/miri-script/target +# Prepare cargo invocation. +# We have to overwrite the toolchain since `+miri` might be activated and not installed. +TOOLCHAIN="+stable" +CARGO_FLAGS=("-q") # If we are being invoked for RA, use JSON output and the default toolchain (to make proc-macros # work in RA). This needs a different target dir to avoid mixing up the builds. +# Also set `-Zroot-dir` so that RA can identify where the error occurred. if [ -n "$MIRI_IN_RA" ]; then - MESSAGE_FORMAT="--message-format=json" TOOLCHAIN="" - MIRI_SCRIPT_TARGET_DIR="$MIRI_SCRIPT_TARGET_DIR"/ra + CARGO_FLAGS+=("--message-format=json" "-Zroot-dir=$ROOT_DIR") + TARGET_DIR="$ROOT_DIR"/target fi -# We need a nightly toolchain, for `-Zroot-dir`. -cargo $TOOLCHAIN build $CARGO_EXTRA_FLAGS --manifest-path "$ROOT_DIR"/miri-script/Cargo.toml \ - -Zroot-dir="$ROOT_DIR" \ - -q --target-dir "$MIRI_SCRIPT_TARGET_DIR" $MESSAGE_FORMAT || \ - ( echo "Failed to build miri-script. Is the 'nightly' toolchain installed?"; exit 1 ) -# Instead of doing just `cargo run --manifest-path .. $@`, we invoke miri-script binary directly. Invoking `cargo run` goes through -# rustup (that sets it's own environmental variables), which is undesirable. -"$MIRI_SCRIPT_TARGET_DIR"/debug/miri-script "$@" +# Run cargo. +cargo $TOOLCHAIN build --manifest-path "$ROOT_DIR"/miri-script/Cargo.toml \ + --target-dir "$TARGET_DIR" "${CARGO_FLAGS[@]}" || \ + ( echo "Failed to build miri-script. Is the 'stable' toolchain installed?"; exit 1 ) +# Instead of doing just `cargo run --manifest-path .. $@`, we invoke miri-script binary directly. +# Invoking `cargo run` goes through rustup (that sets it's own environmental variables), which is +# undesirable. +"$TARGET_DIR"/debug/miri-script "$@" diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index d18e9c7791f..e948beef004 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -231,11 +231,6 @@ impl Command { cmd!(sh, "rustup override set miri").run()?; // Cleanup. cmd!(sh, "cargo clean").run()?; - // Call `cargo metadata` on the sources in case that changes the lockfile - // (which fails under some setups when it is done from inside vscode). - let sysroot = cmd!(sh, "rustc --print sysroot").read()?; - let sysroot = sysroot.trim(); - cmd!(sh, "cargo metadata --format-version 1 --manifest-path {sysroot}/lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.toml").ignore_stdout().run()?; Ok(()) } @@ -707,9 +702,12 @@ impl Command { let mut early_flags = Vec::<OsString>::new(); // In `dep` mode, the target is already passed via `MIRI_TEST_TARGET` - if !dep && let Some(target) = &target { - early_flags.push("--target".into()); - early_flags.push(target.into()); + #[expect(clippy::collapsible_if)] // we need to wait until this is stable + if !dep { + if let Some(target) = &target { + early_flags.push("--target".into()); + early_flags.push(target.into()); + } } early_flags.push("--edition".into()); early_flags.push(edition.as_deref().unwrap_or("2021").into()); @@ -737,8 +735,11 @@ impl Command { // Add Miri flags let mut cmd = cmd.args(&miri_flags).args(&early_flags).args(&flags); // For `--dep` we also need to set the target in the env var. - if dep && let Some(target) = &target { - cmd = cmd.env("MIRI_TEST_TARGET", target); + #[expect(clippy::collapsible_if)] // we need to wait until this is stable + if dep { + if let Some(target) = &target { + cmd = cmd.env("MIRI_TEST_TARGET", target); + } } // Finally, run the thing. Ok(cmd.run()?) diff --git a/src/tools/miri/miri.bat b/src/tools/miri/miri.bat index b046b6310dd..6f9a8f38d65 100644 --- a/src/tools/miri/miri.bat +++ b/src/tools/miri/miri.bat @@ -5,8 +5,8 @@ set MIRI_SCRIPT_TARGET_DIR=%0\..\miri-script\target :: If any other steps are added, the "|| exit /b" must be appended to early :: return from the script. If not, it will continue execution. -cargo +nightly build %CARGO_EXTRA_FLAGS% -q --target-dir %MIRI_SCRIPT_TARGET_DIR% --manifest-path %0\..\miri-script\Cargo.toml ^ - || (echo Failed to build miri-script. Is the 'nightly' toolchain installed? & exit /b) +cargo +stable build %CARGO_EXTRA_FLAGS% -q --target-dir %MIRI_SCRIPT_TARGET_DIR% --manifest-path %0\..\miri-script\Cargo.toml ^ + || (echo Failed to build miri-script. Is the 'stable' toolchain installed? & exit /b) :: Forwards all arguments to this file to the executable. :: We invoke the binary directly to avoid going through rustup, which would set some extra diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index b2ab17fd76a..0130cf13a45 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -d41e12f1f4e4884c356f319b881921aa37040de5 +733b47ea4b1b86216f14ef56e49440c33933f230 diff --git a/src/tools/miri/rustfmt.toml b/src/tools/miri/rustfmt.toml index 49650d8486c..92fd22b7f55 100644 --- a/src/tools/miri/rustfmt.toml +++ b/src/tools/miri/rustfmt.toml @@ -8,3 +8,8 @@ imports_granularity = "Module" force_multiline_blocks = true match_arm_blocks = false match_arm_leading_pipes = "Preserve" + +ignore = [ + # This file is copy-pasted from the tracing_chrome crate and should remain like the original. + "src/bin/log/tracing_chrome.rs" +] diff --git a/src/tools/miri/src/bin/log/mod.rs b/src/tools/miri/src/bin/log/mod.rs new file mode 100644 index 00000000000..f3b2fdb5348 --- /dev/null +++ b/src/tools/miri/src/bin/log/mod.rs @@ -0,0 +1,2 @@ +pub mod setup; +mod tracing_chrome; diff --git a/src/tools/miri/src/bin/log/setup.rs b/src/tools/miri/src/bin/log/setup.rs new file mode 100644 index 00000000000..da0ba528b2c --- /dev/null +++ b/src/tools/miri/src/bin/log/setup.rs @@ -0,0 +1,126 @@ +use std::env::{self, VarError}; +use std::str::FromStr; +use std::sync::{Mutex, OnceLock}; + +use rustc_middle::ty::TyCtxt; +use rustc_session::{CtfeBacktrace, EarlyDiagCtxt}; + +/// The tracing layer from `tracing-chrome` starts a thread in the background that saves data to +/// file and closes the file when stopped. If the thread is not stopped properly, the file will be +/// missing end terminators (`]` for JSON arrays) and other data may also not be flushed. Therefore +/// we need to keep a guard that, when [Drop]ped, will send a signal to stop the thread. Make sure +/// to manually drop this guard using [deinit_loggers], if you are exiting the program with +/// [std::process::exit]! +#[must_use] +struct TracingGuard { + #[cfg(feature = "tracing")] + _chrome: super::tracing_chrome::FlushGuard, + _no_construct: (), +} + +// This ensures TracingGuard is always a drop-type, even when the `_chrome` field is disabled. +impl Drop for TracingGuard { + fn drop(&mut self) {} +} + +fn rustc_logger_config() -> rustc_log::LoggerConfig { + // Start with the usual env vars. + let mut cfg = rustc_log::LoggerConfig::from_env("RUSTC_LOG"); + + // Overwrite if MIRI_LOG is set. + if let Ok(var) = env::var("MIRI_LOG") { + // MIRI_LOG serves as default for RUSTC_LOG, if that is not set. + if matches!(cfg.filter, Err(VarError::NotPresent)) { + // We try to be a bit clever here: if `MIRI_LOG` is just a single level + // used for everything, we only apply it to the parts of rustc that are + // CTFE-related. Otherwise, we use it verbatim for `RUSTC_LOG`. + // This way, if you set `MIRI_LOG=trace`, you get only the right parts of + // rustc traced, but you can also do `MIRI_LOG=miri=trace,rustc_const_eval::interpret=debug`. + if tracing::Level::from_str(&var).is_ok() { + cfg.filter = Ok(format!( + "rustc_middle::mir::interpret={var},rustc_const_eval::interpret={var},miri={var}" + )); + } else { + cfg.filter = Ok(var); + } + } + } + + cfg +} + +/// The global logger can only be set once per process, so track whether that already happened and +/// keep a [TracingGuard] so it can be [Drop]ped later using [deinit_loggers]. +static LOGGER_INITED: OnceLock<Mutex<Option<TracingGuard>>> = OnceLock::new(); + +fn init_logger_once(early_dcx: &EarlyDiagCtxt) { + // If the logger is not yet initialized, initialize it. + LOGGER_INITED.get_or_init(|| { + let guard = if env::var_os("MIRI_TRACING").is_some() { + #[cfg(not(feature = "tracing"))] + { + crate::fatal_error!( + "fatal error: cannot enable MIRI_TRACING since Miri was not built with the \"tracing\" feature" + ); + } + + #[cfg(feature = "tracing")] + { + let (chrome_layer, chrome_guard) = + super::tracing_chrome::ChromeLayerBuilder::new().include_args(true).build(); + rustc_driver::init_logger_with_additional_layer( + early_dcx, + rustc_logger_config(), + || { + tracing_subscriber::layer::SubscriberExt::with( + tracing_subscriber::Registry::default(), + chrome_layer, + ) + }, + ); + + Some(TracingGuard { _chrome: chrome_guard, _no_construct: () }) + } + } else { + // initialize the logger without any tracing enabled + rustc_driver::init_logger(early_dcx, rustc_logger_config()); + None + }; + Mutex::new(guard) + }); +} + +pub fn init_early_loggers(early_dcx: &EarlyDiagCtxt) { + // We only initialize `rustc` if the env var is set (so the user asked for it). + // If it is not set, we avoid initializing now so that we can initialize later with our custom + // settings, and *not* log anything for what happens before `miri` starts interpreting. + if env::var_os("RUSTC_LOG").is_some() { + init_logger_once(early_dcx); + } +} + +pub fn init_late_loggers(early_dcx: &EarlyDiagCtxt, tcx: TyCtxt<'_>) { + // If the logger is not yet initialized, initialize it. + init_logger_once(early_dcx); + + // If `MIRI_BACKTRACE` is set and `RUSTC_CTFE_BACKTRACE` is not, set `RUSTC_CTFE_BACKTRACE`. + // Do this late, so we ideally only apply this to Miri's errors. + if let Some(val) = env::var_os("MIRI_BACKTRACE") { + let ctfe_backtrace = match &*val.to_string_lossy() { + "immediate" => CtfeBacktrace::Immediate, + "0" => CtfeBacktrace::Disabled, + _ => CtfeBacktrace::Capture, + }; + *tcx.sess.ctfe_backtrace.borrow_mut() = ctfe_backtrace; + } +} + +/// Must be called before the program terminates to ensure the trace file is closed correctly. Not +/// doing so will result in invalid trace files. Also see [TracingGuard]. +pub fn deinit_loggers() { + if let Some(guard) = LOGGER_INITED.get() + && let Ok(mut guard) = guard.lock() + { + std::mem::drop(guard.take()); + } +} diff --git a/src/tools/miri/src/bin/log/tracing_chrome.rs b/src/tools/miri/src/bin/log/tracing_chrome.rs new file mode 100644 index 00000000000..459acea6f0b --- /dev/null +++ b/src/tools/miri/src/bin/log/tracing_chrome.rs @@ -0,0 +1,625 @@ +// SPDX-License-Identifier: MIT +// SPDX-FileCopyrightText: Copyright (c) 2020 Thoren Paulson +//! This file is taken unmodified from the following link, except for file attributes and +//! `extern crate` at the top. +//! https://github.com/thoren-d/tracing-chrome/blob/7e2625ab4aeeef2f0ef9bde9d6258dd181c04472/src/lib.rs +//! Depending on the tracing-chrome crate from crates.io is unfortunately not possible, since it +//! depends on `tracing_core` which conflicts with rustc_private's `tracing_core` (meaning it would +//! not be possible to use the [ChromeLayer] in a context that expects a [Layer] from +//! rustc_private's `tracing_core` version). +#![allow(warnings)] +#![cfg(feature = "tracing")] + +// This is here and not in src/lib.rs since it is a direct dependency of tracing_chrome.rs and +// should not be included if the "tracing" feature is disabled. +extern crate tracing_core; + +use tracing_core::{field::Field, span, Event, Subscriber}; +use tracing_subscriber::{ + layer::Context, + registry::{LookupSpan, SpanRef}, + Layer, +}; + +use serde_json::{json, Value as JsonValue}; +use std::{ + marker::PhantomData, + path::Path, + sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, Mutex, + }, +}; + +use std::io::{BufWriter, Write}; +use std::sync::mpsc; +use std::sync::mpsc::Sender; +use std::{ + cell::{Cell, RefCell}, + thread::JoinHandle, +}; + +thread_local! { + static OUT: RefCell<Option<Sender<Message>>> = const { RefCell::new(None) }; + static TID: RefCell<Option<usize>> = const { RefCell::new(None) }; +} + +type NameFn<S> = Box<dyn Fn(&EventOrSpan<'_, '_, S>) -> String + Send + Sync>; +type Object = serde_json::Map<String, JsonValue>; + +/// A [`Layer`](tracing_subscriber::Layer) that writes a Chrome trace file. +pub struct ChromeLayer<S> +where + S: Subscriber + for<'span> LookupSpan<'span> + Send + Sync, +{ + out: Arc<Mutex<Sender<Message>>>, + start: std::time::Instant, + max_tid: AtomicUsize, + include_args: bool, + include_locations: bool, + trace_style: TraceStyle, + name_fn: Option<NameFn<S>>, + cat_fn: Option<NameFn<S>>, + _inner: PhantomData<S>, +} + +/// A builder for [`ChromeLayer`](crate::ChromeLayer). +#[derive(Default)] +pub struct ChromeLayerBuilder<S> +where + S: Subscriber + for<'span> LookupSpan<'span> + Send + Sync, +{ + out_writer: Option<Box<dyn Write + Send>>, + name_fn: Option<NameFn<S>>, + cat_fn: Option<NameFn<S>>, + include_args: bool, + include_locations: bool, + trace_style: TraceStyle, + _inner: PhantomData<S>, +} + +/// Decides how traces will be recorded. +#[derive(Default)] +pub enum TraceStyle { + /// Traces will be recorded as a group of threads. + /// In this style, spans should be entered and exited on the same thread. + #[default] + Threaded, + + /// Traces will recorded as a group of asynchronous operations. + Async, +} + +impl<S> ChromeLayerBuilder<S> +where + S: Subscriber + for<'span> LookupSpan<'span> + Send + Sync, +{ + pub fn new() -> Self { + ChromeLayerBuilder { + out_writer: None, + name_fn: None, + cat_fn: None, + include_args: false, + include_locations: true, + trace_style: TraceStyle::Threaded, + _inner: PhantomData, + } + } + + /// Set the file to which to output the trace. + /// + /// Defaults to `./trace-{unix epoch in micros}.json`. + /// + /// # Panics + /// + /// If `file` could not be opened/created. To handle errors, + /// open a file and pass it to [`writer`](crate::ChromeLayerBuilder::writer) instead. + pub fn file<P: AsRef<Path>>(self, file: P) -> Self { + self.writer(std::fs::File::create(file).expect("Failed to create trace file.")) + } + + /// Supply an arbitrary writer to which to write trace contents. + /// + /// # Examples + /// + /// ```rust + /// # use tracing_chrome::ChromeLayerBuilder; + /// # use tracing_subscriber::prelude::*; + /// let (layer, guard) = ChromeLayerBuilder::new().writer(std::io::sink()).build(); + /// # tracing_subscriber::registry().with(layer).init(); + /// ``` + pub fn writer<W: Write + Send + 'static>(mut self, writer: W) -> Self { + self.out_writer = Some(Box::new(writer)); + self + } + + /// Include arguments in each trace entry. + /// + /// Defaults to `false`. + /// + /// Includes the arguments used when creating a span/event + /// in the "args" section of the trace entry. + pub fn include_args(mut self, include: bool) -> Self { + self.include_args = include; + self + } + + /// Include file+line with each trace entry. + /// + /// Defaults to `true`. + /// + /// This can add quite a bit of data to the output so turning + /// it off might be helpful when collecting larger traces. + pub fn include_locations(mut self, include: bool) -> Self { + self.include_locations = include; + self + } + + /// Sets the style used when recording trace events. + /// + /// See [`TraceStyle`](crate::TraceStyle) for details. + pub fn trace_style(mut self, style: TraceStyle) -> Self { + self.trace_style = style; + self + } + + /// Allows supplying a function that derives a name from + /// an Event or Span. The result is used as the "name" field + /// on trace entries. + /// + /// # Example + /// ``` + /// use tracing_chrome::{ChromeLayerBuilder, EventOrSpan}; + /// use tracing_subscriber::{registry::Registry, prelude::*}; + /// + /// let (chrome_layer, _guard) = ChromeLayerBuilder::new().name_fn(Box::new(|event_or_span| { + /// match event_or_span { + /// EventOrSpan::Event(ev) => { ev.metadata().name().into() }, + /// EventOrSpan::Span(_s) => { "span".into() }, + /// } + /// })).build(); + /// tracing_subscriber::registry().with(chrome_layer).init() + /// ``` + pub fn name_fn(mut self, name_fn: NameFn<S>) -> Self { + self.name_fn = Some(name_fn); + self + } + + /// Allows supplying a function that derives a category from + /// an Event or Span. The result is used as the "cat" field on + /// trace entries. + /// + /// # Example + /// ``` + /// use tracing_chrome::{ChromeLayerBuilder, EventOrSpan}; + /// use tracing_subscriber::{registry::Registry, prelude::*}; + /// + /// let (chrome_layer, _guard) = ChromeLayerBuilder::new().category_fn(Box::new(|_| { + /// "my_module".into() + /// })).build(); + /// tracing_subscriber::registry().with(chrome_layer).init() + /// ``` + pub fn category_fn(mut self, cat_fn: NameFn<S>) -> Self { + self.cat_fn = Some(cat_fn); + self + } + + /// Creates a [`ChromeLayer`](crate::ChromeLayer) and associated [`FlushGuard`](crate::FlushGuard). + /// + /// # Panics + /// + /// If no file or writer was specified and the default trace file could not be opened/created. + pub fn build(self) -> (ChromeLayer<S>, FlushGuard) { + ChromeLayer::new(self) + } +} + +/// This guard will signal the thread writing the trace file to stop and join it when dropped. +pub struct FlushGuard { + sender: Sender<Message>, + handle: Cell<Option<JoinHandle<()>>>, +} + +impl FlushGuard { + /// Signals the trace writing thread to flush to disk. + pub fn flush(&self) { + if let Some(handle) = self.handle.take() { + let _ignored = self.sender.send(Message::Flush); + self.handle.set(Some(handle)); + } + } + + /// Finishes the current trace and starts a new one. + /// + /// If a [`Write`](std::io::Write) implementation is supplied, + /// the new trace is written to it. Otherwise, the new trace + /// goes to `./trace-{unix epoc in micros}.json`. + pub fn start_new(&self, writer: Option<Box<dyn Write + Send>>) { + if let Some(handle) = self.handle.take() { + let _ignored = self.sender.send(Message::StartNew(writer)); + self.handle.set(Some(handle)); + } + } +} + +impl Drop for FlushGuard { + fn drop(&mut self) { + if let Some(handle) = self.handle.take() { + let _ignored = self.sender.send(Message::Drop); + if handle.join().is_err() { + eprintln!("tracing_chrome: Trace writing thread panicked."); + } + } + } +} + +struct Callsite { + tid: usize, + name: String, + target: String, + file: Option<&'static str>, + line: Option<u32>, + args: Option<Arc<Object>>, +} + +enum Message { + Enter(f64, Callsite, Option<u64>), + Event(f64, Callsite), + Exit(f64, Callsite, Option<u64>), + NewThread(usize, String), + Flush, + Drop, + StartNew(Option<Box<dyn Write + Send>>), +} + +/// Represents either an [`Event`](tracing_core::Event) or [`SpanRef`](tracing_subscriber::registry::SpanRef). +pub enum EventOrSpan<'a, 'b, S> +where + S: Subscriber + for<'span> LookupSpan<'span> + Send + Sync, +{ + Event(&'a Event<'b>), + Span(&'a SpanRef<'b, S>), +} + +fn create_default_writer() -> Box<dyn Write + Send> { + Box::new( + std::fs::File::create(format!( + "./trace-{}.json", + std::time::SystemTime::UNIX_EPOCH + .elapsed() + .unwrap() + .as_micros() + )) + .expect("Failed to create trace file."), + ) +} + +impl<S> ChromeLayer<S> +where + S: Subscriber + for<'span> LookupSpan<'span> + Send + Sync, +{ + fn new(mut builder: ChromeLayerBuilder<S>) -> (ChromeLayer<S>, FlushGuard) { + let (tx, rx) = mpsc::channel(); + OUT.with(|val| val.replace(Some(tx.clone()))); + + let out_writer = builder + .out_writer + .unwrap_or_else(|| create_default_writer()); + + let handle = std::thread::spawn(move || { + let mut write = BufWriter::new(out_writer); + write.write_all(b"[\n").unwrap(); + + let mut has_started = false; + let mut thread_names: Vec<(usize, String)> = Vec::new(); + for msg in rx { + if let Message::Flush = &msg { + write.flush().unwrap(); + continue; + } else if let Message::Drop = &msg { + break; + } else if let Message::StartNew(writer) = msg { + // Finish off current file + write.write_all(b"\n]").unwrap(); + write.flush().unwrap(); + + // Get or create new writer + let out_writer = writer.unwrap_or_else(|| create_default_writer()); + write = BufWriter::new(out_writer); + write.write_all(b"[\n").unwrap(); + has_started = false; + + // Write saved thread names + for (tid, name) in thread_names.iter() { + let entry = json!({ + "ph": "M", + "pid": 1, + "name": "thread_name", + "tid": *tid, + "args": { + "name": name, + }, + }); + + if has_started { + write.write_all(b",\n").unwrap(); + } + serde_json::to_writer(&mut write, &entry).unwrap(); + has_started = true; + } + continue; + } + + let (ph, ts, callsite, id) = match &msg { + Message::Enter(ts, callsite, None) => ("B", Some(ts), Some(callsite), None), + Message::Enter(ts, callsite, Some(root_id)) => { + ("b", Some(ts), Some(callsite), Some(root_id)) + } + Message::Event(ts, callsite) => ("i", Some(ts), Some(callsite), None), + Message::Exit(ts, callsite, None) => ("E", Some(ts), Some(callsite), None), + Message::Exit(ts, callsite, Some(root_id)) => { + ("e", Some(ts), Some(callsite), Some(root_id)) + } + Message::NewThread(_tid, _name) => ("M", None, None, None), + Message::Flush | Message::Drop | Message::StartNew(_) => { + panic!("Was supposed to break by now.") + } + }; + let mut entry = json!({ + "ph": ph, + "pid": 1, + }); + + if let Message::NewThread(tid, name) = msg { + thread_names.push((tid, name.clone())); + entry["name"] = "thread_name".into(); + entry["tid"] = tid.into(); + entry["args"] = json!({ "name": name }); + } else { + let ts = ts.unwrap(); + let callsite = callsite.unwrap(); + entry["ts"] = (*ts).into(); + entry["name"] = callsite.name.clone().into(); + entry["cat"] = callsite.target.clone().into(); + entry["tid"] = callsite.tid.into(); + + if let Some(&id) = id { + entry["id"] = id.into(); + } + + if ph == "i" { + entry["s"] = "t".into(); + } + + if let (Some(file), Some(line)) = (callsite.file, callsite.line) { + entry[".file"] = file.into(); + entry[".line"] = line.into(); + } + + if let Some(call_args) = &callsite.args { + if !call_args.is_empty() { + entry["args"] = (**call_args).clone().into(); + } + } + } + + if has_started { + write.write_all(b",\n").unwrap(); + } + serde_json::to_writer(&mut write, &entry).unwrap(); + has_started = true; + } + + write.write_all(b"\n]").unwrap(); + write.flush().unwrap(); + }); + + let guard = FlushGuard { + sender: tx.clone(), + handle: Cell::new(Some(handle)), + }; + let layer = ChromeLayer { + out: Arc::new(Mutex::new(tx)), + start: std::time::Instant::now(), + max_tid: AtomicUsize::new(0), + name_fn: builder.name_fn.take(), + cat_fn: builder.cat_fn.take(), + include_args: builder.include_args, + include_locations: builder.include_locations, + trace_style: builder.trace_style, + _inner: PhantomData, + }; + + (layer, guard) + } + + fn get_tid(&self) -> (usize, bool) { + TID.with(|value| { + let tid = *value.borrow(); + match tid { + Some(tid) => (tid, false), + None => { + let tid = self.max_tid.fetch_add(1, Ordering::SeqCst); + value.replace(Some(tid)); + (tid, true) + } + } + }) + } + + fn get_callsite(&self, data: EventOrSpan<S>) -> Callsite { + let (tid, new_thread) = self.get_tid(); + let name = self.name_fn.as_ref().map(|name_fn| name_fn(&data)); + let target = self.cat_fn.as_ref().map(|cat_fn| cat_fn(&data)); + let meta = match data { + EventOrSpan::Event(e) => e.metadata(), + EventOrSpan::Span(s) => s.metadata(), + }; + let args = match data { + EventOrSpan::Event(e) => { + if self.include_args { + let mut args = Object::new(); + e.record(&mut JsonVisitor { object: &mut args }); + Some(Arc::new(args)) + } else { + None + } + } + EventOrSpan::Span(s) => s + .extensions() + .get::<ArgsWrapper>() + .map(|e| &e.args) + .cloned(), + }; + let name = name.unwrap_or_else(|| meta.name().into()); + let target = target.unwrap_or_else(|| meta.target().into()); + let (file, line) = if self.include_locations { + (meta.file(), meta.line()) + } else { + (None, None) + }; + + if new_thread { + let name = match std::thread::current().name() { + Some(name) => name.to_owned(), + None => tid.to_string(), + }; + self.send_message(Message::NewThread(tid, name)); + } + + Callsite { + tid, + name, + target, + file, + line, + args, + } + } + + fn get_root_id(span: SpanRef<S>) -> u64 { + span.scope() + .from_root() + .take(1) + .next() + .unwrap_or(span) + .id() + .into_u64() + } + + fn enter_span(&self, span: SpanRef<S>, ts: f64) { + let callsite = self.get_callsite(EventOrSpan::Span(&span)); + let root_id = match self.trace_style { + TraceStyle::Async => Some(ChromeLayer::get_root_id(span)), + _ => None, + }; + self.send_message(Message::Enter(ts, callsite, root_id)); + } + + fn exit_span(&self, span: SpanRef<S>, ts: f64) { + let callsite = self.get_callsite(EventOrSpan::Span(&span)); + let root_id = match self.trace_style { + TraceStyle::Async => Some(ChromeLayer::get_root_id(span)), + _ => None, + }; + self.send_message(Message::Exit(ts, callsite, root_id)); + } + + fn get_ts(&self) -> f64 { + self.start.elapsed().as_nanos() as f64 / 1000.0 + } + + fn send_message(&self, message: Message) { + OUT.with(move |val| { + if val.borrow().is_some() { + let _ignored = val.borrow().as_ref().unwrap().send(message); + } else { + let out = self.out.lock().unwrap().clone(); + let _ignored = out.send(message); + val.replace(Some(out)); + } + }); + } +} + +impl<S> Layer<S> for ChromeLayer<S> +where + S: Subscriber + for<'span> LookupSpan<'span> + Send + Sync, +{ + fn on_enter(&self, id: &span::Id, ctx: Context<'_, S>) { + if let TraceStyle::Async = self.trace_style { + return; + } + + let ts = self.get_ts(); + self.enter_span(ctx.span(id).expect("Span not found."), ts); + } + + fn on_record(&self, id: &span::Id, values: &span::Record<'_>, ctx: Context<'_, S>) { + if self.include_args { + let span = ctx.span(id).unwrap(); + let mut exts = span.extensions_mut(); + + let args = exts.get_mut::<ArgsWrapper>(); + + if let Some(args) = args { + let args = Arc::make_mut(&mut args.args); + values.record(&mut JsonVisitor { object: args }); + } + } + } + + fn on_event(&self, event: &Event<'_>, _ctx: Context<'_, S>) { + let ts = self.get_ts(); + let callsite = self.get_callsite(EventOrSpan::Event(event)); + self.send_message(Message::Event(ts, callsite)); + } + + fn on_exit(&self, id: &span::Id, ctx: Context<'_, S>) { + if let TraceStyle::Async = self.trace_style { + return; + } + let ts = self.get_ts(); + self.exit_span(ctx.span(id).expect("Span not found."), ts); + } + + fn on_new_span(&self, attrs: &span::Attributes<'_>, id: &span::Id, ctx: Context<'_, S>) { + if self.include_args { + let mut args = Object::new(); + attrs.record(&mut JsonVisitor { object: &mut args }); + ctx.span(id).unwrap().extensions_mut().insert(ArgsWrapper { + args: Arc::new(args), + }); + } + if let TraceStyle::Threaded = self.trace_style { + return; + } + + let ts = self.get_ts(); + self.enter_span(ctx.span(id).expect("Span not found."), ts); + } + + fn on_close(&self, id: span::Id, ctx: Context<'_, S>) { + if let TraceStyle::Threaded = self.trace_style { + return; + } + + let ts = self.get_ts(); + self.exit_span(ctx.span(&id).expect("Span not found."), ts); + } +} + +struct JsonVisitor<'a> { + object: &'a mut Object, +} + +impl<'a> tracing_subscriber::field::Visit for JsonVisitor<'a> { + fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) { + self.object + .insert(field.name().to_owned(), format!("{value:?}").into()); + } +} + +struct ArgsWrapper { + args: Arc<Object>, +} diff --git a/src/tools/miri/src/bin/miri.rs b/src/tools/miri/src/bin/miri.rs index 56be370d5da..e3b579a4ac6 100644 --- a/src/tools/miri/src/bin/miri.rs +++ b/src/tools/miri/src/bin/miri.rs @@ -10,6 +10,8 @@ // Some "regular" crates we want to share with rustc extern crate tracing; +#[cfg(feature = "tracing")] +extern crate tracing_subscriber; // The rustc crates we need extern crate rustc_abi; @@ -24,14 +26,16 @@ extern crate rustc_middle; extern crate rustc_session; extern crate rustc_span; -use std::env::{self, VarError}; +mod log; + +use std::env; use std::num::NonZero; use std::ops::Range; use std::path::PathBuf; use std::rc::Rc; use std::str::FromStr; +use std::sync::Arc; use std::sync::atomic::{AtomicI32, AtomicU32, Ordering}; -use std::sync::{Arc, Once}; use miri::{ BacktraceStyle, BorrowTrackerMethod, GenmcConfig, GenmcCtx, MiriConfig, MiriEntryFnType, @@ -52,12 +56,14 @@ use rustc_middle::query::LocalCrate; use rustc_middle::traits::{ObligationCause, ObligationCauseCode}; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_middle::util::Providers; +use rustc_session::EarlyDiagCtxt; use rustc_session::config::{CrateType, ErrorOutputType, OptLevel}; use rustc_session::search_paths::PathKind; -use rustc_session::{CtfeBacktrace, EarlyDiagCtxt}; use rustc_span::def_id::DefId; use tracing::debug; +use crate::log::setup::{deinit_loggers, init_early_loggers, init_late_loggers}; + struct MiriCompilerCalls { miri_config: Option<MiriConfig>, many_seeds: Option<ManySeedsConfig>, @@ -154,13 +160,13 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { if tcx.sess.dcx().has_errors_or_delayed_bugs().is_some() { tcx.dcx().fatal("miri cannot be run on programs that fail compilation"); } - - let early_dcx = EarlyDiagCtxt::new(tcx.sess.opts.error_format); - init_late_loggers(&early_dcx, tcx); if !tcx.crate_types().contains(&CrateType::Executable) { tcx.dcx().fatal("miri only makes sense on bin crates"); } + let early_dcx = EarlyDiagCtxt::new(tcx.sess.opts.error_format); + init_late_loggers(&early_dcx, tcx); + let (entry_def_id, entry_type) = entry_fn(tcx); let mut config = self.miri_config.take().expect("after_analysis must only be called once"); @@ -213,7 +219,7 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { if !many_seeds.keep_going { // `abort_if_errors` would actually not stop, since `par_for_each` waits for the // rest of the to finish, so we just exit immediately. - std::process::exit(return_code); + exit(return_code); } exit_code.store(return_code, Ordering::Relaxed); num_failed.fetch_add(1, Ordering::Relaxed); @@ -223,7 +229,7 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { if num_failed > 0 { eprintln!("{num_failed}/{total} SEEDS FAILED", total = many_seeds.seeds.count()); } - std::process::exit(exit_code.0.into_inner()); + exit(exit_code.0.into_inner()); } else { let return_code = miri::eval_entry(tcx, entry_def_id, entry_type, &config, None) .unwrap_or_else(|| { @@ -232,7 +238,7 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { tcx.dcx().abort_if_errors(); rustc_driver::EXIT_FAILURE }); - std::process::exit(return_code); + exit(return_code); } // Unreachable. @@ -328,83 +334,31 @@ impl rustc_driver::Callbacks for MiriBeRustCompilerCalls { } } -fn show_error(msg: &impl std::fmt::Display) -> ! { - eprintln!("fatal error: {msg}"); - std::process::exit(1) -} - -macro_rules! show_error { - ($($tt:tt)*) => { show_error(&format_args!($($tt)*)) }; +fn exit(exit_code: i32) -> ! { + // Drop the tracing guard before exiting, so tracing calls are flushed correctly. + deinit_loggers(); + std::process::exit(exit_code); } -fn rustc_logger_config() -> rustc_log::LoggerConfig { - // Start with the usual env vars. - let mut cfg = rustc_log::LoggerConfig::from_env("RUSTC_LOG"); - - // Overwrite if MIRI_LOG is set. - if let Ok(var) = env::var("MIRI_LOG") { - // MIRI_LOG serves as default for RUSTC_LOG, if that is not set. - if matches!(cfg.filter, Err(VarError::NotPresent)) { - // We try to be a bit clever here: if `MIRI_LOG` is just a single level - // used for everything, we only apply it to the parts of rustc that are - // CTFE-related. Otherwise, we use it verbatim for `RUSTC_LOG`. - // This way, if you set `MIRI_LOG=trace`, you get only the right parts of - // rustc traced, but you can also do `MIRI_LOG=miri=trace,rustc_const_eval::interpret=debug`. - if tracing::Level::from_str(&var).is_ok() { - cfg.filter = Ok(format!( - "rustc_middle::mir::interpret={var},rustc_const_eval::interpret={var},miri={var}" - )); - } else { - cfg.filter = Ok(var); - } - } - } - - cfg -} - -/// The global logger can only be set once per process, so track -/// whether that already happened. -static LOGGER_INITED: Once = Once::new(); - -fn init_early_loggers(early_dcx: &EarlyDiagCtxt) { - // We only initialize `rustc` if the env var is set (so the user asked for it). - // If it is not set, we avoid initializing now so that we can initialize later with our custom - // settings, and *not* log anything for what happens before `miri` starts interpreting. - if env::var_os("RUSTC_LOG").is_some() { - LOGGER_INITED.call_once(|| { - rustc_driver::init_logger(early_dcx, rustc_logger_config()); - }); - } +fn fatal_error_(msg: &impl std::fmt::Display) -> ! { + eprintln!("fatal error: {msg}"); + exit(1) } -fn init_late_loggers(early_dcx: &EarlyDiagCtxt, tcx: TyCtxt<'_>) { - // If the logger is not yet initialized, initialize it. - LOGGER_INITED.call_once(|| { - rustc_driver::init_logger(early_dcx, rustc_logger_config()); - }); - - // If `MIRI_BACKTRACE` is set and `RUSTC_CTFE_BACKTRACE` is not, set `RUSTC_CTFE_BACKTRACE`. - // Do this late, so we ideally only apply this to Miri's errors. - if let Some(val) = env::var_os("MIRI_BACKTRACE") { - let ctfe_backtrace = match &*val.to_string_lossy() { - "immediate" => CtfeBacktrace::Immediate, - "0" => CtfeBacktrace::Disabled, - _ => CtfeBacktrace::Capture, - }; - *tcx.sess.ctfe_backtrace.borrow_mut() = ctfe_backtrace; - } +macro_rules! fatal_error { + ($($tt:tt)*) => { $crate::fatal_error_(&format_args!($($tt)*)) }; } +use fatal_error; /// Execute a compiler with the given CLI arguments and callbacks. fn run_compiler_and_exit( args: &[String], callbacks: &mut (dyn rustc_driver::Callbacks + Send), ) -> ! { - // Invoke compiler, and handle return code. + // Invoke compiler, catch any unwinding panics and handle return code. let exit_code = rustc_driver::catch_with_exit_code(move || rustc_driver::run_compiler(args, callbacks)); - std::process::exit(exit_code) + exit(exit_code) } /// Parses a comma separated list of `T` from the given string: @@ -567,7 +521,7 @@ fn main() { params.precise_interior_mut = false; } _ => - show_error!( + fatal_error!( "`-Zmiri-tree-borrows` is required before `-Zmiri-tree-borrows-no-precise-interior-mut`" ), }; @@ -594,7 +548,7 @@ fn main() { "warn-nobacktrace" => miri::IsolatedOp::Reject(miri::RejectOpWith::WarningWithoutBacktrace), _ => - show_error!( + fatal_error!( "-Zmiri-isolation-error must be `abort`, `hide`, `warn`, or `warn-nobacktrace`" ), }; @@ -625,16 +579,16 @@ fn main() { "all" => RetagFields::Yes, "none" => RetagFields::No, "scalar" => RetagFields::OnlyScalar, - _ => show_error!("`-Zmiri-retag-fields` can only be `all`, `none`, or `scalar`"), + _ => fatal_error!("`-Zmiri-retag-fields` can only be `all`, `none`, or `scalar`"), }; } else if let Some(param) = arg.strip_prefix("-Zmiri-seed=") { let seed = param.parse::<u64>().unwrap_or_else(|_| { - show_error!("-Zmiri-seed must be an integer that fits into u64") + fatal_error!("-Zmiri-seed must be an integer that fits into u64") }); miri_config.seed = Some(seed); } else if let Some(param) = arg.strip_prefix("-Zmiri-many-seeds=") { let range = parse_range(param).unwrap_or_else(|err| { - show_error!( + fatal_error!( "-Zmiri-many-seeds requires a range in the form `from..to` or `..to`: {err}" ) }); @@ -651,51 +605,51 @@ fn main() { miri_config.forwarded_env_vars.push(param.to_owned()); } else if let Some(param) = arg.strip_prefix("-Zmiri-env-set=") { let Some((name, value)) = param.split_once('=') else { - show_error!("-Zmiri-env-set requires an argument of the form <name>=<value>"); + fatal_error!("-Zmiri-env-set requires an argument of the form <name>=<value>"); }; miri_config.set_env_vars.insert(name.to_owned(), value.to_owned()); } else if let Some(param) = arg.strip_prefix("-Zmiri-track-pointer-tag=") { let ids: Vec<u64> = parse_comma_list(param).unwrap_or_else(|err| { - show_error!("-Zmiri-track-pointer-tag requires a comma separated list of valid `u64` arguments: {err}") + fatal_error!("-Zmiri-track-pointer-tag requires a comma separated list of valid `u64` arguments: {err}") }); for id in ids.into_iter().map(miri::BorTag::new) { if let Some(id) = id { miri_config.tracked_pointer_tags.insert(id); } else { - show_error!("-Zmiri-track-pointer-tag requires nonzero arguments"); + fatal_error!("-Zmiri-track-pointer-tag requires nonzero arguments"); } } } else if let Some(param) = arg.strip_prefix("-Zmiri-track-alloc-id=") { let ids = parse_comma_list::<NonZero<u64>>(param).unwrap_or_else(|err| { - show_error!("-Zmiri-track-alloc-id requires a comma separated list of valid non-zero `u64` arguments: {err}") + fatal_error!("-Zmiri-track-alloc-id requires a comma separated list of valid non-zero `u64` arguments: {err}") }); miri_config.tracked_alloc_ids.extend(ids.into_iter().map(miri::AllocId)); } else if arg == "-Zmiri-track-alloc-accesses" { miri_config.track_alloc_accesses = true; } else if let Some(param) = arg.strip_prefix("-Zmiri-address-reuse-rate=") { miri_config.address_reuse_rate = parse_rate(param) - .unwrap_or_else(|err| show_error!("-Zmiri-address-reuse-rate {err}")); + .unwrap_or_else(|err| fatal_error!("-Zmiri-address-reuse-rate {err}")); } else if let Some(param) = arg.strip_prefix("-Zmiri-address-reuse-cross-thread-rate=") { miri_config.address_reuse_cross_thread_rate = parse_rate(param) - .unwrap_or_else(|err| show_error!("-Zmiri-address-reuse-cross-thread-rate {err}")); + .unwrap_or_else(|err| fatal_error!("-Zmiri-address-reuse-cross-thread-rate {err}")); } else if let Some(param) = arg.strip_prefix("-Zmiri-compare-exchange-weak-failure-rate=") { miri_config.cmpxchg_weak_failure_rate = parse_rate(param).unwrap_or_else(|err| { - show_error!("-Zmiri-compare-exchange-weak-failure-rate {err}") + fatal_error!("-Zmiri-compare-exchange-weak-failure-rate {err}") }); } else if let Some(param) = arg.strip_prefix("-Zmiri-preemption-rate=") { - miri_config.preemption_rate = - parse_rate(param).unwrap_or_else(|err| show_error!("-Zmiri-preemption-rate {err}")); + miri_config.preemption_rate = parse_rate(param) + .unwrap_or_else(|err| fatal_error!("-Zmiri-preemption-rate {err}")); } else if arg == "-Zmiri-report-progress" { // This makes it take a few seconds between progress reports on my laptop. miri_config.report_progress = Some(1_000_000); } else if let Some(param) = arg.strip_prefix("-Zmiri-report-progress=") { let interval = param.parse::<u32>().unwrap_or_else(|err| { - show_error!("-Zmiri-report-progress requires a `u32`: {}", err) + fatal_error!("-Zmiri-report-progress requires a `u32`: {}", err) }); miri_config.report_progress = Some(interval); } else if let Some(param) = arg.strip_prefix("-Zmiri-provenance-gc=") { let interval = param.parse::<u32>().unwrap_or_else(|err| { - show_error!("-Zmiri-provenance-gc requires a `u32`: {}", err) + fatal_error!("-Zmiri-provenance-gc requires a `u32`: {}", err) }); miri_config.gc_interval = interval; } else if let Some(param) = arg.strip_prefix("-Zmiri-measureme=") { @@ -705,7 +659,7 @@ fn main() { "0" => BacktraceStyle::Off, "1" => BacktraceStyle::Short, "full" => BacktraceStyle::Full, - _ => show_error!("-Zmiri-backtrace may only be 0, 1, or full"), + _ => fatal_error!("-Zmiri-backtrace may only be 0, 1, or full"), }; } else if let Some(param) = arg.strip_prefix("-Zmiri-native-lib=") { let filename = param.to_string(); @@ -722,27 +676,27 @@ fn main() { miri_config.native_lib.push(filename.into()); } } else { - show_error!("-Zmiri-native-lib `{}` does not exist", filename); + fatal_error!("-Zmiri-native-lib `{}` does not exist", filename); } } else if arg == "-Zmiri-native-lib-enable-tracing" { miri_config.native_lib_enable_tracing = true; } else if let Some(param) = arg.strip_prefix("-Zmiri-num-cpus=") { let num_cpus = param .parse::<u32>() - .unwrap_or_else(|err| show_error!("-Zmiri-num-cpus requires a `u32`: {}", err)); + .unwrap_or_else(|err| fatal_error!("-Zmiri-num-cpus requires a `u32`: {}", err)); if !(1..=miri::MAX_CPUS).contains(&usize::try_from(num_cpus).unwrap()) { - show_error!("-Zmiri-num-cpus must be in the range 1..={}", miri::MAX_CPUS); + fatal_error!("-Zmiri-num-cpus must be in the range 1..={}", miri::MAX_CPUS); } miri_config.num_cpus = num_cpus; } else if let Some(param) = arg.strip_prefix("-Zmiri-force-page-size=") { let page_size = param.parse::<u64>().unwrap_or_else(|err| { - show_error!("-Zmiri-force-page-size requires a `u64`: {}", err) + fatal_error!("-Zmiri-force-page-size requires a `u64`: {}", err) }); // Convert from kilobytes to bytes. let page_size = if page_size.is_power_of_two() { page_size * 1024 } else { - show_error!("-Zmiri-force-page-size requires a power of 2: {page_size}"); + fatal_error!("-Zmiri-force-page-size requires a power of 2: {page_size}"); }; miri_config.page_size = Some(page_size); } else { @@ -753,22 +707,22 @@ fn main() { // Tree Borrows implies strict provenance, and is not compatible with native calls. if matches!(miri_config.borrow_tracker, Some(BorrowTrackerMethod::TreeBorrows { .. })) { if miri_config.provenance_mode != ProvenanceMode::Strict { - show_error!( + fatal_error!( "Tree Borrows does not support integer-to-pointer casts, and hence requires strict provenance" ); } if !miri_config.native_lib.is_empty() { - show_error!("Tree Borrows is not compatible with calling native functions"); + fatal_error!("Tree Borrows is not compatible with calling native functions"); } } // Native calls and strict provenance are not compatible. if !miri_config.native_lib.is_empty() && miri_config.provenance_mode == ProvenanceMode::Strict { - show_error!("strict provenance is not compatible with calling native functions"); + fatal_error!("strict provenance is not compatible with calling native functions"); } // You can set either one seed or many. if many_seeds.is_some() && miri_config.seed.is_some() { - show_error!("Only one of `-Zmiri-seed` and `-Zmiri-many-seeds can be set"); + fatal_error!("Only one of `-Zmiri-seed` and `-Zmiri-many-seeds can be set"); } // Ensure we have parallelism for many-seeds mode. @@ -784,12 +738,12 @@ fn main() { assert_eq!(genmc_config.is_some(), miri_config.genmc_mode); if genmc_config.is_some() { if !miri_config.data_race_detector { - show_error!("Cannot disable data race detection in GenMC mode (currently)"); + fatal_error!("Cannot disable data race detection in GenMC mode (currently)"); } else if !miri_config.weak_memory_emulation { - show_error!("Cannot disable weak memory emulation in GenMC mode"); + fatal_error!("Cannot disable weak memory emulation in GenMC mode"); } } else if miri_config.weak_memory_emulation && !miri_config.data_race_detector { - show_error!( + fatal_error!( "Weak memory emulation cannot be enabled when the data race detector is disabled" ); }; diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index 714eb1fba91..b5e7e9d0ac0 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -971,14 +971,6 @@ pub trait EvalContextExt<'tcx>: MiriInterpCxExt<'tcx> { } } - /// After all threads are done running, this allows data races to occur for subsequent - /// 'administrative' machine accesses (that logically happen outside of the Abstract Machine). - fn allow_data_races_all_threads_done(&mut self) { - let this = self.eval_context_ref(); - assert!(this.have_all_terminated()); - this.machine.data_race.set_ongoing_action_data_race_free(true); - } - /// Calls the callback with the "release" clock of the current thread. /// Other threads can acquire this clock in the future to establish synchronization /// with this program point. diff --git a/src/tools/miri/src/concurrency/init_once.rs b/src/tools/miri/src/concurrency/init_once.rs index c26384f65f6..165215f9270 100644 --- a/src/tools/miri/src/concurrency/init_once.rs +++ b/src/tools/miri/src/concurrency/init_once.rs @@ -1,13 +1,11 @@ +use std::cell::RefCell; use std::collections::VecDeque; - -use rustc_index::Idx; +use std::rc::Rc; use super::thread::DynUnblockCallback; use super::vector_clock::VClock; use crate::*; -super::sync::declare_id!(InitOnceId); - #[derive(Default, Debug, Copy, Clone, PartialEq, Eq)] /// The current status of a one time initialization. pub enum InitOnceStatus { @@ -25,44 +23,70 @@ pub(super) struct InitOnce { clock: VClock, } -impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} -pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { +impl InitOnce { #[inline] - fn init_once_status(&mut self, id: InitOnceId) -> InitOnceStatus { - let this = self.eval_context_ref(); - this.machine.sync.init_onces[id].status - } - - /// Put the thread into the queue waiting for the initialization. - #[inline] - fn init_once_enqueue_and_block(&mut self, id: InitOnceId, callback: DynUnblockCallback<'tcx>) { - let this = self.eval_context_mut(); - let thread = this.active_thread(); - let init_once = &mut this.machine.sync.init_onces[id]; - assert_ne!(init_once.status, InitOnceStatus::Complete, "queueing on complete init once"); - init_once.waiters.push_back(thread); - this.block_thread(BlockReason::InitOnce(id), None, callback); + pub fn status(&self) -> InitOnceStatus { + self.status } /// Begin initializing this InitOnce. Must only be called after checking that it is currently /// uninitialized. #[inline] - fn init_once_begin(&mut self, id: InitOnceId) { - let this = self.eval_context_mut(); - let init_once = &mut this.machine.sync.init_onces[id]; + pub fn begin(&mut self) { assert_eq!( - init_once.status, + self.status(), InitOnceStatus::Uninitialized, "beginning already begun or complete init once" ); - init_once.status = InitOnceStatus::Begun; + self.status = InitOnceStatus::Begun; } +} +#[derive(Default, Clone, Debug)] +pub struct InitOnceRef(Rc<RefCell<InitOnce>>); + +impl InitOnceRef { + pub fn new() -> Self { + Self(Default::default()) + } + + pub fn status(&self) -> InitOnceStatus { + self.0.borrow().status() + } + + pub fn begin(&self) { + self.0.borrow_mut().begin(); + } +} + +impl VisitProvenance for InitOnceRef { + // InitOnce contains no provenance. + fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {} +} + +impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} +pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { + /// Put the thread into the queue waiting for the initialization. #[inline] - fn init_once_complete(&mut self, id: InitOnceId) -> InterpResult<'tcx> { + fn init_once_enqueue_and_block( + &mut self, + init_once_ref: InitOnceRef, + callback: DynUnblockCallback<'tcx>, + ) { let this = self.eval_context_mut(); - let init_once = &mut this.machine.sync.init_onces[id]; + let thread = this.active_thread(); + let mut init_once = init_once_ref.0.borrow_mut(); + assert_ne!(init_once.status, InitOnceStatus::Complete, "queueing on complete init once"); + + init_once.waiters.push_back(thread); + this.block_thread(BlockReason::InitOnce, None, callback); + } + #[inline] + fn init_once_complete(&mut self, init_once_ref: &InitOnceRef) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + + let mut init_once = init_once_ref.0.borrow_mut(); assert_eq!( init_once.status, InitOnceStatus::Begun, @@ -79,17 +103,19 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Wake up everyone. // need to take the queue to avoid having `this` be borrowed multiple times - for waiter in std::mem::take(&mut init_once.waiters) { - this.unblock_thread(waiter, BlockReason::InitOnce(id))?; + let waiters = std::mem::take(&mut init_once.waiters); + drop(init_once); + for waiter in waiters { + this.unblock_thread(waiter, BlockReason::InitOnce)?; } interp_ok(()) } #[inline] - fn init_once_fail(&mut self, id: InitOnceId) -> InterpResult<'tcx> { + fn init_once_fail(&mut self, init_once_ref: &InitOnceRef) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let init_once = &mut this.machine.sync.init_onces[id]; + let mut init_once = init_once_ref.0.borrow_mut(); assert_eq!( init_once.status, InitOnceStatus::Begun, @@ -106,7 +132,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Wake up one waiting thread, so they can go ahead and try to init this. if let Some(waiter) = init_once.waiters.pop_front() { - this.unblock_thread(waiter, BlockReason::InitOnce(id))?; + drop(init_once); + this.unblock_thread(waiter, BlockReason::InitOnce)?; } interp_ok(()) @@ -115,15 +142,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Synchronize with the previous completion of an InitOnce. /// Must only be called after checking that it is complete. #[inline] - fn init_once_observe_completed(&mut self, id: InitOnceId) { + fn init_once_observe_completed(&mut self, init_once_ref: &InitOnceRef) { let this = self.eval_context_mut(); + let init_once = init_once_ref.0.borrow(); assert_eq!( - this.init_once_status(id), + init_once.status, InitOnceStatus::Complete, "observing the completion of incomplete init once" ); - this.acquire_clock(&this.machine.sync.init_onces[id].clock); + this.acquire_clock(&init_once.clock); } } diff --git a/src/tools/miri/src/concurrency/mod.rs b/src/tools/miri/src/concurrency/mod.rs index 17d0f3f5ff6..49bcc0d30b5 100644 --- a/src/tools/miri/src/concurrency/mod.rs +++ b/src/tools/miri/src/concurrency/mod.rs @@ -9,18 +9,9 @@ mod vector_clock; pub mod weak_memory; // Import either the real genmc adapter or a dummy module. -cfg_select! { - feature = "genmc" => { - mod genmc; - pub use self::genmc::{GenmcCtx, GenmcConfig}; - } - _ => { - #[path = "genmc/dummy.rs"] - mod genmc_dummy; - use self::genmc_dummy as genmc; - pub use self::genmc::{GenmcCtx, GenmcConfig}; - } -} +#[cfg_attr(not(feature = "genmc"), path = "genmc/dummy.rs")] +mod genmc; pub use self::data_race_handler::{AllocDataRaceHandler, GlobalDataRaceHandler}; +pub use self::genmc::{GenmcConfig, GenmcCtx}; pub use self::vector_clock::VClock; diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index 74379d6438d..179094db0fe 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -8,45 +8,10 @@ use std::time::Duration; use rustc_abi::Size; use rustc_data_structures::fx::FxHashMap; -use rustc_index::{Idx, IndexVec}; -use super::init_once::InitOnce; use super::vector_clock::VClock; use crate::*; -/// We cannot use the `newtype_index!` macro because we have to use 0 as a -/// sentinel value meaning that the identifier is not assigned. This is because -/// the pthreads static initializers initialize memory with zeros (see the -/// `src/shims/sync.rs` file). -macro_rules! declare_id { - ($name: ident) => { - /// 0 is used to indicate that the id was not yet assigned and, - /// therefore, is not a valid identifier. - #[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)] - pub struct $name(std::num::NonZero<u32>); - - impl $crate::VisitProvenance for $name { - fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {} - } - - impl Idx for $name { - fn new(idx: usize) -> Self { - // We use 0 as a sentinel value (see the comment above) and, - // therefore, need to shift by one when converting from an index - // into a vector. - let shifted_idx = u32::try_from(idx).unwrap().strict_add(1); - $name(std::num::NonZero::new(shifted_idx).unwrap()) - } - fn index(self) -> usize { - // See the comment in `Self::new`. - // (This cannot underflow because `self.0` is `NonZero<u32>`.) - usize::try_from(self.0.get() - 1).unwrap() - } - } - }; -} -pub(super) use declare_id; - /// The mutex state. #[derive(Default, Debug)] struct Mutex { @@ -64,8 +29,8 @@ struct Mutex { pub struct MutexRef(Rc<RefCell<Mutex>>); impl MutexRef { - fn new() -> Self { - MutexRef(Rc::new(RefCell::new(Mutex::default()))) + pub fn new() -> Self { + Self(Default::default()) } /// Get the id of the thread that currently owns this lock, or `None` if it is not locked. @@ -75,9 +40,8 @@ impl MutexRef { } impl VisitProvenance for MutexRef { - fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { - // Mutex contains no provenance. - } + // Mutex contains no provenance. + fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {} } /// The read-write lock state. @@ -138,8 +102,8 @@ impl RwLock { pub struct RwLockRef(Rc<RefCell<RwLock>>); impl RwLockRef { - fn new() -> Self { - RwLockRef(Rc::new(RefCell::new(RwLock::default()))) + pub fn new() -> Self { + Self(Default::default()) } pub fn is_locked(&self) -> bool { @@ -152,13 +116,10 @@ impl RwLockRef { } impl VisitProvenance for RwLockRef { - fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { - // RwLockRef contains no provenance. - } + // RwLock contains no provenance. + fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {} } -declare_id!(CondvarId); - /// The conditional variable state. #[derive(Default, Debug)] struct Condvar { @@ -171,6 +132,24 @@ struct Condvar { clock: VClock, } +#[derive(Default, Clone, Debug)] +pub struct CondvarRef(Rc<RefCell<Condvar>>); + +impl CondvarRef { + pub fn new() -> Self { + Self(Default::default()) + } + + pub fn is_awaited(&self) -> bool { + !self.0.borrow().waiters.is_empty() + } +} + +impl VisitProvenance for CondvarRef { + // Condvar contains no provenance. + fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {} +} + /// The futex state. #[derive(Default, Debug)] struct Futex { @@ -183,19 +162,22 @@ struct Futex { clock: VClock, } -#[derive(Default, Clone)] +#[derive(Default, Clone, Debug)] pub struct FutexRef(Rc<RefCell<Futex>>); impl FutexRef { + pub fn new() -> Self { + Self(Default::default()) + } + pub fn waiters(&self) -> usize { self.0.borrow().waiters.len() } } impl VisitProvenance for FutexRef { - fn visit_provenance(&self, _visit: &mut VisitWith<'_>) { - // No provenance in `Futex`. - } + // Futex contains no provenance. + fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {} } /// A thread waiting on a futex. @@ -207,13 +189,6 @@ struct FutexWaiter { bitset: u32, } -/// The state of all synchronization objects. -#[derive(Default, Debug)] -pub struct SynchronizationObjects { - condvars: IndexVec<CondvarId, Condvar>, - pub(super) init_onces: IndexVec<InitOnceId, InitOnce>, -} - // Private extension trait for local helper methods impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {} pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { @@ -237,23 +212,6 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { } } -impl SynchronizationObjects { - pub fn mutex_create(&mut self) -> MutexRef { - MutexRef::new() - } - pub fn rwlock_create(&mut self) -> RwLockRef { - RwLockRef::new() - } - - pub fn condvar_create(&mut self) -> CondvarId { - self.condvars.push(Default::default()) - } - - pub fn init_once_create(&mut self) -> InitOnceId { - self.init_onces.push(Default::default()) - } -} - impl<'tcx> AllocExtra<'tcx> { fn get_sync<T: 'static>(&self, offset: Size) -> Option<&T> { self.sync.get(&offset).and_then(|s| s.downcast_ref::<T>()) @@ -663,19 +621,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ); } - /// Is the conditional variable awaited? - #[inline] - fn condvar_is_awaited(&mut self, id: CondvarId) -> bool { - let this = self.eval_context_mut(); - !this.machine.sync.condvars[id].waiters.is_empty() - } - /// Release the mutex and let the current thread wait on the given condition variable. /// Once it is signaled, the mutex will be acquired and `retval_succ` will be written to `dest`. /// If the timeout happens first, `retval_timeout` will be written to `dest`. fn condvar_wait( &mut self, - condvar: CondvarId, + condvar_ref: CondvarRef, mutex_ref: MutexRef, timeout: Option<(TimeoutClock, TimeoutAnchor, Duration)>, retval_succ: Scalar, @@ -695,14 +646,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ); } let thread = this.active_thread(); - let waiters = &mut this.machine.sync.condvars[condvar].waiters; - waiters.push_back(thread); + + condvar_ref.0.borrow_mut().waiters.push_back(thread); this.block_thread( - BlockReason::Condvar(condvar), + BlockReason::Condvar, timeout, callback!( @capture<'tcx> { - condvar: CondvarId, + condvar_ref: CondvarRef, mutex_ref: MutexRef, retval_succ: Scalar, retval_timeout: Scalar, @@ -714,7 +665,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // The condvar was signaled. Make sure we get the clock for that. if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { data_race.acquire_clock( - &this.machine.sync.condvars[condvar].clock, + &condvar_ref.0.borrow().clock, &this.machine.threads, ); } @@ -725,7 +676,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { UnblockKind::TimedOut => { // We have to remove the waiter from the queue again. let thread = this.active_thread(); - let waiters = &mut this.machine.sync.condvars[condvar].waiters; + let waiters = &mut condvar_ref.0.borrow_mut().waiters; waiters.retain(|waiter| *waiter != thread); // Now get back the lock. this.condvar_reacquire_mutex(mutex_ref, retval_timeout, dest) @@ -739,9 +690,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Wake up some thread (if there is any) sleeping on the conditional /// variable. Returns `true` iff any thread was woken up. - fn condvar_signal(&mut self, id: CondvarId) -> InterpResult<'tcx, bool> { + fn condvar_signal(&mut self, condvar_ref: &CondvarRef) -> InterpResult<'tcx, bool> { let this = self.eval_context_mut(); - let condvar = &mut this.machine.sync.condvars[id]; + let mut condvar = condvar_ref.0.borrow_mut(); // Each condvar signal happens-before the end of the condvar wake if let Some(data_race) = this.machine.data_race.as_vclocks_ref() { @@ -750,7 +701,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let Some(waiter) = condvar.waiters.pop_front() else { return interp_ok(false); }; - this.unblock_thread(waiter, BlockReason::Condvar(id))?; + drop(condvar); + this.unblock_thread(waiter, BlockReason::Condvar)?; interp_ok(true) } diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index c8a408fd8ac..56c19794854 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -97,13 +97,13 @@ pub enum BlockReason { /// Blocked on a mutex. Mutex, /// Blocked on a condition variable. - Condvar(CondvarId), + Condvar, /// Blocked on a reader-writer lock. RwLock, /// Blocked on a Futex variable. Futex, /// Blocked on an InitOnce. - InitOnce(InitOnceId), + InitOnce, /// Blocked on epoll. Epoll, /// Blocked on eventfd. diff --git a/src/tools/miri/src/eval.rs b/src/tools/miri/src/eval.rs index 44612dae1ee..63578912c24 100644 --- a/src/tools/miri/src/eval.rs +++ b/src/tools/miri/src/eval.rs @@ -283,16 +283,6 @@ impl<'tcx> MainThreadState<'tcx> { // to be like a global `static`, so that all memory reached by it is considered to "not leak". this.terminate_active_thread(TlsAllocAction::Leak)?; - // Machine cleanup. Only do this if all threads have terminated; threads that are still running - // might cause Stacked Borrows errors (https://github.com/rust-lang/miri/issues/2396). - if this.have_all_terminated() { - // Even if all threads have terminated, we have to beware of data races since some threads - // might not have joined the main thread (https://github.com/rust-lang/miri/issues/2020, - // https://github.com/rust-lang/miri/issues/2508). - this.allow_data_races_all_threads_done(); - EnvVars::cleanup(this).expect("error during env var cleanup"); - } - // Stop interpreter loop. throw_machine_stop!(TerminationInfo::Exit { code: exit_code, leak_check: true }); } diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index 908f39af299..b259243602e 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -1333,7 +1333,6 @@ where /// Check that the number of varargs is at least the minimum what we expect. /// Fixed args should not be included. -/// Use `check_vararg_fixed_arg_count` to extract the varargs slice from full function arguments. pub fn check_min_vararg_count<'a, 'tcx, const N: usize>( name: &'a str, args: &'a [OpTy<'tcx>], diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 60bfdfb1fd9..c86e33e5185 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -125,10 +125,8 @@ pub use crate::concurrency::cpu_affinity::MAX_CPUS; pub use crate::concurrency::data_race::{ AtomicFenceOrd, AtomicReadOrd, AtomicRwOrd, AtomicWriteOrd, EvalContextExt as _, }; -pub use crate::concurrency::init_once::{EvalContextExt as _, InitOnceId}; -pub use crate::concurrency::sync::{ - CondvarId, EvalContextExt as _, MutexRef, RwLockRef, SynchronizationObjects, -}; +pub use crate::concurrency::init_once::{EvalContextExt as _, InitOnceRef}; +pub use crate::concurrency::sync::{CondvarRef, EvalContextExt as _, MutexRef, RwLockRef}; pub use crate::concurrency::thread::{ BlockReason, DynUnblockCallback, EvalContextExt as _, StackEmptyCallback, ThreadId, ThreadManager, TimeoutAnchor, TimeoutClock, UnblockKind, diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 17c13a9e33c..4f3dc485325 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -130,11 +130,11 @@ pub enum MiriMemoryKind { WinHeap, /// Windows "local" memory (to be freed with `LocalFree`) WinLocal, - /// Memory for args, errno, and other parts of the machine-managed environment. + /// Memory for args, errno, env vars, and other parts of the machine-managed environment. /// This memory may leak. Machine, - /// Memory allocated by the runtime (e.g. env vars). Separate from `Machine` - /// because we clean it up and leak-check it. + /// Memory allocated by the runtime, e.g. for readdir. Separate from `Machine` because we clean + /// it up (or expect the user to invoke operations that clean it up) and leak-check it. Runtime, /// Globals copied from `tcx`. /// This memory may leak. @@ -499,9 +499,6 @@ pub struct MiriMachine<'tcx> { /// in `sched_getaffinity` pub(crate) thread_cpu_affinity: FxHashMap<ThreadId, CpuAffinityMask>, - /// The state of the primitive synchronization objects. - pub(crate) sync: SynchronizationObjects, - /// Precomputed `TyLayout`s for primitive data types that are commonly used inside Miri. pub(crate) layouts: PrimitiveLayouts<'tcx>, @@ -713,7 +710,6 @@ impl<'tcx> MiriMachine<'tcx> { layouts, threads, thread_cpu_affinity, - sync: SynchronizationObjects::default(), static_roots: Vec::new(), profiler, string_cache: Default::default(), @@ -903,7 +899,6 @@ impl VisitProvenance for MiriMachine<'_> { let MiriMachine { threads, thread_cpu_affinity: _, - sync: _, tls, env_vars, main_fn_ret_place, @@ -1019,6 +1014,8 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> { const PANIC_ON_ALLOC_FAIL: bool = false; + const TRACING_ENABLED: bool = cfg!(feature = "tracing"); + #[inline(always)] fn enforce_alignment(ecx: &MiriInterpCx<'tcx>) -> bool { ecx.machine.check_alignment != AlignmentCheck::None diff --git a/src/tools/miri/src/shims/env.rs b/src/tools/miri/src/shims/env.rs index e99a8fd6e8c..689cd3a7269 100644 --- a/src/tools/miri/src/shims/env.rs +++ b/src/tools/miri/src/shims/env.rs @@ -59,15 +59,6 @@ impl<'tcx> EnvVars<'tcx> { interp_ok(()) } - pub(crate) fn cleanup(ecx: &mut InterpCx<'tcx, MiriMachine<'tcx>>) -> InterpResult<'tcx> { - let this = ecx.eval_context_mut(); - match this.machine.env_vars { - EnvVars::Unix(_) => UnixEnvVars::cleanup(this), - EnvVars::Windows(_) => interp_ok(()), // no cleanup needed - EnvVars::Uninit => interp_ok(()), - } - } - pub(crate) fn unix(&self) -> &UnixEnvVars<'tcx> { match self { EnvVars::Unix(env) => env, @@ -110,8 +101,30 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } } + /// Get the process identifier. fn get_pid(&self) -> u32 { let this = self.eval_context_ref(); if this.machine.communicate() { std::process::id() } else { 1000 } } + + /// Get an "OS" thread ID for the current thread. + fn get_current_tid(&self) -> u32 { + let this = self.eval_context_ref(); + self.get_tid(this.machine.threads.active_thread()) + } + + /// Get an "OS" thread ID for any thread. + fn get_tid(&self, thread: ThreadId) -> u32 { + let this = self.eval_context_ref(); + let index = thread.to_u32(); + let target_os = &this.tcx.sess.target.os; + if target_os == "linux" || target_os == "netbsd" { + // On Linux, the main thread has PID == TID so we uphold this. NetBSD also appears + // to exhibit the same behavior, though I can't find a citation. + this.get_pid().strict_add(index) + } else { + // Other platforms do not display any relationship between PID and TID. + index + } + } } diff --git a/src/tools/miri/src/shims/extern_static.rs b/src/tools/miri/src/shims/extern_static.rs index f02a3f425d1..49c0c380a08 100644 --- a/src/tools/miri/src/shims/extern_static.rs +++ b/src/tools/miri/src/shims/extern_static.rs @@ -55,7 +55,7 @@ impl<'tcx> MiriMachine<'tcx> { ecx, &["__cxa_thread_atexit_impl", "__clock_gettime64"], )?; - Self::weak_symbol_extern_statics(ecx, &["getrandom", "statx"])?; + Self::weak_symbol_extern_statics(ecx, &["getrandom", "gettid", "statx"])?; } "freebsd" => { Self::null_ptr_extern_statics(ecx, &["__cxa_thread_atexit_impl"])?; diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 1b66735ddb1..9ddba8c2b48 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -615,7 +615,9 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // This is a no-op shim that only exists to prevent making the allocator shims instantly stable. let [] = this.check_shim(abi, CanonAbi::Rust, link_name, args)?; } - name if name == this.mangle_internal_symbol("__rust_alloc_error_handler_should_panic_v2") => { + name if name + == this.mangle_internal_symbol("__rust_alloc_error_handler_should_panic_v2") => + { // Gets the value of the `oom` option. let [] = this.check_shim(abi, CanonAbi::Rust, link_name, args)?; let val = this.tcx.sess.opts.unstable_opts.oom.should_panic(); diff --git a/src/tools/miri/src/shims/time.rs b/src/tools/miri/src/shims/time.rs index 28f4ca5bb1b..2fc42c13edd 100644 --- a/src/tools/miri/src/shims/time.rs +++ b/src/tools/miri/src/shims/time.rs @@ -330,18 +330,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { interp_ok(Scalar::from_i32(0)) // KERN_SUCCESS } - fn nanosleep( - &mut self, - req_op: &OpTy<'tcx>, - _rem: &OpTy<'tcx>, // Signal handlers are not supported, so rem will never be written to. - ) -> InterpResult<'tcx, Scalar> { + fn nanosleep(&mut self, duration: &OpTy<'tcx>, rem: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); this.assert_target_os_is_unix("nanosleep"); - let req = this.deref_pointer_as(req_op, this.libc_ty_layout("timespec"))?; + let duration = this.deref_pointer_as(duration, this.libc_ty_layout("timespec"))?; + let _rem = this.read_pointer(rem)?; // Signal handlers are not supported, so rem will never be written to. - let duration = match this.read_timespec(&req)? { + let duration = match this.read_timespec(&duration)? { Some(duration) => duration, None => { return this.set_last_error_and_return_i32(LibcError("EINVAL")); diff --git a/src/tools/miri/src/shims/unix/env.rs b/src/tools/miri/src/shims/unix/env.rs index 604fb0974d2..eb4365e2004 100644 --- a/src/tools/miri/src/shims/unix/env.rs +++ b/src/tools/miri/src/shims/unix/env.rs @@ -1,6 +1,6 @@ +use std::env; use std::ffi::{OsStr, OsString}; use std::io::ErrorKind; -use std::{env, mem}; use rustc_abi::{FieldIdx, Size}; use rustc_data_structures::fx::FxHashMap; @@ -50,20 +50,6 @@ impl<'tcx> UnixEnvVars<'tcx> { interp_ok(UnixEnvVars { map: env_vars_machine, environ }) } - pub(crate) fn cleanup(ecx: &mut InterpCx<'tcx, MiriMachine<'tcx>>) -> InterpResult<'tcx> { - // Deallocate individual env vars. - let env_vars = mem::take(&mut ecx.machine.env_vars.unix_mut().map); - for (_name, ptr) in env_vars { - ecx.deallocate_ptr(ptr, None, MiriMemoryKind::Runtime.into())?; - } - // Deallocate environ var list. - let environ = &ecx.machine.env_vars.unix().environ; - let old_vars_ptr = ecx.read_pointer(environ)?; - ecx.deallocate_ptr(old_vars_ptr, None, MiriMemoryKind::Runtime.into())?; - - interp_ok(()) - } - pub(crate) fn environ(&self) -> Pointer { self.environ.ptr() } @@ -112,7 +98,7 @@ fn alloc_env_var<'tcx>( let mut name_osstring = name.to_os_string(); name_osstring.push("="); name_osstring.push(value); - ecx.alloc_os_str_as_c_str(name_osstring.as_os_str(), MiriMemoryKind::Runtime.into()) + ecx.alloc_os_str_as_c_str(name_osstring.as_os_str(), MiriMemoryKind::Machine.into()) } /// Allocates an `environ` block with the given list of pointers. @@ -128,7 +114,7 @@ fn alloc_environ_block<'tcx>( ecx.machine.layouts.mut_raw_ptr.ty, u64::try_from(vars.len()).unwrap(), ))?; - let vars_place = ecx.allocate(vars_layout, MiriMemoryKind::Runtime.into())?; + let vars_place = ecx.allocate(vars_layout, MiriMemoryKind::Machine.into())?; for (idx, var) in vars.into_iter_enumerated() { let place = ecx.project_field(&vars_place, idx)?; ecx.write_pointer(var, &place)?; @@ -171,7 +157,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { if let Some((name, value)) = new { let var_ptr = alloc_env_var(this, &name, &value)?; if let Some(var) = this.machine.env_vars.unix_mut().map.insert(name, var_ptr) { - this.deallocate_ptr(var, None, MiriMemoryKind::Runtime.into())?; + this.deallocate_ptr(var, None, MiriMemoryKind::Machine.into())?; } this.update_environ()?; interp_ok(Scalar::from_i32(0)) // return zero on success @@ -195,7 +181,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } if let Some(old) = success { if let Some(var) = old { - this.deallocate_ptr(var, None, MiriMemoryKind::Runtime.into())?; + this.deallocate_ptr(var, None, MiriMemoryKind::Machine.into())?; } this.update_environ()?; interp_ok(Scalar::from_i32(0)) @@ -253,7 +239,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Deallocate the old environ list. let environ = this.machine.env_vars.unix().environ.clone(); let old_vars_ptr = this.read_pointer(&environ)?; - this.deallocate_ptr(old_vars_ptr, None, MiriMemoryKind::Runtime.into())?; + this.deallocate_ptr(old_vars_ptr, None, MiriMemoryKind::Machine.into())?; // Write the new list. let vals = this.machine.env_vars.unix().map.values().copied().collect(); @@ -274,15 +260,52 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { interp_ok(Scalar::from_u32(this.get_pid())) } - fn linux_gettid(&mut self) -> InterpResult<'tcx, Scalar> { + /// The `gettid`-like function for Unix platforms that take no parameters and return a 32-bit + /// integer. It is not always named "gettid". + fn unix_gettid(&mut self, link_name: &str) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_ref(); - this.assert_target_os("linux", "gettid"); + this.assert_target_os_is_unix(link_name); + + // For most platforms the return type is an `i32`, but some are unsigned. The TID + // will always be positive so we don't need to differentiate. + interp_ok(Scalar::from_u32(this.get_current_tid())) + } - let index = this.machine.threads.active_thread().to_u32(); + /// The Apple-specific `int pthread_threadid_np(pthread_t thread, uint64_t *thread_id)`, which + /// allows querying the ID for arbitrary threads, identified by their pthread_t. + /// + /// API documentation: <https://www.manpagez.com/man/3/pthread_threadid_np/>. + fn apple_pthread_threadip_np( + &mut self, + thread_op: &OpTy<'tcx>, + tid_op: &OpTy<'tcx>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + this.assert_target_os("macos", "pthread_threadip_np"); + + let tid_dest = this.read_pointer(tid_op)?; + if this.ptr_is_null(tid_dest)? { + // If NULL is passed, an error is immediately returned + return interp_ok(this.eval_libc("EINVAL")); + } + + let thread = this.read_scalar(thread_op)?.to_int(this.libc_ty_layout("pthread_t").size)?; + let thread = if thread == 0 { + // Null thread ID indicates that we are querying the active thread. + this.machine.threads.active_thread() + } else { + // Our pthread_t is just the raw ThreadId. + let Ok(thread) = this.thread_id_try_from(thread) else { + return interp_ok(this.eval_libc("ESRCH")); + }; + thread + }; - // Compute a TID for this thread, ensuring that the main thread has PID == TID. - let tid = this.get_pid().strict_add(index); + let tid = this.get_tid(thread); + let tid_dest = this.deref_pointer_as(tid_op, this.machine.layouts.u64)?; + this.write_int(tid, &tid_dest)?; - interp_ok(Scalar::from_u32(tid)) + // Possible errors have been handled, return success. + interp_ok(Scalar::from_u32(0)) } } diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index b3c58397a02..438a9b420be 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -963,8 +963,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_null(dest)?; } "nanosleep" => { - let [req, rem] = this.check_shim(abi, CanonAbi::C, link_name, args)?; - let result = this.nanosleep(req, rem)?; + let [duration, rem] = this.check_shim(abi, CanonAbi::C, link_name, args)?; + let result = this.nanosleep(duration, rem)?; this.write_scalar(result, dest)?; } "sched_getaffinity" => { diff --git a/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs b/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs index 42502d5bf09..33564a2f84c 100644 --- a/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/freebsd/foreign_items.rs @@ -56,6 +56,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { }; this.write_scalar(res, dest)?; } + "pthread_getthreadid_np" => { + let [] = this.check_shim(abi, CanonAbi::C, link_name, args)?; + let result = this.unix_gettid(link_name.as_str())?; + this.write_scalar(result, dest)?; + } "cpuset_getaffinity" => { // The "same" kind of api as `sched_getaffinity` but more fine grained control for FreeBSD specifically. diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs index aeaff1cb13a..b3e99e6cc68 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs @@ -18,7 +18,7 @@ use crate::*; const TASK_COMM_LEN: u64 = 16; pub fn is_dyn_sym(name: &str) -> bool { - matches!(name, "statx") + matches!(name, "gettid" | "statx") } impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} @@ -117,7 +117,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } "gettid" => { let [] = this.check_shim(abi, CanonAbi::C, link_name, args)?; - let result = this.linux_gettid()?; + let result = this.unix_gettid(link_name.as_str())?; this.write_scalar(result, dest)?; } diff --git a/src/tools/miri/src/shims/unix/linux_like/sync.rs b/src/tools/miri/src/shims/unix/linux_like/sync.rs index 86e8b57824c..9fad74c0241 100644 --- a/src/tools/miri/src/shims/unix/linux_like/sync.rs +++ b/src/tools/miri/src/shims/unix/linux_like/sync.rs @@ -15,12 +15,13 @@ pub fn futex<'tcx>( ) -> InterpResult<'tcx> { let [addr, op, val] = check_min_vararg_count("`syscall(SYS_futex, ...)`", varargs)?; + // See <https://man7.org/linux/man-pages/man2/futex.2.html> for docs. // The first three arguments (after the syscall number itself) are the same to all futex operations: - // (int *addr, int op, int val). + // (uint32_t *addr, int op, uint32_t val). // We checked above that these definitely exist. let addr = ecx.read_pointer(addr)?; let op = ecx.read_scalar(op)?.to_i32()?; - let val = ecx.read_scalar(val)?.to_i32()?; + let val = ecx.read_scalar(val)?.to_u32()?; // This is a vararg function so we have to bring our own type for this pointer. let addr = ecx.ptr_to_mplace(addr, ecx.machine.layouts.i32); @@ -138,7 +139,7 @@ pub fn futex<'tcx>( // It's not uncommon for `addr` to be passed as another type than `*mut i32`, such as `*const AtomicI32`. // We do an acquire read -- it only seems reasonable that if we observe a value here, we // actually establish an ordering with that value. - let futex_val = ecx.read_scalar_atomic(&addr, AtomicReadOrd::Acquire)?.to_i32()?; + let futex_val = ecx.read_scalar_atomic(&addr, AtomicReadOrd::Acquire)?.to_u32()?; if val == futex_val { // The value still matches, so we block the thread and make it wait for FUTEX_WAKE. diff --git a/src/tools/miri/src/shims/unix/linux_like/syscall.rs b/src/tools/miri/src/shims/unix/linux_like/syscall.rs index d42d6b9073e..d3534e6e1bc 100644 --- a/src/tools/miri/src/shims/unix/linux_like/syscall.rs +++ b/src/tools/miri/src/shims/unix/linux_like/syscall.rs @@ -4,6 +4,7 @@ use rustc_span::Symbol; use rustc_target::callconv::FnAbi; use crate::helpers::check_min_vararg_count; +use crate::shims::unix::env::EvalContextExt; use crate::shims::unix::linux_like::eventfd::EvalContextExt as _; use crate::shims::unix::linux_like::sync::futex; use crate::*; @@ -24,6 +25,7 @@ pub fn syscall<'tcx>( let sys_getrandom = ecx.eval_libc("SYS_getrandom").to_target_usize(ecx)?; let sys_futex = ecx.eval_libc("SYS_futex").to_target_usize(ecx)?; let sys_eventfd2 = ecx.eval_libc("SYS_eventfd2").to_target_usize(ecx)?; + let sys_gettid = ecx.eval_libc("SYS_gettid").to_target_usize(ecx)?; match ecx.read_target_usize(op)? { // `libc::syscall(NR_GETRANDOM, buf.as_mut_ptr(), buf.len(), GRND_NONBLOCK)` @@ -53,6 +55,10 @@ pub fn syscall<'tcx>( let result = ecx.eventfd(initval, flags)?; ecx.write_int(result.to_i32()?, dest)?; } + num if num == sys_gettid => { + let result = ecx.unix_gettid("SYS_gettid")?; + ecx.write_int(result.to_u32()?, dest)?; + } num => { throw_unsup_format!("syscall: unsupported syscall number {num}"); } diff --git a/src/tools/miri/src/shims/unix/macos/foreign_items.rs b/src/tools/miri/src/shims/unix/macos/foreign_items.rs index ae921a013a4..23303718091 100644 --- a/src/tools/miri/src/shims/unix/macos/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/macos/foreign_items.rs @@ -222,6 +222,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { }; this.write_scalar(res, dest)?; } + "pthread_threadid_np" => { + let [thread, tid_ptr] = this.check_shim(abi, CanonAbi::C, link_name, args)?; + let res = this.apple_pthread_threadip_np(thread, tid_ptr)?; + this.write_scalar(res, dest)?; + } // Synchronization primitives "os_sync_wait_on_address" => { diff --git a/src/tools/miri/src/shims/unix/macos/sync.rs b/src/tools/miri/src/shims/unix/macos/sync.rs index 19f55e6c917..05616dd5a42 100644 --- a/src/tools/miri/src/shims/unix/macos/sync.rs +++ b/src/tools/miri/src/shims/unix/macos/sync.rs @@ -68,10 +68,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // LAZY_INIT_COOKIE). This can't be hit via `std::sync::Mutex`. interp_ok(MacOsUnfairLock::Poisoned) }, - |ecx| { - let mutex_ref = ecx.machine.sync.mutex_create(); - interp_ok(MacOsUnfairLock::Active { mutex_ref }) - }, + |_| interp_ok(MacOsUnfairLock::Active { mutex_ref: MutexRef::new() }), ) } } diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index 50eb4d92289..e20e3b79c3b 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -171,8 +171,7 @@ fn mutex_create<'tcx>( kind: MutexKind, ) -> InterpResult<'tcx, PthreadMutex> { let mutex = ecx.deref_pointer_as(mutex_ptr, ecx.libc_ty_layout("pthread_mutex_t"))?; - let id = ecx.machine.sync.mutex_create(); - let data = PthreadMutex { mutex_ref: id, kind }; + let data = PthreadMutex { mutex_ref: MutexRef::new(), kind }; ecx.lazy_sync_init(&mutex, mutex_init_offset(ecx)?, data.clone())?; interp_ok(data) } @@ -193,8 +192,7 @@ where || throw_ub_format!("`pthread_mutex_t` can't be moved after first use"), |ecx| { let kind = mutex_kind_from_static_initializer(ecx, &mutex)?; - let id = ecx.machine.sync.mutex_create(); - interp_ok(PthreadMutex { mutex_ref: id, kind }) + interp_ok(PthreadMutex { mutex_ref: MutexRef::new(), kind }) }, ) } @@ -278,8 +276,7 @@ where )? { throw_unsup_format!("unsupported static initializer used for `pthread_rwlock_t`"); } - let rwlock_ref = ecx.machine.sync.rwlock_create(); - interp_ok(PthreadRwLock { rwlock_ref }) + interp_ok(PthreadRwLock { rwlock_ref: RwLockRef::new() }) }, ) } @@ -372,9 +369,9 @@ enum ClockId { Monotonic, } -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Clone)] struct PthreadCondvar { - id: CondvarId, + condvar_ref: CondvarRef, clock: ClockId, } @@ -384,9 +381,8 @@ fn cond_create<'tcx>( clock: ClockId, ) -> InterpResult<'tcx, PthreadCondvar> { let cond = ecx.deref_pointer_as(cond_ptr, ecx.libc_ty_layout("pthread_cond_t"))?; - let id = ecx.machine.sync.condvar_create(); - let data = PthreadCondvar { id, clock }; - ecx.lazy_sync_init(&cond, cond_init_offset(ecx)?, data)?; + let data = PthreadCondvar { condvar_ref: CondvarRef::new(), clock }; + ecx.lazy_sync_init(&cond, cond_init_offset(ecx)?, data.clone())?; interp_ok(data) } @@ -411,8 +407,7 @@ where throw_unsup_format!("unsupported static initializer used for `pthread_cond_t`"); } // This used the static initializer. The clock there is always CLOCK_REALTIME. - let id = ecx.machine.sync.condvar_create(); - interp_ok(PthreadCondvar { id, clock: ClockId::Realtime }) + interp_ok(PthreadCondvar { condvar_ref: CondvarRef::new(), clock: ClockId::Realtime }) }, ) } @@ -817,15 +812,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn pthread_cond_signal(&mut self, cond_op: &OpTy<'tcx>) -> InterpResult<'tcx, ()> { let this = self.eval_context_mut(); - let id = cond_get_data(this, cond_op)?.id; - this.condvar_signal(id)?; + let condvar = cond_get_data(this, cond_op)?.condvar_ref.clone(); + this.condvar_signal(&condvar)?; interp_ok(()) } fn pthread_cond_broadcast(&mut self, cond_op: &OpTy<'tcx>) -> InterpResult<'tcx, ()> { let this = self.eval_context_mut(); - let id = cond_get_data(this, cond_op)?.id; - while this.condvar_signal(id)? {} + let condvar = cond_get_data(this, cond_op)?.condvar_ref.clone(); + while this.condvar_signal(&condvar)? {} interp_ok(()) } @@ -837,11 +832,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let data = *cond_get_data(this, cond_op)?; + let data = cond_get_data(this, cond_op)?.clone(); let mutex_ref = mutex_get_data(this, mutex_op)?.mutex_ref.clone(); this.condvar_wait( - data.id, + data.condvar_ref, mutex_ref, None, // no timeout Scalar::from_i32(0), @@ -861,7 +856,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let data = *cond_get_data(this, cond_op)?; + let data = cond_get_data(this, cond_op)?.clone(); let mutex_ref = mutex_get_data(this, mutex_op)?.mutex_ref.clone(); // Extract the timeout. @@ -884,7 +879,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { }; this.condvar_wait( - data.id, + data.condvar_ref, mutex_ref, Some((timeout_clock, TimeoutAnchor::Absolute, duration)), Scalar::from_i32(0), @@ -900,8 +895,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // Reading the field also has the side-effect that we detect double-`destroy` // since we make the field uninit below. - let id = cond_get_data(this, cond_op)?.id; - if this.condvar_is_awaited(id) { + let condvar = &cond_get_data(this, cond_op)?.condvar_ref; + if condvar.is_awaited() { throw_ub_format!("destroying an awaited conditional variable"); } diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs index de10357f5fa..959abc0baca 100644 --- a/src/tools/miri/src/shims/windows/foreign_items.rs +++ b/src/tools/miri/src/shims/windows/foreign_items.rs @@ -629,6 +629,23 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_scalar(name, &name_ptr)?; this.write_scalar(res, dest)?; } + "GetThreadId" => { + let [handle] = this.check_shim(abi, sys_conv, link_name, args)?; + let handle = this.read_handle(handle, "GetThreadId")?; + let thread = match handle { + Handle::Thread(thread) => thread, + Handle::Pseudo(PseudoHandle::CurrentThread) => this.active_thread(), + _ => this.invalid_handle("GetThreadDescription")?, + }; + let tid = this.get_tid(thread); + this.write_scalar(Scalar::from_u32(tid), dest)?; + } + "GetCurrentThreadId" => { + let [] = this.check_shim(abi, sys_conv, link_name, args)?; + let thread = this.active_thread(); + let tid = this.get_tid(thread); + this.write_scalar(Scalar::from_u32(tid), dest)?; + } // Miscellaneous "ExitProcess" => { diff --git a/src/tools/miri/src/shims/windows/handle.rs b/src/tools/miri/src/shims/windows/handle.rs index 1e30bf25ed9..8a965ea316d 100644 --- a/src/tools/miri/src/shims/windows/handle.rs +++ b/src/tools/miri/src/shims/windows/handle.rs @@ -166,6 +166,10 @@ impl Handle { /// Structurally invalid handles return [`HandleError::InvalidHandle`]. /// If the handle is structurally valid but semantically invalid, e.g. a for non-existent thread /// ID, returns [`HandleError::ThreadNotFound`]. + /// + /// This function is deliberately private; shims should always use `read_handle`. + /// That enforces handle validity even when Windows does not: for now, we argue invalid + /// handles are always a bug and programmers likely want to know about them. fn try_from_scalar<'tcx>( handle: Scalar, cx: &MiriInterpCx<'tcx>, diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index 8d5ea7db9e4..9165e76b63d 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -2,13 +2,13 @@ use std::time::Duration; use rustc_abi::Size; -use crate::concurrency::init_once::InitOnceStatus; +use crate::concurrency::init_once::{EvalContextExt as _, InitOnceStatus}; use crate::concurrency::sync::FutexRef; use crate::*; -#[derive(Copy, Clone)] +#[derive(Clone)] struct WindowsInitOnce { - id: InitOnceId, + init_once: InitOnceRef, } struct WindowsFutex { @@ -37,10 +37,9 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { &init_once, init_offset, || throw_ub_format!("`INIT_ONCE` can't be moved after first use"), - |this| { + |_| { // TODO: check that this is still all-zero. - let id = this.machine.sync.init_once_create(); - interp_ok(WindowsInitOnce { id }) + interp_ok(WindowsInitOnce { init_once: InitOnceRef::new() }) }, ) } @@ -48,20 +47,20 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Returns `true` if we were succssful, `false` if we would block. fn init_once_try_begin( &mut self, - id: InitOnceId, + init_once_ref: &InitOnceRef, pending_place: &MPlaceTy<'tcx>, dest: &MPlaceTy<'tcx>, ) -> InterpResult<'tcx, bool> { let this = self.eval_context_mut(); - interp_ok(match this.init_once_status(id) { + interp_ok(match init_once_ref.status() { InitOnceStatus::Uninitialized => { - this.init_once_begin(id); + init_once_ref.begin(); this.write_scalar(this.eval_windows("c", "TRUE"), pending_place)?; this.write_scalar(this.eval_windows("c", "TRUE"), dest)?; true } InitOnceStatus::Complete => { - this.init_once_observe_completed(id); + this.init_once_observe_completed(init_once_ref); this.write_scalar(this.eval_windows("c", "FALSE"), pending_place)?; this.write_scalar(this.eval_windows("c", "TRUE"), dest)?; true @@ -84,7 +83,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let id = this.init_once_get_data(init_once_op)?.id; + let init_once = this.init_once_get_data(init_once_op)?.init_once.clone(); let flags = this.read_scalar(flags_op)?.to_u32()?; // PBOOL is int* let pending_place = this.deref_pointer_as(pending_op, this.machine.layouts.i32)?; @@ -98,7 +97,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { throw_unsup_format!("non-null `lpContext` in `InitOnceBeginInitialize`"); } - if this.init_once_try_begin(id, &pending_place, dest)? { + if this.init_once_try_begin(&init_once, &pending_place, dest)? { // Done! return interp_ok(()); } @@ -106,16 +105,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // We have to block, and then try again when we are woken up. let dest = dest.clone(); this.init_once_enqueue_and_block( - id, + init_once.clone(), callback!( @capture<'tcx> { - id: InitOnceId, + init_once: InitOnceRef, pending_place: MPlaceTy<'tcx>, dest: MPlaceTy<'tcx>, } |this, unblock: UnblockKind| { assert_eq!(unblock, UnblockKind::Ready); - let ret = this.init_once_try_begin(id, &pending_place, &dest)?; + let ret = this.init_once_try_begin(&init_once, &pending_place, &dest)?; assert!(ret, "we were woken up but init_once_try_begin still failed"); interp_ok(()) } @@ -132,7 +131,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let id = this.init_once_get_data(init_once_op)?.id; + let init_once = this.init_once_get_data(init_once_op)?.init_once.clone(); let flags = this.read_scalar(flags_op)?.to_u32()?; let context = this.read_pointer(context_op)?; @@ -148,7 +147,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { throw_unsup_format!("non-null `lpContext` in `InitOnceBeginInitialize`"); } - if this.init_once_status(id) != InitOnceStatus::Begun { + if init_once.status() != InitOnceStatus::Begun { // The docs do not say anything about this case, but it seems better to not allow it. throw_ub_format!( "calling InitOnceComplete on a one time initialization that has not begun or is already completed" @@ -156,9 +155,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } if success { - this.init_once_complete(id)?; + this.init_once_complete(&init_once)?; } else { - this.init_once_fail(id)?; + this.init_once_fail(&init_once)?; } interp_ok(this.eval_windows("c", "TRUE")) diff --git a/src/tools/miri/tests/fail-dep/concurrency/windows_thread_invalid.rs b/src/tools/miri/tests/fail-dep/concurrency/windows_thread_invalid.rs new file mode 100644 index 00000000000..2e0729c9b49 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/windows_thread_invalid.rs @@ -0,0 +1,9 @@ +//! Ensure we error if thread functions are called with invalid handles +//@only-target: windows # testing Windows API + +use windows_sys::Win32::System::Threading::GetThreadId; + +fn main() { + let _tid = unsafe { GetThreadId(std::ptr::dangling_mut()) }; + //~^ ERROR: invalid handle +} diff --git a/src/tools/miri/tests/fail-dep/concurrency/windows_thread_invalid.stderr b/src/tools/miri/tests/fail-dep/concurrency/windows_thread_invalid.stderr new file mode 100644 index 00000000000..8d4b049b740 --- /dev/null +++ b/src/tools/miri/tests/fail-dep/concurrency/windows_thread_invalid.stderr @@ -0,0 +1,13 @@ +error: abnormal termination: invalid handle 1 passed to GetThreadId + --> tests/fail-dep/concurrency/windows_thread_invalid.rs:LL:CC + | +LL | let _tid = unsafe { GetThreadId(std::ptr::dangling_mut()) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ abnormal termination occurred here + | + = note: BACKTRACE: + = note: inside `main` at tests/fail-dep/concurrency/windows_thread_invalid.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs index 3ccfecc6fb3..4c852135367 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs +++ b/src/tools/miri/tests/native-lib/pass/ptr_read_access.rs @@ -1,7 +1,3 @@ -// Only works on Unix targets -//@ignore-target: windows wasm -//@only-on-host - fn main() { test_access_pointer(); test_access_simple(); diff --git a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs index bd4e0b23601..86a9c97f4ce 100644 --- a/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs +++ b/src/tools/miri/tests/native-lib/pass/ptr_write_access.rs @@ -1,6 +1,3 @@ -// Only works on Unix targets -//@ignore-target: windows wasm -//@only-on-host //@compile-flags: -Zmiri-permissive-provenance #![feature(box_as_ptr)] diff --git a/src/tools/miri/tests/native-lib/pass/scalar_arguments.rs b/src/tools/miri/tests/native-lib/pass/scalar_arguments.rs index c896bd8dd34..9e99977a692 100644 --- a/src/tools/miri/tests/native-lib/pass/scalar_arguments.rs +++ b/src/tools/miri/tests/native-lib/pass/scalar_arguments.rs @@ -1,7 +1,3 @@ -// Only works on Unix targets -//@ignore-target: windows wasm -//@only-on-host - extern "C" { fn add_one_int(x: i32) -> i32; fn add_int16(x: i16) -> i16; diff --git a/src/tools/miri/tests/native-lib/scalar_arguments.c b/src/tools/miri/tests/native-lib/scalar_arguments.c index acccf06f3df..8cf38f74413 100644 --- a/src/tools/miri/tests/native-lib/scalar_arguments.c +++ b/src/tools/miri/tests/native-lib/scalar_arguments.c @@ -22,11 +22,11 @@ EXPORT uint32_t get_unsigned_int(void) { return -10; } -EXPORT short add_int16(int16_t x) { +EXPORT int16_t add_int16(int16_t x) { return x + 3; } -EXPORT long add_short_to_long(int16_t x, int64_t y) { +EXPORT int64_t add_short_to_long(int16_t x, int64_t y) { return x + y; } diff --git a/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs b/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs index f8f1c554f0d..19d86f09595 100644 --- a/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs +++ b/src/tools/miri/tests/pass-dep/concurrency/linux-futex.rs @@ -33,6 +33,11 @@ fn wake_nobody() { 0, ); } + + // Wake u32::MAX waiters. + unsafe { + assert_eq!(libc::syscall(libc::SYS_futex, addr_of!(futex), libc::FUTEX_WAKE, u32::MAX), 0); + } } fn wake_dangling() { diff --git a/src/tools/miri/tests/pass-dep/shims/gettid.rs b/src/tools/miri/tests/pass-dep/shims/gettid.rs new file mode 100644 index 00000000000..b7a2fa49ef8 --- /dev/null +++ b/src/tools/miri/tests/pass-dep/shims/gettid.rs @@ -0,0 +1,183 @@ +//! Test for `gettid` and similar functions for retrieving an OS thread ID. +//@ revisions: with_isolation without_isolation +//@ [without_isolation] compile-flags: -Zmiri-disable-isolation + +#![feature(linkage)] + +fn gettid() -> u64 { + cfg_if::cfg_if! { + if #[cfg(any(target_os = "android", target_os = "linux"))] { + gettid_linux_like() + } else if #[cfg(target_os = "nto")] { + unsafe { libc::gettid() as u64 } + } else if #[cfg(target_os = "openbsd")] { + unsafe { libc::getthrid() as u64 } + } else if #[cfg(target_os = "freebsd")] { + unsafe { libc::pthread_getthreadid_np() as u64 } + } else if #[cfg(target_os = "netbsd")] { + unsafe { libc::_lwp_self() as u64 } + } else if #[cfg(any(target_os = "solaris", target_os = "illumos"))] { + // On Solaris and Illumos, the `pthread_t` is the OS TID. + unsafe { libc::pthread_self() as u64 } + } else if #[cfg(target_vendor = "apple")] { + let mut id = 0u64; + let status: libc::c_int = unsafe { libc::pthread_threadid_np(0, &mut id) }; + assert_eq!(status, 0); + id + } else if #[cfg(windows)] { + use windows_sys::Win32::System::Threading::GetCurrentThreadId; + unsafe { GetCurrentThreadId() as u64 } + } else { + compile_error!("platform has no gettid") + } + } +} + +/// Test the libc function, the syscall, and the extern symbol. +#[cfg(any(target_os = "android", target_os = "linux"))] +fn gettid_linux_like() -> u64 { + unsafe extern "C" { + #[linkage = "extern_weak"] + static gettid: Option<unsafe extern "C" fn() -> libc::pid_t>; + } + + let from_libc = unsafe { libc::gettid() as u64 }; + let from_sys = unsafe { libc::syscall(libc::SYS_gettid) as u64 }; + let from_static = unsafe { gettid.unwrap()() as u64 }; + + assert_eq!(from_libc, from_sys); + assert_eq!(from_libc, from_static); + + from_libc +} + +/// Specific platforms can query the tid of arbitrary threads from a `pthread_t` / `HANDLE` +#[cfg(any(target_vendor = "apple", windows))] +mod queried { + use std::ffi::c_void; + use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; + use std::{ptr, thread}; + + use super::*; + + static SPAWNED_TID: AtomicU64 = AtomicU64::new(0); + static CAN_JOIN: AtomicBool = AtomicBool::new(false); + + /// Save this thread's TID, give the spawning thread a chance to query it separately before + /// being joined. + fn thread_body() { + SPAWNED_TID.store(gettid(), Ordering::Relaxed); + + // Spin until the main thread has a chance to read this thread's ID + while !CAN_JOIN.load(Ordering::Relaxed) { + thread::yield_now(); + } + } + + /// Spawn a thread, query then return its TID. + #[cfg(target_vendor = "apple")] + fn spawn_update_join() -> u64 { + extern "C" fn thread_start(_data: *mut c_void) -> *mut c_void { + thread_body(); + ptr::null_mut() + } + + let mut t: libc::pthread_t = 0; + let mut spawned_tid_from_handle = 0u64; + + unsafe { + let res = libc::pthread_create(&mut t, ptr::null(), thread_start, ptr::null_mut()); + assert_eq!(res, 0, "failed to spawn thread"); + + let res = libc::pthread_threadid_np(t, &mut spawned_tid_from_handle); + assert_eq!(res, 0, "failed to query thread ID"); + CAN_JOIN.store(true, Ordering::Relaxed); + + let res = libc::pthread_join(t, ptr::null_mut()); + assert_eq!(res, 0, "failed to join thread"); + + // Apple also has two documented return values for invalid threads and null pointers + let res = libc::pthread_threadid_np(libc::pthread_t::MAX, &mut 0); + assert_eq!(res, libc::ESRCH, "expected ESRCH for invalid TID"); + let res = libc::pthread_threadid_np(0, ptr::null_mut()); + assert_eq!(res, libc::EINVAL, "invalid EINVAL for a null pointer"); + } + + spawned_tid_from_handle + } + + /// Spawn a thread, query then return its TID. + #[cfg(windows)] + fn spawn_update_join() -> u64 { + use windows_sys::Win32::Foundation::WAIT_FAILED; + use windows_sys::Win32::System::Threading::{ + CreateThread, GetThreadId, INFINITE, WaitForSingleObject, + }; + + extern "system" fn thread_start(_data: *mut c_void) -> u32 { + thread_body(); + 0 + } + + let spawned_tid_from_handle; + let mut tid_at_spawn = 0u32; + + unsafe { + let handle = + CreateThread(ptr::null(), 0, Some(thread_start), ptr::null(), 0, &mut tid_at_spawn); + assert!(!handle.is_null(), "failed to spawn thread"); + + spawned_tid_from_handle = GetThreadId(handle); + assert_ne!(spawned_tid_from_handle, 0, "failed to query thread ID"); + CAN_JOIN.store(true, Ordering::Relaxed); + + let res = WaitForSingleObject(handle, INFINITE); + assert_ne!(res, WAIT_FAILED, "failed to join thread"); + } + + // Windows also indirectly returns the TID from `CreateThread`, ensure that matches up. + assert_eq!(spawned_tid_from_handle, tid_at_spawn); + + spawned_tid_from_handle.into() + } + + pub fn check() { + let spawned_tid_from_handle = spawn_update_join(); + let spawned_tid_from_self = SPAWNED_TID.load(Ordering::Relaxed); + let current_tid = gettid(); + + // Ensure that we got a different thread ID. + assert_ne!(spawned_tid_from_handle, current_tid); + + // Ensure that we get the same result from `gettid` and from querying a thread's handle + assert_eq!(spawned_tid_from_handle, spawned_tid_from_self); + } +} + +fn main() { + let tid = gettid(); + + std::thread::spawn(move || { + assert_ne!(gettid(), tid); + }); + + // Test that in isolation mode a deterministic value will be returned. + // The value is not important, we only care that whatever the value is, + // won't change from execution to execution. + if cfg!(with_isolation) { + if cfg!(target_os = "linux") { + // Linux starts the TID at the PID, which is 1000. + assert_eq!(tid, 1000); + } else { + // Other platforms start counting from 0. + assert_eq!(tid, 0); + } + } + + // On Linux and NetBSD, the first TID is the PID. + #[cfg(any(target_os = "linux", target_os = "netbsd"))] + assert_eq!(tid, unsafe { libc::getpid() } as u64); + + #[cfg(any(target_vendor = "apple", windows))] + queried::check(); +} diff --git a/src/tools/miri/tests/pass/alloc-access-tracking.rs b/src/tools/miri/tests/pass/alloc-access-tracking.rs index b285250c8ab..0e88951dc43 100644 --- a/src/tools/miri/tests/pass/alloc-access-tracking.rs +++ b/src/tools/miri/tests/pass/alloc-access-tracking.rs @@ -1,7 +1,7 @@ #![no_std] #![no_main] -//@compile-flags: -Zmiri-track-alloc-id=18 -Zmiri-track-alloc-accesses -Cpanic=abort -//@normalize-stderr-test: "id 18" -> "id $$ALLOC" +//@compile-flags: -Zmiri-track-alloc-id=20 -Zmiri-track-alloc-accesses -Cpanic=abort +//@normalize-stderr-test: "id 20" -> "id $$ALLOC" //@only-target: linux # alloc IDs differ between OSes (due to extern static allocations) extern "Rust" { diff --git a/src/tools/miri/tests/pass/float.rs b/src/tools/miri/tests/pass/float.rs index 0fec1fb15eb..fe7316c6680 100644 --- a/src/tools/miri/tests/pass/float.rs +++ b/src/tools/miri/tests/pass/float.rs @@ -1066,7 +1066,6 @@ pub fn libm() { assert_eq!((-1f32).powf(f32::NEG_INFINITY), 1.0); assert_eq!((-1f64).powf(f64::NEG_INFINITY), 1.0); - assert_eq!(0f32.powi(10), 0.0); assert_eq!(0f64.powi(100), 0.0); assert_eq!(0f32.powi(9), 0.0); @@ -1490,7 +1489,6 @@ fn test_non_determinism() { test_operations_f64(19., 11.); test_operations_f128(25., 18.); - // SNaN^0 = (1 | NaN) ensure_nondet(|| f32::powf(SNAN_F32, 0.0).is_nan()); ensure_nondet(|| f64::powf(SNAN_F64, 0.0).is_nan()); diff --git a/src/tools/miri/tests/ui.rs b/src/tools/miri/tests/ui.rs index 46472b51f9c..5239f8338ee 100644 --- a/src/tools/miri/tests/ui.rs +++ b/src/tools/miri/tests/ui.rs @@ -29,20 +29,17 @@ fn miri_path() -> PathBuf { PathBuf::from(env::var("MIRI").unwrap_or_else(|_| env!("CARGO_BIN_EXE_miri").into())) } -fn get_host() -> String { - rustc_version::VersionMeta::for_command(std::process::Command::new(miri_path())) - .expect("failed to parse rustc version info") - .host -} - pub fn flagsplit(flags: &str) -> Vec<String> { // This code is taken from `RUSTFLAGS` handling in cargo. flags.split(' ').map(str::trim).filter(|s| !s.is_empty()).map(str::to_string).collect() } // Build the shared object file for testing native function calls. -fn build_native_lib() -> PathBuf { - let cc = env::var("CC").unwrap_or_else(|_| "cc".into()); +fn build_native_lib(target: &str) -> PathBuf { + // Loosely follow the logic of the `cc` crate for finding the compiler. + let cc = env::var(format!("CC_{target}")) + .or_else(|_| env::var("CC")) + .unwrap_or_else(|_| "cc".into()); // Target directory that we can write to. let so_target_dir = Path::new(env!("CARGO_TARGET_TMPDIR")).join("miri-native-lib"); // Create the directory if it does not already exist. @@ -201,7 +198,7 @@ fn run_tests( // If we're testing the native-lib functionality, then build the shared object file for testing // external C function calls and push the relevant compiler flag. if path.starts_with("tests/native-lib/") { - let native_lib = build_native_lib(); + let native_lib = build_native_lib(target); let mut flag = std::ffi::OsString::from("-Zmiri-native-lib="); flag.push(native_lib.into_os_string()); config.program.args.push(flag); @@ -305,14 +302,21 @@ fn ui( .with_context(|| format!("ui tests in {path} for {target} failed")) } -fn get_target() -> String { - env::var("MIRI_TEST_TARGET").ok().unwrap_or_else(get_host) +fn get_host() -> String { + rustc_version::VersionMeta::for_command(std::process::Command::new(miri_path())) + .expect("failed to parse rustc version info") + .host +} + +fn get_target(host: &str) -> String { + env::var("MIRI_TEST_TARGET").ok().unwrap_or_else(|| host.to_owned()) } fn main() -> Result<()> { ui_test::color_eyre::install()?; - let target = get_target(); + let host = get_host(); + let target = get_target(&host); let tmpdir = tempfile::Builder::new().prefix("miri-uitest-").tempdir()?; let mut args = std::env::args_os(); @@ -329,7 +333,7 @@ fn main() -> Result<()> { ui(Mode::Panic, "tests/panic", &target, WithDependencies, tmpdir.path())?; ui(Mode::Fail, "tests/fail", &target, WithoutDependencies, tmpdir.path())?; ui(Mode::Fail, "tests/fail-dep", &target, WithDependencies, tmpdir.path())?; - if cfg!(unix) { + if cfg!(unix) && target == host { ui(Mode::Pass, "tests/native-lib/pass", &target, WithoutDependencies, tmpdir.path())?; ui(Mode::Fail, "tests/native-lib/fail", &target, WithoutDependencies, tmpdir.path())?; } diff --git a/tests/ui/attributes/malformed-attrs.rs b/tests/ui/attributes/malformed-attrs.rs index dbe9c35b0a4..a09fe86557d 100644 --- a/tests/ui/attributes/malformed-attrs.rs +++ b/tests/ui/attributes/malformed-attrs.rs @@ -219,4 +219,11 @@ macro_rules! slump { () => {} } +#[ignore = 1] +//~^ ERROR valid forms for the attribute are +//~| WARN this was previously accepted by the compiler +fn thing() { + +} + fn main() {} diff --git a/tests/ui/attributes/malformed-attrs.stderr b/tests/ui/attributes/malformed-attrs.stderr index 32b0ddf87ba..9fe4f45b3ef 100644 --- a/tests/ui/attributes/malformed-attrs.stderr +++ b/tests/ui/attributes/malformed-attrs.stderr @@ -309,15 +309,6 @@ LL | #[link] = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #57571 <https://github.com/rust-lang/rust/issues/57571> -error: valid forms for the attribute are `#[ignore]` and `#[ignore = "reason"]` - --> $DIR/malformed-attrs.rs:93:1 - | -LL | #[ignore()] - | ^^^^^^^^^^^ - | - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #57571 <https://github.com/rust-lang/rust/issues/57571> - error: invalid argument --> $DIR/malformed-attrs.rs:187:1 | @@ -600,6 +591,24 @@ LL | #[inline = 5] = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #57571 <https://github.com/rust-lang/rust/issues/57571> +error: valid forms for the attribute are `#[ignore = "reason"]` and `#[ignore]` + --> $DIR/malformed-attrs.rs:93:1 + | +LL | #[ignore()] + | ^^^^^^^^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #57571 <https://github.com/rust-lang/rust/issues/57571> + +error: valid forms for the attribute are `#[ignore = "reason"]` and `#[ignore]` + --> $DIR/malformed-attrs.rs:222:1 + | +LL | #[ignore = 1] + | ^^^^^^^^^^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #57571 <https://github.com/rust-lang/rust/issues/57571> + error[E0308]: mismatched types --> $DIR/malformed-attrs.rs:110:23 | @@ -611,7 +620,7 @@ LL | #[coroutine = 63] || {} = note: expected unit type `()` found coroutine `{coroutine@$DIR/malformed-attrs.rs:110:23: 110:25}` -error: aborting due to 73 previous errors; 3 warnings emitted +error: aborting due to 74 previous errors; 3 warnings emitted Some errors have detailed explanations: E0308, E0463, E0539, E0565, E0658, E0805. For more information about an error, try `rustc --explain E0308`. diff --git a/tests/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr b/tests/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr index 9280dfdf92e..5d7d1caeeab 100644 --- a/tests/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr +++ b/tests/ui/feature-gates/issue-43106-gating-of-builtin-attrs.stderr @@ -367,12 +367,6 @@ warning: `#[should_panic]` only has an effect on functions LL | #![should_panic] | ^^^^^^^^^^^^^^^^ -warning: `#[ignore]` only has an effect on functions - --> $DIR/issue-43106-gating-of-builtin-attrs.rs:54:1 - | -LL | #![ignore] - | ^^^^^^^^^^ - warning: `#[proc_macro_derive]` only has an effect on functions --> $DIR/issue-43106-gating-of-builtin-attrs.rs:60:1 | @@ -387,6 +381,12 @@ LL | #![link()] | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! +warning: `#[ignore]` only has an effect on functions + --> $DIR/issue-43106-gating-of-builtin-attrs.rs:54:1 + | +LL | #![ignore] + | ^^^^^^^^^^ + warning: attribute should be applied to a foreign function or static --> $DIR/issue-43106-gating-of-builtin-attrs.rs:66:1 | diff --git a/tests/ui/lint/lint_map_unit_fn.stderr b/tests/ui/lint/lint_map_unit_fn.stderr index 91542af0f6d..930ecd30d1d 100644 --- a/tests/ui/lint/lint_map_unit_fn.stderr +++ b/tests/ui/lint/lint_map_unit_fn.stderr @@ -25,19 +25,18 @@ LL + x.iter_mut().for_each(foo); error: `Iterator::map` call that discard the iterator's values --> $DIR/lint_map_unit_fn.rs:11:18 | -LL | x.iter_mut().map(|items| { - | ^ ------- - | | | - | ____________________|___this function returns `()`, which is likely not what you wanted - | | __________________| - | | | -LL | | | -LL | | | items.sort(); -LL | | | }); - | | | -^ after this call to map, the resulting iterator is `impl Iterator<Item = ()>`, which means the only information carried by the iterator is the number of items - | | |_____|| - | |_______| - | called `Iterator::map` with callable that returns `()` +LL | x.iter_mut().map(|items| { + | ^ ------- + | | | + | ___________________|___this function returns `()`, which is likely not what you wanted + | | __________________| + | || +LL | || +LL | || items.sort(); +LL | || }); + | ||_____-^ after this call to map, the resulting iterator is `impl Iterator<Item = ()>`, which means the only information carried by the iterator is the number of items + | |______| + | called `Iterator::map` with callable that returns `()` | = note: `Iterator::map`, like many of the methods on `Iterator`, gets executed lazily, meaning that its effects won't be visible until it is iterated help: you might have meant to use `Iterator::for_each` diff --git a/tests/ui/lint/unused/unused-attr-duplicate.stderr b/tests/ui/lint/unused/unused-attr-duplicate.stderr index eff478d04ae..275eb056305 100644 --- a/tests/ui/lint/unused/unused-attr-duplicate.stderr +++ b/tests/ui/lint/unused/unused-attr-duplicate.stderr @@ -41,18 +41,6 @@ LL | #[path = "auxiliary/lint_unused_extern_crate.rs"] = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! error: unused attribute - --> $DIR/unused-attr-duplicate.rs:53:1 - | -LL | #[ignore = "some text"] - | ^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute - | -note: attribute also specified here - --> $DIR/unused-attr-duplicate.rs:52:1 - | -LL | #[ignore] - | ^^^^^^^^^ - -error: unused attribute --> $DIR/unused-attr-duplicate.rs:55:1 | LL | #[should_panic(expected = "values don't match")] @@ -166,6 +154,18 @@ LL | #[macro_export] | ^^^^^^^^^^^^^^^ error: unused attribute + --> $DIR/unused-attr-duplicate.rs:53:1 + | +LL | #[ignore = "some text"] + | ^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute + | +note: attribute also specified here + --> $DIR/unused-attr-duplicate.rs:52:1 + | +LL | #[ignore] + | ^^^^^^^^^ + +error: unused attribute --> $DIR/unused-attr-duplicate.rs:60:1 | LL | #[must_use = "some message"] diff --git a/tests/ui/malformed/malformed-regressions.stderr b/tests/ui/malformed/malformed-regressions.stderr index 535db55a13d..8c22919a1c2 100644 --- a/tests/ui/malformed/malformed-regressions.stderr +++ b/tests/ui/malformed/malformed-regressions.stderr @@ -8,15 +8,6 @@ LL | #[doc] = note: for more information, see issue #57571 <https://github.com/rust-lang/rust/issues/57571> = note: `#[deny(ill_formed_attribute_input)]` on by default -error: valid forms for the attribute are `#[ignore]` and `#[ignore = "reason"]` - --> $DIR/malformed-regressions.rs:3:1 - | -LL | #[ignore()] - | ^^^^^^^^^^^ - | - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #57571 <https://github.com/rust-lang/rust/issues/57571> - error: attribute must be of the form `#[link(name = "...", /*opt*/ kind = "dylib|static|...", /*opt*/ wasm_import_module = "...", /*opt*/ import_name_type = "decorated|noprefix|undecorated")]` --> $DIR/malformed-regressions.rs:7:1 | @@ -35,6 +26,15 @@ LL | #[link = ""] = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #57571 <https://github.com/rust-lang/rust/issues/57571> +error: valid forms for the attribute are `#[ignore = "reason"]` and `#[ignore]` + --> $DIR/malformed-regressions.rs:3:1 + | +LL | #[ignore()] + | ^^^^^^^^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #57571 <https://github.com/rust-lang/rust/issues/57571> + error: valid forms for the attribute are `#[inline(always|never)]` and `#[inline]` --> $DIR/malformed-regressions.rs:5:1 | |