various fixes for naked_asm! implementation

folkertdev · folkertdev · commit 5fc60d1e52ea · 2024-10-06T19:00:09.000+02:00
- fix for divergence
- fix error message
- fix another cranelift test
- fix some cranelift things
- don't set the NORETURN option for naked asm
- fix use of naked_asm! in doc comment
- fix use of naked_asm! in run-make test
- use `span_bug` in unreachable branch
diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs
@@ -2435,21 +2435,29 @@ pub enum AsmMacro {
 }
 
 impl AsmMacro {
-    pub const fn macro_name(&self) -> &'static str {
+    pub const fn macro_name(self) -> &'static str {
         match self {
             AsmMacro::Asm => "asm",
             AsmMacro::GlobalAsm => "global_asm",
             AsmMacro::NakedAsm => "naked_asm",
         }
     }
 
-    pub const fn is_supported_option(&self, option: InlineAsmOptions) -> bool {
+    pub const fn is_supported_option(self, option: InlineAsmOptions) -> bool {
         match self {
             AsmMacro::Asm => true,
             AsmMacro::GlobalAsm => InlineAsmOptions::GLOBAL_OPTIONS.contains(option),
             AsmMacro::NakedAsm => InlineAsmOptions::NAKED_OPTIONS.contains(option),
         }
     }
+
+    pub const fn diverges(self, options: InlineAsmOptions) -> bool {
+        match self {
+            AsmMacro::Asm => options.contains(InlineAsmOptions::NORETURN),
+            AsmMacro::GlobalAsm => true,
+            AsmMacro::NakedAsm => true,
+        }
+    }
 }
 
 /// Inline assembly.
diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs
@@ -742,6 +742,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R>
             }
 
             TerminatorKind::InlineAsm {
+                asm_macro: _,
                 template: _,
                 operands,
                 options: _,
diff --git a/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs b/compiler/rustc_borrowck/src/polonius/loan_invalidations.rs
@@ -169,6 +169,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LoanInvalidationsGenerator<'a, 'tcx> {
                 }
             }
             TerminatorKind::InlineAsm {
+                asm_macro: _,
                 template: _,
                 operands,
                 options: _,
diff --git a/compiler/rustc_builtin_macros/src/asm.rs b/compiler/rustc_builtin_macros/src/asm.rs
@@ -850,22 +850,13 @@ pub(super) fn expand_naked_asm<'cx>(
                 return ExpandResult::Retry(());
             };
             let expr = match mac {
-                Ok(mut inline_asm) => {
-                    // for future compatibility, we always set the NORETURN option.
-                    //
-                    // When we turn `asm!` into `naked_asm!` with this implementation, we can drop
-                    // the `options(noreturn)`, which makes the upgrade smooth when `naked_asm!`
-                    // starts disallowing the `noreturn` option in the future
-                    inline_asm.options |= ast::InlineAsmOptions::NORETURN;
-
-                    P(ast::Expr {
-                        id: ast::DUMMY_NODE_ID,
-                        kind: ast::ExprKind::InlineAsm(P(inline_asm)),
-                        span: sp,
-                        attrs: ast::AttrVec::new(),
-                        tokens: None,
-                    })
-                }
+                Ok(inline_asm) => P(ast::Expr {
+                    id: ast::DUMMY_NODE_ID,
+                    kind: ast::ExprKind::InlineAsm(P(inline_asm)),
+                    span: sp,
+                    attrs: ast::AttrVec::new(),
+                    tokens: None,
+                }),
                 Err(guar) => DummyResult::raw_expr(sp, Some(guar)),
             };
             MacEager::expr(expr)
diff --git a/compiler/rustc_codegen_cranelift/example/mini_core.rs b/compiler/rustc_codegen_cranelift/example/mini_core.rs
@@ -726,6 +726,12 @@ pub macro global_asm() {
     /* compiler built-in */
 }
 
+#[rustc_builtin_macro]
+#[rustc_macro_transparency = "semitransparent"]
+pub macro naked_asm() {
+    /* compiler built-in */
+}
+
 pub static A_STATIC: u8 = 42;
 
 #[lang = "panic_location"]
