From f6bbc80619bb5efb2200a1085fcb0d1d5d6922f3 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 23 Sep 2023 02:37:08 +0000 Subject: [PATCH 1/3] Don't ICE when encountering asm sym with regions --- compiler/rustc_borrowck/src/universal_regions.rs | 13 ++++++++++++- tests/ui/asm/global-asm-with-regions-in-sym.rs | 5 +++++ 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 tests/ui/asm/global-asm-with-regions-in-sym.rs diff --git a/compiler/rustc_borrowck/src/universal_regions.rs b/compiler/rustc_borrowck/src/universal_regions.rs index 02b52784edec1..1e6fe193459d7 100644 --- a/compiler/rustc_borrowck/src/universal_regions.rs +++ b/compiler/rustc_borrowck/src/universal_regions.rs @@ -15,6 +15,7 @@ use rustc_data_structures::fx::FxHashMap; use rustc_errors::Diagnostic; use rustc_hir as hir; +use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::lang_items::LangItem; use rustc_hir::BodyOwnerKind; @@ -711,7 +712,17 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> { // For a constant body, there are no inputs, and one // "output" (the type of the constant). assert_eq!(self.mir_def.to_def_id(), def_id); - let ty = tcx.type_of(self.mir_def).instantiate_identity(); + let mut ty = tcx.type_of(self.mir_def).instantiate_identity(); + // `SymFn` in global asm may only reference `'static`, but those + // regions are erased as part of computing the type of the anon + // const. Put them back, since the `UniversalRegionsBuilder` is + // not setup to handle erased regions. + if tcx.def_kind(tcx.parent(def_id)) == DefKind::GlobalAsm { + ty = tcx.fold_regions(ty, |re, _| { + assert_eq!(re, tcx.lifetimes.re_erased); + tcx.lifetimes.re_static + }); + } let ty = indices.fold_to_region_vids(tcx, ty); ty::Binder::dummy(tcx.mk_type_list(&[ty])) } diff --git a/tests/ui/asm/global-asm-with-regions-in-sym.rs b/tests/ui/asm/global-asm-with-regions-in-sym.rs new file mode 100644 index 0000000000000..25bc9620d9184 --- /dev/null +++ b/tests/ui/asm/global-asm-with-regions-in-sym.rs @@ -0,0 +1,5 @@ +// build-pass + +core::arch::global_asm!("/* {} */", sym <&'static ()>::clone); + +fn main() {} From 10e62c32538e95d7d73f16712125adc4d0ed1099 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 23 Sep 2023 03:10:38 +0000 Subject: [PATCH 2/3] Handle lifetimes correctly in in sym in line-asm --- compiler/rustc_ast_lowering/src/asm.rs | 37 +++++--- compiler/rustc_ast_lowering/src/expr.rs | 2 +- compiler/rustc_ast_lowering/src/item.rs | 4 +- compiler/rustc_codegen_ssa/src/mono_item.rs | 93 ++++++++++--------- compiler/rustc_hir/src/hir.rs | 10 +- compiler/rustc_hir/src/intravisit.rs | 7 +- .../src/check/intrinsicck.rs | 31 +++++-- .../src/collect/generics_of.rs | 2 +- .../rustc_hir_analysis/src/collect/type_of.rs | 2 +- compiler/rustc_hir_pretty/src/lib.rs | 7 +- compiler/rustc_hir_typeck/src/expr.rs | 6 +- .../rustc_hir_typeck/src/expr_use_visitor.rs | 5 +- compiler/rustc_hir_typeck/src/lib.rs | 4 +- compiler/rustc_middle/src/thir.rs | 5 +- compiler/rustc_middle/src/thir/visit.rs | 5 +- .../rustc_mir_build/src/build/expr/into.rs | 7 +- compiler/rustc_mir_build/src/thir/cx/expr.rs | 7 +- compiler/rustc_mir_build/src/thir/print.rs | 10 +- compiler/rustc_monomorphize/src/collector.rs | 11 ++- compiler/rustc_passes/src/liveness.rs | 6 +- compiler/rustc_passes/src/naked_functions.rs | 5 +- .../inline-asm-with-regions-in-sym.bad.stderr | 14 +++ .../ui/asm/inline-asm-with-regions-in-sym.rs | 21 +++++ tests/ui/asm/x86_64/type-check-2.rs | 30 +++--- tests/ui/asm/x86_64/type-check-2.stderr | 36 +++---- 25 files changed, 248 insertions(+), 119 deletions(-) create mode 100644 tests/ui/asm/inline-asm-with-regions-in-sym.bad.stderr create mode 100644 tests/ui/asm/inline-asm-with-regions-in-sym.rs diff --git a/compiler/rustc_ast_lowering/src/asm.rs b/compiler/rustc_ast_lowering/src/asm.rs index a1e626996800f..face8bf6dbd50 100644 --- a/compiler/rustc_ast_lowering/src/asm.rs +++ b/compiler/rustc_ast_lowering/src/asm.rs @@ -26,6 +26,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { &mut self, sp: Span, asm: &InlineAsm, + is_global: bool, ) -> &'hir hir::InlineAsm<'hir> { // Rustdoc needs to support asm! from foreign architectures: don't try // lowering the register constraints in this case. @@ -221,18 +222,24 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { tokens: None, }; - // Wrap the expression in an AnonConst. - let parent_def_id = self.current_hir_id_owner; - let node_id = self.next_node_id(); - self.create_def( - parent_def_id.def_id, - node_id, - DefPathData::AnonConst, - *op_sp, - ); - let anon_const = AnonConst { id: node_id, value: P(expr) }; - hir::InlineAsmOperand::SymFn { - anon_const: self.lower_anon_const(&anon_const), + if is_global { + // Wrap the expression in an AnonConst. + let parent_def_id = self.current_hir_id_owner; + let node_id = self.next_node_id(); + self.create_def( + parent_def_id.def_id, + node_id, + DefPathData::AnonConst, + *op_sp, + ); + let anon_const = AnonConst { id: node_id, value: P(expr) }; + hir::InlineAsmOperand::SymFnInGlobal { + anon_const: self.lower_anon_const(&anon_const), + } + } else { + hir::InlineAsmOperand::SymFnInInline { + expr: self.lower_expr(&expr), + } } } } @@ -288,7 +295,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { op_span: op_sp, }); } - hir::InlineAsmOperand::SymFn { .. } + hir::InlineAsmOperand::SymFnInGlobal { .. } + | hir::InlineAsmOperand::SymFnInInline { .. } | hir::InlineAsmOperand::SymStatic { .. } => { sess.emit_err(InvalidAsmTemplateModifierSym { placeholder_span, @@ -333,7 +341,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { | hir::InlineAsmOperand::SplitInOut { .. } => (true, true), hir::InlineAsmOperand::Const { .. } - | hir::InlineAsmOperand::SymFn { .. } + | hir::InlineAsmOperand::SymFnInGlobal { .. } + | hir::InlineAsmOperand::SymFnInInline { .. } | hir::InlineAsmOperand::SymStatic { .. } => { unreachable!() } diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 57c54f8540c78..a15eb1d07e1a2 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -283,7 +283,7 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::ExprKind::Become(sub_expr) } ExprKind::InlineAsm(asm) => { - hir::ExprKind::InlineAsm(self.lower_inline_asm(e.span, asm)) + hir::ExprKind::InlineAsm(self.lower_inline_asm(e.span, asm, false)) } ExprKind::FormatArgs(fmt) => self.lower_format_args(e.span, fmt), ExprKind::OffsetOf(container, fields) => hir::ExprKind::OffsetOf( diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index edc1e2f0b84d9..57e951ad0b2cb 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -311,7 +311,9 @@ impl<'hir> LoweringContext<'_, 'hir> { .arena .alloc_from_iter(fm.items.iter().map(|x| self.lower_foreign_item_ref(x))), }, - ItemKind::GlobalAsm(asm) => hir::ItemKind::GlobalAsm(self.lower_inline_asm(span, asm)), + ItemKind::GlobalAsm(asm) => { + hir::ItemKind::GlobalAsm(self.lower_inline_asm(span, asm, true)) + } ItemKind::TyAlias(box TyAlias { generics, where_clauses, ty, .. }) => { // We lower // diff --git a/compiler/rustc_codegen_ssa/src/mono_item.rs b/compiler/rustc_codegen_ssa/src/mono_item.rs index 6fbf992eda9ec..616867464795b 100644 --- a/compiler/rustc_codegen_ssa/src/mono_item.rs +++ b/compiler/rustc_codegen_ssa/src/mono_item.rs @@ -35,52 +35,55 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> { MonoItem::GlobalAsm(item_id) => { let item = cx.tcx().hir().item(item_id); if let hir::ItemKind::GlobalAsm(ref asm) = item.kind { - let operands: Vec<_> = asm - .operands - .iter() - .map(|(op, op_sp)| match *op { - hir::InlineAsmOperand::Const { ref anon_const } => { - let const_value = cx - .tcx() - .const_eval_poly(anon_const.def_id.to_def_id()) - .unwrap_or_else(|_| { - span_bug!(*op_sp, "asm const cannot be resolved") - }); - let ty = cx - .tcx() - .typeck_body(anon_const.body) - .node_type(anon_const.hir_id); - let string = common::asm_const_to_str( - cx.tcx(), - *op_sp, - const_value, - cx.layout_of(ty), - ); - GlobalAsmOperandRef::Const { string } - } - hir::InlineAsmOperand::SymFn { ref anon_const } => { - let ty = cx - .tcx() - .typeck_body(anon_const.body) - .node_type(anon_const.hir_id); - let instance = match ty.kind() { - &ty::FnDef(def_id, args) => Instance::new(def_id, args), - _ => span_bug!(*op_sp, "asm sym is not a function"), - }; + let operands: Vec<_> = + asm.operands + .iter() + .map(|(op, op_sp)| match *op { + hir::InlineAsmOperand::Const { ref anon_const } => { + let const_value = cx + .tcx() + .const_eval_poly(anon_const.def_id.to_def_id()) + .unwrap_or_else(|_| { + span_bug!(*op_sp, "asm const cannot be resolved") + }); + let ty = cx + .tcx() + .typeck_body(anon_const.body) + .node_type(anon_const.hir_id); + let string = common::asm_const_to_str( + cx.tcx(), + *op_sp, + const_value, + cx.layout_of(ty), + ); + GlobalAsmOperandRef::Const { string } + } + hir::InlineAsmOperand::SymFnInGlobal { ref anon_const } => { + let fn_ty = + cx.tcx().type_of(anon_const.def_id).no_bound_vars().expect( + "`sym` in `global_asm!` should not have generics", + ); + let instance = match fn_ty.kind() { + &ty::FnDef(def_id, args) => Instance::new(def_id, args), + _ => span_bug!(*op_sp, "asm sym is not a function"), + }; - GlobalAsmOperandRef::SymFn { instance } - } - hir::InlineAsmOperand::SymStatic { path: _, def_id } => { - GlobalAsmOperandRef::SymStatic { def_id } - } - hir::InlineAsmOperand::In { .. } - | hir::InlineAsmOperand::Out { .. } - | hir::InlineAsmOperand::InOut { .. } - | hir::InlineAsmOperand::SplitInOut { .. } => { - span_bug!(*op_sp, "invalid operand type for global_asm!") - } - }) - .collect(); + GlobalAsmOperandRef::SymFn { instance } + } + hir::InlineAsmOperand::SymFnInInline { .. } => { + bug!("should not encounter `SymFnInInline` in `global_asm!`"); + } + hir::InlineAsmOperand::SymStatic { path: _, def_id } => { + GlobalAsmOperandRef::SymStatic { def_id } + } + hir::InlineAsmOperand::In { .. } + | hir::InlineAsmOperand::Out { .. } + | hir::InlineAsmOperand::InOut { .. } + | hir::InlineAsmOperand::SplitInOut { .. } => { + span_bug!(*op_sp, "invalid operand type for global_asm!") + } + }) + .collect(); cx.codegen_global_asm(asm.template, &operands, asm.options, asm.line_spans); } else { diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 3eec66611edd1..661edfad0cd6e 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -2766,9 +2766,12 @@ pub enum InlineAsmOperand<'hir> { Const { anon_const: AnonConst, }, - SymFn { + SymFnInGlobal { anon_const: AnonConst, }, + SymFnInInline { + expr: &'hir Expr<'hir>, + }, SymStatic { path: QPath<'hir>, def_id: DefId, @@ -2782,7 +2785,10 @@ impl<'hir> InlineAsmOperand<'hir> { | Self::Out { reg, .. } | Self::InOut { reg, .. } | Self::SplitInOut { reg, .. } => Some(reg), - Self::Const { .. } | Self::SymFn { .. } | Self::SymStatic { .. } => None, + Self::Const { .. } + | Self::SymFnInGlobal { .. } + | Self::SymFnInInline { .. } + | Self::SymStatic { .. } => None, } } diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index d9195a374c24a..c7fc229445f06 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -1219,7 +1219,12 @@ pub fn walk_inline_asm<'v, V: Visitor<'v>>(visitor: &mut V, asm: &'v InlineAsm<' } } InlineAsmOperand::Const { anon_const, .. } - | InlineAsmOperand::SymFn { anon_const, .. } => visitor.visit_anon_const(anon_const), + | InlineAsmOperand::SymFnInGlobal { anon_const, .. } => { + visitor.visit_anon_const(anon_const) + } + InlineAsmOperand::SymFnInInline { expr } => { + visitor.visit_expr(expr); + } InlineAsmOperand::SymStatic { path, .. } => visitor.visit_qpath(path, id, *op_sp), } } diff --git a/compiler/rustc_hir_analysis/src/check/intrinsicck.rs b/compiler/rustc_hir_analysis/src/check/intrinsicck.rs index cd7e991720454..56874ee1d5cec 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsicck.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsicck.rs @@ -11,7 +11,7 @@ use rustc_target::asm::{InlineAsmReg, InlineAsmRegClass, InlineAsmRegOrRegClass, pub struct InlineAsmCtxt<'a, 'tcx> { tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, - get_operand_ty: Box) -> Ty<'tcx> + 'a>, + get_expr_ty: Box) -> Ty<'tcx> + 'a>, } impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> { @@ -19,16 +19,16 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> { InlineAsmCtxt { tcx, param_env: ty::ParamEnv::empty(), - get_operand_ty: Box::new(|e| bug!("asm operand in global asm: {e:?}")), + get_expr_ty: Box::new(|e| bug!("asm operand in global asm: {e:?}")), } } pub fn new_in_fn( tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, - get_operand_ty: impl Fn(&'tcx hir::Expr<'tcx>) -> Ty<'tcx> + 'a, + get_expr_ty: impl Fn(&'tcx hir::Expr<'tcx>) -> Ty<'tcx> + 'a, ) -> Self { - InlineAsmCtxt { tcx, param_env, get_operand_ty: Box::new(get_operand_ty) } + InlineAsmCtxt { tcx, param_env, get_expr_ty: Box::new(get_expr_ty) } } // FIXME(compiler-errors): This could use `<$ty as Pointee>::Metadata == ()` @@ -124,7 +124,7 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> { tied_input: Option<(&'tcx hir::Expr<'tcx>, Option)>, target_features: &FxIndexSet, ) -> Option { - let ty = (self.get_operand_ty)(expr); + let ty = (self.get_expr_ty)(expr); if ty.has_non_region_infer() { bug!("inference variable in asm operand ty: {:?} {:?}", expr, ty); } @@ -178,7 +178,7 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> { let msg = "incompatible types for asm inout argument"; let mut err = self.tcx.sess.struct_span_err(vec![in_expr.span, expr.span], msg); - let in_expr_ty = (self.get_operand_ty)(in_expr); + let in_expr_ty = (self.get_expr_ty)(in_expr); err.span_label(in_expr.span, format!("type `{in_expr_ty}`")); err.span_label(expr.span, format!("type `{ty}`")); err.note( @@ -437,7 +437,7 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> { hir::InlineAsmOperand::Const { .. } | hir::InlineAsmOperand::SymStatic { .. } => {} // Check that sym actually points to a function. Later passes // depend on this. - hir::InlineAsmOperand::SymFn { anon_const } => { + hir::InlineAsmOperand::SymFnInGlobal { anon_const } => { let ty = self.tcx.type_of(anon_const.def_id).instantiate_identity(); match ty.kind() { ty::Never | ty::Error(_) => {} @@ -454,6 +454,23 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> { } }; } + hir::InlineAsmOperand::SymFnInInline { expr } => { + let ty = (self.get_expr_ty)(expr); + match ty.kind() { + ty::Never | ty::Error(_) => {} + ty::FnDef(..) => {} + _ => { + let mut err = + self.tcx.sess.struct_span_err(*op_sp, "invalid `sym` operand"); + err.span_label( + expr.span, + format!("is {} `{}`", ty.kind().article(), ty), + ); + err.help("`sym` operands must refer to either a function or a static"); + err.emit(); + } + }; + } } } } diff --git a/compiler/rustc_hir_analysis/src/collect/generics_of.rs b/compiler/rustc_hir_analysis/src/collect/generics_of.rs index 3d60c57b9d5ad..3e83c92a48ed0 100644 --- a/compiler/rustc_hir_analysis/src/collect/generics_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/generics_of.rs @@ -128,7 +128,7 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { Node::Expr(&Expr { kind: ExprKind::InlineAsm(asm), .. }) if asm.operands.iter().any(|(op, _op_sp)| match op { hir::InlineAsmOperand::Const { anon_const } - | hir::InlineAsmOperand::SymFn { anon_const } => { + | hir::InlineAsmOperand::SymFnInGlobal { anon_const } => { anon_const.hir_id == hir_id } _ => false, diff --git a/compiler/rustc_hir_analysis/src/collect/type_of.rs b/compiler/rustc_hir_analysis/src/collect/type_of.rs index d12337687e2e1..fa8e9007f72ce 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of.rs @@ -39,7 +39,7 @@ fn anon_const_type_of<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> Ty<'tcx> { | Node::Item(&Item { kind: ItemKind::GlobalAsm(asm), .. }) if asm.operands.iter().any(|(op, _op_sp)| match op { hir::InlineAsmOperand::Const { anon_const } - | hir::InlineAsmOperand::SymFn { anon_const } => anon_const.hir_id == hir_id, + | hir::InlineAsmOperand::SymFnInGlobal { anon_const } => anon_const.hir_id == hir_id, _ => false, }) => { diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs index 8587b009f25aa..b806b379a1639 100644 --- a/compiler/rustc_hir_pretty/src/lib.rs +++ b/compiler/rustc_hir_pretty/src/lib.rs @@ -1309,11 +1309,16 @@ impl<'a> State<'a> { s.space(); s.print_anon_const(anon_const); } - hir::InlineAsmOperand::SymFn { ref anon_const } => { + hir::InlineAsmOperand::SymFnInGlobal { ref anon_const } => { s.word("sym_fn"); s.space(); s.print_anon_const(anon_const); } + hir::InlineAsmOperand::SymFnInInline { ref expr } => { + s.word("sym_fn"); + s.space(); + s.print_expr(expr); + } hir::InlineAsmOperand::SymStatic { ref path, def_id: _ } => { s.word("sym_static"); s.space(); diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 4ad14ce3059f3..1abfefd6cd86c 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -3113,10 +3113,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.check_expr_asm_operand(out_expr, false); } } + hir::InlineAsmOperand::SymFnInInline { expr } => { + self.check_expr(expr); + } // `AnonConst`s have their own body and is type-checked separately. // As they don't flow into the type system we don't need them to // be well-formed. - hir::InlineAsmOperand::Const { .. } | hir::InlineAsmOperand::SymFn { .. } => {} + hir::InlineAsmOperand::Const { .. } + | hir::InlineAsmOperand::SymFnInGlobal { .. } => {} hir::InlineAsmOperand::SymStatic { .. } => {} } } diff --git a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs index 840910732d89f..25fd724e27dfa 100644 --- a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs @@ -291,7 +291,10 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { } hir::InlineAsmOperand::Out { expr: None, .. } | hir::InlineAsmOperand::Const { .. } - | hir::InlineAsmOperand::SymFn { .. } + | hir::InlineAsmOperand::SymFnInGlobal { .. } + // `sym`, even in body, operates like a constant, + // so no need to consume or mutate anything here. + | hir::InlineAsmOperand::SymFnInInline { .. } | hir::InlineAsmOperand::SymStatic { .. } => {} } } diff --git a/compiler/rustc_hir_typeck/src/lib.rs b/compiler/rustc_hir_typeck/src/lib.rs index f17f1d14bf390..00fd8db27d732 100644 --- a/compiler/rustc_hir_typeck/src/lib.rs +++ b/compiler/rustc_hir_typeck/src/lib.rs @@ -223,7 +223,9 @@ fn typeck_with_fallback<'tcx>( // Inline assembly constants must be integers. Some(fcx.next_int_var()) } - hir::InlineAsmOperand::SymFn { anon_const } if anon_const.hir_id == id => { + hir::InlineAsmOperand::SymFnInGlobal { anon_const } + if anon_const.hir_id == id => + { Some(fcx.next_ty_var(TypeVariableOrigin { kind: TypeVariableOriginKind::MiscVariable, span, diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index 8c39614903cb0..a78f1babdb748 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -566,10 +566,13 @@ pub enum InlineAsmOperand<'tcx> { value: mir::Const<'tcx>, span: Span, }, - SymFn { + SymFnInGlobal { value: mir::Const<'tcx>, span: Span, }, + SymFnInInline { + expr: ExprId, + }, SymStatic { def_id: DefId, }, diff --git a/compiler/rustc_middle/src/thir/visit.rs b/compiler/rustc_middle/src/thir/visit.rs index b84e15688848b..308bccb475596 100644 --- a/compiler/rustc_middle/src/thir/visit.rs +++ b/compiler/rustc_middle/src/thir/visit.rs @@ -154,9 +154,12 @@ pub fn walk_expr<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, expr: &Exp visitor.visit_expr(&visitor.thir()[*out_expr]); } } + SymFnInInline { expr } => { + visitor.visit_expr(&visitor.thir()[*expr]); + } Out { expr: None, reg: _, late: _ } | Const { value: _, span: _ } - | SymFn { value: _, span: _ } + | SymFnInGlobal { value: _, span: _ } | SymStatic { def_id: _ } => {} } } diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index a4de42d45c995..2349b3f6aea10 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -440,7 +440,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }), } } - thir::InlineAsmOperand::SymFn { value, span } => { + thir::InlineAsmOperand::SymFnInGlobal { value, span } => { mir::InlineAsmOperand::SymFn { value: Box::new(ConstOperand { span, @@ -449,6 +449,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }), } } + thir::InlineAsmOperand::SymFnInInline { expr } => { + mir::InlineAsmOperand::SymFn { + value: Box::new(this.as_constant(&this.thir[expr])), + } + } thir::InlineAsmOperand::SymStatic { def_id } => { mir::InlineAsmOperand::SymStatic { def_id } } diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index 16a85d4276177..bd3cbd923502a 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -653,12 +653,15 @@ impl<'tcx> Cx<'tcx> { InlineAsmOperand::Const { value, span } } - hir::InlineAsmOperand::SymFn { ref anon_const } => { + hir::InlineAsmOperand::SymFnInGlobal { ref anon_const } => { let value = mir::Const::from_anon_const(tcx, anon_const.def_id, self.param_env); let span = tcx.def_span(anon_const.def_id); - InlineAsmOperand::SymFn { value, span } + InlineAsmOperand::SymFnInGlobal { value, span } + } + hir::InlineAsmOperand::SymFnInInline { expr } => { + InlineAsmOperand::SymFnInInline { expr: self.mirror_expr(expr) } } hir::InlineAsmOperand::SymStatic { path: _, def_id } => { InlineAsmOperand::SymStatic { def_id } diff --git a/compiler/rustc_mir_build/src/thir/print.rs b/compiler/rustc_mir_build/src/thir/print.rs index 3b6276cfeb0e6..6bce109ecc146 100644 --- a/compiler/rustc_mir_build/src/thir/print.rs +++ b/compiler/rustc_mir_build/src/thir/print.rs @@ -897,12 +897,18 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> { print_indented!(self, format!("span: {:?}", span), depth_lvl + 1); print_indented!(self, "}", depth_lvl + 1); } - InlineAsmOperand::SymFn { value, span } => { - print_indented!(self, "InlineAsmOperand::SymFn {", depth_lvl); + InlineAsmOperand::SymFnInGlobal { value, span } => { + print_indented!(self, "InlineAsmOperand::SymFnInGlobal {", depth_lvl); print_indented!(self, format!("value: {:?}", *value), depth_lvl + 1); print_indented!(self, format!("span: {:?}", span), depth_lvl + 1); print_indented!(self, "}", depth_lvl + 1); } + InlineAsmOperand::SymFnInInline { expr } => { + print_indented!(self, "InlineAsmOperand::SymFnInGlobal {", depth_lvl); + print_indented!(self, "expr: ", depth_lvl + 1); + self.print_expr(*expr, depth_lvl + 2); + print_indented!(self, "}", depth_lvl + 1); + } InlineAsmOperand::SymStatic { def_id } => { print_indented!(self, "InlineAsmOperand::SymStatic {", depth_lvl); print_indented!(self, format!("def_id: {:?}", def_id), depth_lvl + 1); diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 4afa4e597f7ce..391d060920acc 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -429,11 +429,16 @@ fn collect_items_rec<'tcx>( // are supported. Therefore the value should not // depend on any other items. } - hir::InlineAsmOperand::SymFn { anon_const } => { - let fn_ty = - tcx.typeck_body(anon_const.body).node_type(anon_const.hir_id); + hir::InlineAsmOperand::SymFnInGlobal { anon_const } => { + let fn_ty = tcx + .type_of(anon_const.def_id) + .no_bound_vars() + .expect("`sym` in `global_asm!` should not have generics"); visit_fn_use(tcx, fn_ty, false, *op_sp, &mut used_items, &[]); } + hir::InlineAsmOperand::SymFnInInline { .. } => { + bug!("should not encounter `SymFnInInline` in `global_asm!`"); + } hir::InlineAsmOperand::SymStatic { path: _, def_id } => { let instance = Instance::mono(tcx, *def_id); if should_codegen_locally(tcx, &instance) { diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index 20e996eaec4c9..41869263ab014 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -1086,7 +1086,8 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { match op { hir::InlineAsmOperand::In { .. } | hir::InlineAsmOperand::Const { .. } - | hir::InlineAsmOperand::SymFn { .. } + | hir::InlineAsmOperand::SymFnInGlobal { .. } + | hir::InlineAsmOperand::SymFnInInline { .. } | hir::InlineAsmOperand::SymStatic { .. } => {} hir::InlineAsmOperand::Out { expr, .. } => { if let Some(expr) = expr { @@ -1125,7 +1126,8 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { succ = self.propagate_through_expr(in_expr, succ); } hir::InlineAsmOperand::Const { .. } - | hir::InlineAsmOperand::SymFn { .. } + | hir::InlineAsmOperand::SymFnInGlobal { .. } + | hir::InlineAsmOperand::SymFnInInline { .. } | hir::InlineAsmOperand::SymStatic { .. } => {} } } diff --git a/compiler/rustc_passes/src/naked_functions.rs b/compiler/rustc_passes/src/naked_functions.rs index 7f36c59ad98d9..d93acf1c4b54c 100644 --- a/compiler/rustc_passes/src/naked_functions.rs +++ b/compiler/rustc_passes/src/naked_functions.rs @@ -232,8 +232,11 @@ impl<'tcx> CheckInlineAssembly<'tcx> { .iter() .filter_map(|&(ref op, op_sp)| match op { InlineAsmOperand::Const { .. } - | InlineAsmOperand::SymFn { .. } + | hir::InlineAsmOperand::SymFnInInline { .. } | InlineAsmOperand::SymStatic { .. } => None, + hir::InlineAsmOperand::SymFnInGlobal { .. } => { + bug!("SymFnInGlobal should never be in an inline `asm!`") + } InlineAsmOperand::In { .. } | InlineAsmOperand::Out { .. } | InlineAsmOperand::InOut { .. } diff --git a/tests/ui/asm/inline-asm-with-regions-in-sym.bad.stderr b/tests/ui/asm/inline-asm-with-regions-in-sym.bad.stderr new file mode 100644 index 0000000000000..913064d8d9bd7 --- /dev/null +++ b/tests/ui/asm/inline-asm-with-regions-in-sym.bad.stderr @@ -0,0 +1,14 @@ +error[E0309]: the parameter type `T` may not live long enough + --> $DIR/inline-asm-with-regions-in-sym.rs:16:26 + | +LL | asm!("/* {} */", sym dep::<'a, T> ); + | ^^^^^^^^^^^^^^^^ ...so that the type `T` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound... + | +LL | fn test<'a: 'a, T: 'a>() { + | ++++ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0309`. diff --git a/tests/ui/asm/inline-asm-with-regions-in-sym.rs b/tests/ui/asm/inline-asm-with-regions-in-sym.rs new file mode 100644 index 0000000000000..6f3d229dae08e --- /dev/null +++ b/tests/ui/asm/inline-asm-with-regions-in-sym.rs @@ -0,0 +1,21 @@ +// revisions: good bad +//[good] build-pass + +use std::arch::asm; + +// lifetime requirement, we should check it!! +#[cfg(bad)] +fn dep<'a, T: 'a>() {} + +// no lifetime requirement +#[cfg(good)] +fn dep<'a: 'a, T>() {} + +fn test<'a: 'a, T>() { + unsafe { + asm!("/* {} */", sym dep::<'a, T> ); + //[bad]~^ ERROR the parameter type `T` may not live long enough + } +} + +fn main() {} diff --git a/tests/ui/asm/x86_64/type-check-2.rs b/tests/ui/asm/x86_64/type-check-2.rs index 80b29ec870fc1..f209a52f20322 100644 --- a/tests/ui/asm/x86_64/type-check-2.rs +++ b/tests/ui/asm/x86_64/type-check-2.rs @@ -24,17 +24,6 @@ fn main() { asm!("{}", out(reg) v[0]); asm!("{}", inout(reg) v[0]); - // Sym operands must point to a function or static - - const C: i32 = 0; - static S: i32 = 0; - asm!("{}", sym S); - asm!("{}", sym main); - asm!("{}", sym C); - //~^ ERROR invalid `sym` operand - asm!("{}", sym x); - //~^ ERROR invalid `sym` operand - // Register operands must be Copy asm!("{}", in(xmm_reg) SimdNonCopy(0.0, 0.0, 0.0, 0.0)); @@ -77,6 +66,25 @@ fn main() { } } +fn bad_sym() { + unsafe { + // Sym operands must point to a function or static + let x: u64; + const C: i32 = 0; + static S: i32 = 0; + asm!("{}", sym S); + asm!("{}", sym main); + asm!("{}", sym C); + //~^ ERROR invalid `sym` operand + + // N.B. this error is emitted in the resolver, and will taint the + // whole typeck body causing other errors to be suppressed. That's + // why it's located in a different function. + asm!("{}", sym x); + //~^ ERROR invalid `sym` operand + } +} + // Sym operands must point to a function or static const C: i32 = 0; diff --git a/tests/ui/asm/x86_64/type-check-2.stderr b/tests/ui/asm/x86_64/type-check-2.stderr index 4f3d5100af056..65642573c6036 100644 --- a/tests/ui/asm/x86_64/type-check-2.stderr +++ b/tests/ui/asm/x86_64/type-check-2.stderr @@ -1,5 +1,5 @@ error: invalid `sym` operand - --> $DIR/type-check-2.rs:35:24 + --> $DIR/type-check-2.rs:83:24 | LL | asm!("{}", sym x); | ^ is a local variable @@ -7,31 +7,23 @@ LL | asm!("{}", sym x); = help: `sym` operands must refer to either a function or a static error: invalid `sym` operand - --> $DIR/type-check-2.rs:86:19 + --> $DIR/type-check-2.rs:94:19 | LL | global_asm!("{}", sym C); | ^^^^^ is an `i32` | = help: `sym` operands must refer to either a function or a static -error: invalid `sym` operand - --> $DIR/type-check-2.rs:33:20 - | -LL | asm!("{}", sym C); - | ^^^^^ is an `i32` - | - = help: `sym` operands must refer to either a function or a static - error: arguments for inline assembly must be copyable - --> $DIR/type-check-2.rs:40:32 + --> $DIR/type-check-2.rs:29:32 | LL | asm!("{}", in(xmm_reg) SimdNonCopy(0.0, 0.0, 0.0, 0.0)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `SimdNonCopy` does not implement the Copy trait -error: cannot use value of type `{closure@$DIR/type-check-2.rs:52:28: 52:36}` for inline assembly - --> $DIR/type-check-2.rs:52:28 +error: cannot use value of type `{closure@$DIR/type-check-2.rs:41:28: 41:36}` for inline assembly + --> $DIR/type-check-2.rs:41:28 | LL | asm!("{}", in(reg) |x: i32| x); | ^^^^^^^^^^ @@ -39,7 +31,7 @@ LL | asm!("{}", in(reg) |x: i32| x); = note: only integers, floats, SIMD vectors, pointers and function pointers can be used as arguments for inline assembly error: cannot use value of type `Vec` for inline assembly - --> $DIR/type-check-2.rs:54:28 + --> $DIR/type-check-2.rs:43:28 | LL | asm!("{}", in(reg) vec![0]); | ^^^^^^^ @@ -48,7 +40,7 @@ LL | asm!("{}", in(reg) vec![0]); = note: this error originates in the macro `vec` (in Nightly builds, run with -Z macro-backtrace for more info) error: cannot use value of type `(i32, i32, i32)` for inline assembly - --> $DIR/type-check-2.rs:56:28 + --> $DIR/type-check-2.rs:45:28 | LL | asm!("{}", in(reg) (1, 2, 3)); | ^^^^^^^^^ @@ -56,7 +48,7 @@ LL | asm!("{}", in(reg) (1, 2, 3)); = note: only integers, floats, SIMD vectors, pointers and function pointers can be used as arguments for inline assembly error: cannot use value of type `[i32; 3]` for inline assembly - --> $DIR/type-check-2.rs:58:28 + --> $DIR/type-check-2.rs:47:28 | LL | asm!("{}", in(reg) [1, 2, 3]); | ^^^^^^^^^ @@ -64,7 +56,7 @@ LL | asm!("{}", in(reg) [1, 2, 3]); = note: only integers, floats, SIMD vectors, pointers and function pointers can be used as arguments for inline assembly error: cannot use value of type `fn() {main}` for inline assembly - --> $DIR/type-check-2.rs:66:31 + --> $DIR/type-check-2.rs:55:31 | LL | asm!("{}", inout(reg) f); | ^ @@ -72,12 +64,20 @@ LL | asm!("{}", inout(reg) f); = note: only integers, floats, SIMD vectors, pointers and function pointers can be used as arguments for inline assembly error: cannot use value of type `&mut i32` for inline assembly - --> $DIR/type-check-2.rs:69:31 + --> $DIR/type-check-2.rs:58:31 | LL | asm!("{}", inout(reg) r); | ^ | = note: only integers, floats, SIMD vectors, pointers and function pointers can be used as arguments for inline assembly +error: invalid `sym` operand + --> $DIR/type-check-2.rs:77:20 + | +LL | asm!("{}", sym C); + | ^^^^^ is an `i32` + | + = help: `sym` operands must refer to either a function or a static + error: aborting due to 10 previous errors From 6d974154679bcc3c458ceaf256eb667aa7f71b5a Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 23 Sep 2023 04:28:24 +0000 Subject: [PATCH 3/3] Fix tools --- compiler/rustc_codegen_cranelift/src/global_asm.rs | 10 ++++++++-- src/tools/clippy/clippy_lints/src/loops/never_loop.rs | 7 ++++--- src/tools/clippy/clippy_utils/src/hir_utils.rs | 5 ++++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/global_asm.rs b/compiler/rustc_codegen_cranelift/src/global_asm.rs index baadd7a9e812b..fe0df813914db 100644 --- a/compiler/rustc_codegen_cranelift/src/global_asm.rs +++ b/compiler/rustc_codegen_cranelift/src/global_asm.rs @@ -39,8 +39,11 @@ pub(crate) fn codegen_global_asm_item(tcx: TyCtxt<'_>, global_asm: &mut String, ); global_asm.push_str(&string); } - InlineAsmOperand::SymFn { anon_const } => { - let ty = tcx.typeck_body(anon_const.body).node_type(anon_const.hir_id); + InlineAsmOperand::SymFnInGlobal { anon_const } => { + let ty = tcx + .type_of(anon_const.def_id) + .no_bound_vars() + .expect("`sym` in `global_asm!` should not have generics"); let instance = match ty.kind() { &ty::FnDef(def_id, args) => Instance::new(def_id, args), _ => span_bug!(op_sp, "asm sym is not a function"), @@ -50,6 +53,9 @@ pub(crate) fn codegen_global_asm_item(tcx: TyCtxt<'_>, global_asm: &mut String, // current codegen unit global_asm.push_str(symbol.name); } + InlineAsmOperand::SymFnInInline { .. } => { + bug!("should not encounter `SymFnInInline` in `global_asm!`"); + } InlineAsmOperand::SymStatic { path: _, def_id } => { let instance = Instance::mono(tcx, def_id).polymorphize(tcx); let symbol = tcx.symbol_name(instance); diff --git a/src/tools/clippy/clippy_lints/src/loops/never_loop.rs b/src/tools/clippy/clippy_lints/src/loops/never_loop.rs index 3d8a4cd948afc..282ef7b1730de 100644 --- a/src/tools/clippy/clippy_lints/src/loops/never_loop.rs +++ b/src/tools/clippy/clippy_lints/src/loops/never_loop.rs @@ -252,9 +252,10 @@ fn never_loop_expr<'tcx>( local_labels, main_loop_id, ), - InlineAsmOperand::Const { .. } | InlineAsmOperand::SymFn { .. } | InlineAsmOperand::SymStatic { .. } => { - NeverLoopResult::Normal - }, + InlineAsmOperand::Const { .. } + | InlineAsmOperand::SymFnInGlobal { .. } + | InlineAsmOperand::SymFnInInline { .. } + | InlineAsmOperand::SymStatic { .. } => NeverLoopResult::Normal, })), ExprKind::OffsetOf(_, _) | ExprKind::Yield(_, _) diff --git a/src/tools/clippy/clippy_utils/src/hir_utils.rs b/src/tools/clippy/clippy_utils/src/hir_utils.rs index 52214e733f1a9..3ba86507f24ae 100644 --- a/src/tools/clippy/clippy_utils/src/hir_utils.rs +++ b/src/tools/clippy/clippy_utils/src/hir_utils.rs @@ -781,9 +781,12 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_expr(out_expr); } }, - InlineAsmOperand::Const { anon_const } | InlineAsmOperand::SymFn { anon_const } => { + InlineAsmOperand::Const { anon_const } | InlineAsmOperand::SymFnInGlobal { anon_const } => { self.hash_body(anon_const.body); }, + InlineAsmOperand::SymFnInInline { expr } => { + self.hash_expr(expr); + } InlineAsmOperand::SymStatic { path, def_id: _ } => self.hash_qpath(path), } }