diff --git a/compiler/rustc_codegen_cranelift/example/mini_core_hello_world.rs b/compiler/rustc_codegen_cranelift/example/mini_core_hello_world.rs
@@ -390,7 +390,7 @@ global_asm! {
 #[naked]
 extern "C" fn naked_test() {
     unsafe {
-        asm!("ret", options(noreturn));
+        naked_asm!("ret");
     }
 }
 
diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs
@@ -8,6 +8,7 @@ use rustc_ast::InlineAsmOptions;
 use rustc_codegen_ssa::base::is_call_from_compiler_builtins_to_upstream_monomorphization;
 use rustc_index::IndexVec;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
+use rustc_middle::mir::InlineAsmMacro;
 use rustc_middle::ty::TypeVisitableExt;
 use rustc_middle::ty::adjustment::PointerCoercion;
 use rustc_middle::ty::layout::FnAbiOf;
@@ -57,6 +58,7 @@ pub(crate) fn codegen_fn<'tcx>(
 
         match &mir.basic_blocks[START_BLOCK].terminator().kind {
             TerminatorKind::InlineAsm {
+                asm_macro: InlineAsmMacro::NakedAsm,
                 template,
                 operands,
                 options,
@@ -498,6 +500,7 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
                 "tail calls are not yet supported in `rustc_codegen_cranelift` backend"
             ),
             TerminatorKind::InlineAsm {
+                asm_macro: _,
                 template,
                 operands,
                 options,
diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs
@@ -3,7 +3,9 @@ use std::cmp;
 use rustc_ast as ast;
 use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
 use rustc_hir::lang_items::LangItem;
-use rustc_middle::mir::{self, AssertKind, BasicBlock, SwitchTargets, UnwindTerminateReason};
+use rustc_middle::mir::{
+    self, AssertKind, BasicBlock, InlineAsmMacro, SwitchTargets, UnwindTerminateReason,
+};
 use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, ValidityRequirement};
 use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths};
 use rustc_middle::ty::{self, Instance, Ty};
@@ -1133,6 +1135,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         &mut self,
         helper: TerminatorCodegenHelper<'tcx>,
         bx: &mut Bx,
+        asm_macro: InlineAsmMacro,
         terminator: &mir::Terminator<'tcx>,
         template: &[ast::InlineAsmTemplatePiece],
         operands: &[mir::InlineAsmOperand<'tcx>],
@@ -1203,11 +1206,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             &operands,
             options,
             line_spans,
-            if options.contains(InlineAsmOptions::NORETURN) {
-                None
-            } else {
-                targets.get(0).copied()
-            },
+            if asm_macro.diverges(options) { None } else { targets.get(0).copied() },
             unwind,
             instance,
             mergeable_succ,
@@ -1381,6 +1380,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             }
 
             mir::TerminatorKind::InlineAsm {
+                asm_macro,
                 template,
                 ref operands,
                 options,
@@ -1390,6 +1390,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             } => self.codegen_asm_terminator(
                 helper,
                 bx,
+                asm_macro,
                 terminator,
                 template,
                 operands,
diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs
@@ -395,7 +395,7 @@ pub trait Machine<'tcx>: Sized {
     ///
     /// This should take care of jumping to the next block (one of `targets`) when asm goto
     /// is triggered, `targets[0]` when the assembly falls through, or diverge in case of
-    /// `InlineAsmOptions::NORETURN` being set.
+    /// naked_asm! or `InlineAsmOptions::NORETURN` being set.
     fn eval_inline_asm(
         _ecx: &mut InterpCx<'tcx, Self>,
         _template: &'tcx [InlineAsmTemplatePiece],
diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs
@@ -3507,11 +3507,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     }
 
     fn check_expr_asm(&self, asm: &'tcx hir::InlineAsm<'tcx>) -> Ty<'tcx> {
-        let mut diverge = match asm.asm_macro {
-            rustc_ast::AsmMacro::Asm => asm.options.contains(ast::InlineAsmOptions::NORETURN),
-            rustc_ast::AsmMacro::GlobalAsm => true,
-            rustc_ast::AsmMacro::NakedAsm => true,
-        };
+        let mut diverge = asm.asm_macro.diverges(asm.options);
 
         for (op, _op_sp) in asm.operands {
             match op {
diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs
@@ -2926,16 +2926,16 @@ declare_lint! {
     /// ```rust
     /// #![feature(asm_experimental_arch, naked_functions)]
     ///
-    /// use std::arch::asm;
+    /// use std::arch::naked_asm;
     ///
     /// #[naked]
     /// pub fn default_abi() -> u32 {
-    ///     unsafe { asm!("", options(noreturn)); }
+    ///     unsafe { naked_asm!(""); }
     /// }
     ///
     /// #[naked]
     /// pub extern "Rust" fn rust_abi() -> u32 {
-    ///     unsafe { asm!("", options(noreturn)); }
+    ///     unsafe { naked_asm!(""); }
     /// }
     /// ```
     ///
diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs
@@ -4,7 +4,7 @@ use std::fs;
 use std::io::{self, Write as _};
 use std::path::{Path, PathBuf};
 
-use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
+use rustc_ast::InlineAsmTemplatePiece;
 use rustc_middle::mir::interpret::{
     AllocBytes, AllocId, Allocation, GlobalAlloc, Pointer, Provenance, alloc_range,
     read_target_uint,
@@ -1024,9 +1024,9 @@ impl<'tcx> TerminatorKind<'tcx> {
                 vec!["real".into(), "unwind".into()]
             }
             FalseUnwind { unwind: _, .. } => vec!["real".into()],
-            InlineAsm { options, ref targets, unwind, .. } => {
+            InlineAsm { asm_macro, options, ref targets, unwind, .. } => {
                 let mut vec = Vec::with_capacity(targets.len() + 1);
-                if !options.contains(InlineAsmOptions::NORETURN) {
+                if !asm_macro.diverges(options) {
                     vec.push("return".into());
                 }
                 vec.resize(targets.len(), "label".into());
diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs
@@ -605,6 +605,25 @@ impl CallSource {
     }
 }
 
+#[derive(Clone, Copy, Debug, TyEncodable, TyDecodable, Hash, HashStable, PartialEq)]
+#[derive(TypeFoldable, TypeVisitable)]
+/// The macro that an inline assembly block was created by
+pub enum InlineAsmMacro {
+    /// The `asm!` macro
+    Asm,
+    /// The `naked_asm!` macro
+    NakedAsm,
+}
+
+impl InlineAsmMacro {
+    pub const fn diverges(self, options: InlineAsmOptions) -> bool {
+        match self {
+            InlineAsmMacro::Asm => options.contains(InlineAsmOptions::NORETURN),
+            InlineAsmMacro::NakedAsm => true,
+        }
+    }
+}
+
 ///////////////////////////////////////////////////////////////////////////
 // Terminators
 
@@ -859,6 +878,9 @@ pub enum TerminatorKind<'tcx> {
     /// Block ends with an inline assembly block. This is a terminator since
     /// inline assembly is allowed to diverge.
     InlineAsm {
+        /// Macro used to create this inline asm: one of `asm!` or `naked_asm!`
+        asm_macro: InlineAsmMacro,
+
         /// The template for the inline assembly, with placeholders.
         template: &'tcx [InlineAsmTemplatePiece],
 
@@ -874,7 +896,7 @@ pub enum TerminatorKind<'tcx> {
 
         /// Valid targets for the inline assembly.
         /// The first element is the fallthrough destination, unless
-        /// InlineAsmOptions::NORETURN is set.
+        /// asm_macro == InlineAsmMacro::NakedAsm or InlineAsmOptions::NORETURN is set.
         targets: Box<[BasicBlock]>,
 
         /// Action to be taken if the inline assembly unwinds. This is present
diff --git a/compiler/rustc_middle/src/mir/terminator.rs b/compiler/rustc_middle/src/mir/terminator.rs
@@ -666,6 +666,7 @@ impl<'tcx> TerminatorKind<'tcx> {
             },
 
             InlineAsm {
+                asm_macro: _,
                 template: _,
                 ref operands,
                 options: _,
diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs
@@ -576,6 +576,7 @@ macro_rules! make_mir_visitor {
                     }
 
                     TerminatorKind::InlineAsm {
+                        asm_macro: _,
                         template: _,
                         operands,
                         options: _,
diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs
@@ -12,7 +12,7 @@ use std::cmp::Ordering;
 use std::fmt;
 use std::ops::Index;
 
-use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
+use rustc_ast::{AsmMacro, InlineAsmOptions, InlineAsmTemplatePiece};
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_hir::{BindingMode, ByRef, HirId, MatchSource, RangeEnd};
@@ -173,6 +173,7 @@ pub struct ClosureExpr<'tcx> {
 
 #[derive(Clone, Debug, HashStable)]
 pub struct InlineAsmExpr<'tcx> {
+    pub asm_macro: AsmMacro,
     pub template: &'tcx [InlineAsmTemplatePiece],
     pub operands: Box<[InlineAsmOperand<'tcx>]>,
     pub options: InlineAsmOptions,
diff --git a/compiler/rustc_middle/src/thir/visit.rs b/compiler/rustc_middle/src/thir/visit.rs
@@ -148,7 +148,13 @@ pub fn walk_expr<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>(
         NamedConst { def_id: _, args: _, user_ty: _ } => {}
         ConstParam { param: _, def_id: _ } => {}
         StaticRef { alloc_id: _, ty: _, def_id: _ } => {}
-        InlineAsm(box InlineAsmExpr { ref operands, template: _, options: _, line_spans: _ }) => {
+        InlineAsm(box InlineAsmExpr {
+            asm_macro: _,
+            ref operands,
+            template: _,
+            options: _,
+            line_spans: _,
+        }) => {
             for op in &**operands {
                 use InlineAsmOperand::*;
                 match op {
diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs
@@ -2,7 +2,7 @@
 
 use std::iter;
 
-use rustc_ast::InlineAsmOptions;
+use rustc_ast::{AsmMacro, InlineAsmOptions};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_hir as hir;
@@ -384,6 +384,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 block.unit()
             }
             ExprKind::InlineAsm(box InlineAsmExpr {
+                asm_macro,
                 template,
                 ref operands,
                 options,
@@ -392,11 +393,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 use rustc_middle::{mir, thir};
 
                 let destination_block = this.cfg.start_new_block();
-                let mut targets = if options.contains(InlineAsmOptions::NORETURN) {
-                    vec![]
-                } else {
-                    vec![destination_block]
-                };
+                let mut targets =
+                    if asm_macro.diverges(options) { vec![] } else { vec![destination_block] };
 
                 let operands = operands
                     .into_iter()
@@ -474,7 +472,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     this.cfg.push_assign_unit(block, source_info, destination, this.tcx);
                 }
 
+                let asm_macro = match asm_macro {
+                    AsmMacro::Asm => InlineAsmMacro::Asm,
+                    AsmMacro::GlobalAsm => {
+                        span_bug!(expr_span, "unexpected global_asm! in inline asm")
+                    }
+                    AsmMacro::NakedAsm => InlineAsmMacro::NakedAsm,
+                };
+
                 this.cfg.terminate(block, source_info, TerminatorKind::InlineAsm {
+                    asm_macro,
                     template,
                     operands,
                     options,
diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs
@@ -672,6 +672,7 @@ impl<'tcx> Cx<'tcx> {
             }
 
             hir::ExprKind::InlineAsm(asm) => ExprKind::InlineAsm(Box::new(InlineAsmExpr {
+                asm_macro: asm.asm_macro,
                 template: asm.template,
                 operands: asm
                     .operands
diff --git a/compiler/rustc_mir_build/src/thir/print.rs b/compiler/rustc_mir_build/src/thir/print.rs
@@ -818,10 +818,12 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> {
     }
 
     fn print_inline_asm_expr(&mut self, expr: &InlineAsmExpr<'tcx>, depth_lvl: usize) {
-        let InlineAsmExpr { template, operands, options, line_spans } = expr;
+        let InlineAsmExpr { asm_macro, template, operands, options, line_spans } = expr;
 
         print_indented!(self, "InlineAsmExpr {", depth_lvl);
 
+        print_indented!(self, format!("asm_macro: {:?}", asm_macro), depth_lvl + 1);
+
         print_indented!(self, "template: [", depth_lvl + 1);
         for template_piece in template.iter() {
             print_indented!(self, format!("{:?}", template_piece), depth_lvl + 2);
diff --git a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs
diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl
diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs
diff --git a/compiler/rustc_passes/src/naked_functions.rs b/compiler/rustc_passes/src/naked_functions.rs
diff --git a/compiler/rustc_smir/src/rustc_smir/convert/mir.rs b/compiler/rustc_smir/src/rustc_smir/convert/mir.rs
diff --git a/src/tools/rustfmt/src/parse/macros/asm.rs b/src/tools/rustfmt/src/parse/macros/asm.rs
diff --git a/tests/codegen/naked-asan.rs b/tests/codegen/naked-asan.rs
diff --git a/tests/run-make/naked-symbol-visibility/a_rust_dylib.rs b/tests/run-make/naked-symbol-visibility/a_rust_dylib.rs
diff --git a/tests/ui/asm/naked-functions.stderr b/tests/ui/asm/naked-functions.stderr

Original file line number	Diff line number	Diff line change
`@@ -2435,21 +2435,29 @@ pub enum AsmMacro {`
`2435`	`2435`	`}`
`2436`	`2436`
`2437`	`2437`	`impl AsmMacro {`
`2438`		`- pub const fn macro_name(&self) -> &'static str {`
	`2438`	`+ pub const fn macro_name(self) -> &'static str {`
`2439`	`2439`	`match self {`
`2440`	`2440`	`AsmMacro::Asm => "asm",`
`2441`	`2441`	`AsmMacro::GlobalAsm => "global_asm",`
`2442`	`2442`	`AsmMacro::NakedAsm => "naked_asm",`
`2443`	`2443`	`}`
`2444`	`2444`	`}`
`2445`	`2445`
`2446`		`- pub const fn is_supported_option(&self, option: InlineAsmOptions) -> bool {`
	`2446`	`+ pub const fn is_supported_option(self, option: InlineAsmOptions) -> bool {`
`2447`	`2447`	`match self {`
`2448`	`2448`	`AsmMacro::Asm => true,`
`2449`	`2449`	`AsmMacro::GlobalAsm => InlineAsmOptions::GLOBAL_OPTIONS.contains(option),`
`2450`	`2450`	`AsmMacro::NakedAsm => InlineAsmOptions::NAKED_OPTIONS.contains(option),`
`2451`	`2451`	`}`
`2452`	`2452`	`}`
	`2453`	`+`
	`2454`	`+ pub const fn diverges(self, options: InlineAsmOptions) -> bool {`
	`2455`	`+ match self {`
	`2456`	`+ AsmMacro::Asm => options.contains(InlineAsmOptions::NORETURN),`
	`2457`	`+ AsmMacro::GlobalAsm => true,`
	`2458`	`+ AsmMacro::NakedAsm => true,`
	`2459`	`+ }`
	`2460`	`+ }`
`2453`	`2461`	`}`
`2454`	`2462`
`2455`	`2463`	`/// Inline assembly.`
Original file line number	Diff line number	Diff line change
`@@ -742,6 +742,7 @@ impl<'a, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'a, 'tcx, R>`
`742`	`742`	`}`
`743`	`743`
`744`	`744`	`TerminatorKind::InlineAsm {`
	`745`	`+ asm_macro: _,`
`745`	`746`	`template: _,`
`746`	`747`	`operands,`
`747`	`748`	`options: _,`
Original file line number	Diff line number	Diff line change
`@@ -169,6 +169,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LoanInvalidationsGenerator<'a, 'tcx> {`
`169`	`169`	`}`
`170`	`170`	`}`
`171`	`171`	`TerminatorKind::InlineAsm {`
	`172`	`+ asm_macro: _,`
`172`	`173`	`template: _,`
`173`	`174`	`operands,`
`174`	`175`	`options: _,`
Original file line number	Diff line number	Diff line change
`@@ -390,7 +390,7 @@ global_asm! {`
`390`	`390`	`#[naked]`
`391`	`391`	`extern "C" fn naked_test() {`
`392`	`392`	`unsafe {`
`393`		`- asm!("ret", options(noreturn));`
	`393`	`+ naked_asm!("ret");`
`394`	`394`	`}`
`395`	`395`	`}`
`396`	`396`