Rollup merge of rust-lang#83437 - Amanieu:asm_syntax, r=petrochenkov

Dylan-DPC · web-flow · commit 02b27cd79e8d · 2021-03-26T02:34:39.000+01:00
Refactor rust-lang#82270 as lint instead of an error This PR fixes several issues with rust-lang#82270 which generated an error when `.intel_syntax` or `.att_syntax` was used in inline assembly: - It is now a warn-by-default lint instead of an error. - The lint only triggers on x86. `.intel_syntax` and `.att_syntax` are only valid on x86. - The lint no longer provides machine-applicable suggestions for two reasons: - These changes should not be made automatically since changes to assembly code can be very subtle. - The template string is not always just a string: it can contain macro invocation (`concat!`), raw strings, escape characters, etc. cc ``@asquared31415``
diff --git a/compiler/rustc_builtin_macros/src/asm.rs b/compiler/rustc_builtin_macros/src/asm.rs
@@ -7,11 +7,10 @@ use rustc_errors::{Applicability, DiagnosticBuilder};
 use rustc_expand::base::{self, *};
 use rustc_parse::parser::Parser;
 use rustc_parse_format as parse;
-use rustc_span::{
-    symbol::{kw, sym, Symbol},
-    BytePos,
-};
+use rustc_session::lint;
+use rustc_span::symbol::{kw, sym, Symbol};
 use rustc_span::{InnerSpan, Span};
+use rustc_target::asm::InlineAsmArch;
 
 struct AsmArgs {
     templates: Vec<P<ast::Expr>>,
@@ -402,8 +401,6 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, sp: Span, args: AsmArgs) -> P<ast
     let mut line_spans = Vec::with_capacity(args.templates.len());
     let mut curarg = 0;
 
-    let default_dialect = ecx.sess.inline_asm_dialect();
-
     for template_expr in args.templates.into_iter() {
         if !template.is_empty() {
             template.push(ast::InlineAsmTemplatePiece::String("\n".to_string()));
@@ -430,56 +427,36 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, sp: Span, args: AsmArgs) -> P<ast
         let template_str = &template_str.as_str();
         let template_snippet = ecx.source_map().span_to_snippet(template_sp).ok();
 
-        if let Some(snippet) = &template_snippet {
-            let snippet = snippet.trim_matches('"');
-            match default_dialect {
-                ast::LlvmAsmDialect::Intel => {
-                    if let Some(span) = check_syntax_directive(snippet, ".intel_syntax") {
-                        let span = template_span.from_inner(span);
-                        let mut err = ecx.struct_span_err(span, "intel syntax is the default syntax on this target, and trying to use this directive may cause issues");
-                        err.span_suggestion(
-                            span,
-                            "remove this assembler directive",
-                            "".to_string(),
-                            Applicability::MachineApplicable,
-                        );
-                        err.emit();
-                    }
-
-                    if let Some(span) = check_syntax_directive(snippet, ".att_syntax") {
-                        let span = template_span.from_inner(span);
-                        let mut err = ecx.struct_span_err(span, "using the .att_syntax directive may cause issues, use the att_syntax option instead");
-                        let asm_end = sp.hi() - BytePos(2);
-                        let suggestions = vec![
-                            (span, "".to_string()),
-                            (
-                                Span::new(asm_end, asm_end, sp.ctxt()),
-                                ", options(att_syntax)".to_string(),
-                            ),
-                        ];
-                        err.multipart_suggestion(
-                        "remove the assembler directive and replace it with options(att_syntax)",
-                        suggestions,
-                        Applicability::MachineApplicable,
-                    );
-                        err.emit();
+        if let Some(InlineAsmArch::X86 | InlineAsmArch::X86_64) = ecx.sess.asm_arch {
+            let find_span = |needle: &str| -> Span {
+                if let Some(snippet) = &template_snippet {
+                    if let Some(pos) = snippet.find(needle) {
+                        let end = pos
+                            + &snippet[pos..]
+                                .find(|c| matches!(c, '\n' | ';' | '\\' | '"'))
+                                .unwrap_or(snippet[pos..].len() - 1);
+                        let inner = InnerSpan::new(pos, end);
+                        return template_sp.from_inner(inner);
                     }
                 }
-                ast::LlvmAsmDialect::Att => {
-                    if let Some(span) = check_syntax_directive(snippet, ".att_syntax") {
-                        let span = template_span.from_inner(span);
-                        let mut err = ecx.struct_span_err(span, "att syntax is the default syntax on this target, and trying to use this directive may cause issues");
-                        err.span_suggestion(
-                            span,
-                            "remove this assembler directive",
-                            "".to_string(),
-                            Applicability::MachineApplicable,
-                        );
-                        err.emit();
-                    }
+                template_sp
+            };
 
-                    // Use of .intel_syntax is ignored
-                }
+            if template_str.contains(".intel_syntax") {
+                ecx.parse_sess().buffer_lint(
+                    lint::builtin::BAD_ASM_STYLE,
+                    find_span(".intel_syntax"),
+                    ecx.resolver.lint_node_id(ecx.current_expansion.id),
+                    "avoid using `.intel_syntax`, Intel syntax is the default",
+                );
+            }
+            if template_str.contains(".att_syntax") {
+                ecx.parse_sess().buffer_lint(
+                    lint::builtin::BAD_ASM_STYLE,
+                    find_span(".att_syntax"),
+                    ecx.resolver.lint_node_id(ecx.current_expansion.id),
+                    "avoid using `.att_syntax`, prefer using `options(att_syntax)` instead",
+                );
             }
         }
 
@@ -690,15 +667,3 @@ pub fn expand_asm<'cx>(
         }
     }
 }
-
-fn check_syntax_directive<S: AsRef<str>>(piece: S, syntax: &str) -> Option<InnerSpan> {
-    let piece = piece.as_ref();
-    if let Some(idx) = piece.find(syntax) {
-        let end =
-            idx + &piece[idx..].find(|c| matches!(c, '\n' | ';')).unwrap_or(piece[idx..].len());
-        // Offset by one because these represent the span with the " removed
-        Some(InnerSpan::new(idx + 1, end + 1))
-    } else {
-        None
-    }
-}
diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs
@@ -2486,6 +2486,52 @@ declare_lint! {
     "using only a subset of a register for inline asm inputs",
 }
 
+declare_lint! {
+    /// The `bad_asm_style` lint detects the use of the `.intel_syntax` and
+    /// `.att_syntax` directives.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,ignore (fails on system llvm)
+    /// #![feature(asm)]
+    ///
+    /// fn main() {
+    ///     #[cfg(target_arch="x86_64")]
+    ///     unsafe {
+    ///         asm!(
+    ///             ".att_syntax",
+    ///             "movl {0}, {0}", in(reg) 0usize
+    ///         );
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// This will produce:
+    ///
+    /// ```text
+    ///  warning: avoid using `.att_syntax`, prefer using `options(att_syntax)` instead
+    ///  --> test.rs:7:14
+    ///   |
+    /// 7 |             ".att_syntax",
+    ///   |              ^^^^^^^^^^^
+    /// 8 |             "movq {0}, {0}", out(reg) _,
+    /// 9 |         );
+    ///   |         - help: add option: `, options(att_syntax)`
+    ///   |
+    ///   = note: `#[warn(bad_asm_style)]` on by default
+    /// ```
+    ///
+    /// ### Explanation
+    ///
+    /// On x86, `asm!` uses the intel assembly syntax by default. While this
+    /// can be switched using assembler directives like `.att_syntax`, using the
+    /// `att_syntax` option is recomended instead because it will also properly
+    /// prefix register placeholders with `%` as required by AT&T syntax.
+    pub BAD_ASM_STYLE,
+    Warn,
+    "incorrect use of inline assembly",
+}
+
 declare_lint! {
     /// The `unsafe_op_in_unsafe_fn` lint detects unsafe operations in unsafe
     /// functions without an explicit unsafe block.
diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs
@@ -793,13 +793,6 @@ impl Session {
         }
     }
 
-    pub fn inline_asm_dialect(&self) -> rustc_ast::LlvmAsmDialect {
-        match self.asm_arch {
-            Some(InlineAsmArch::X86 | InlineAsmArch::X86_64) => rustc_ast::LlvmAsmDialect::Intel,
-            _ => rustc_ast::LlvmAsmDialect::Att,
-        }
-    }
-
     pub fn relocation_model(&self) -> RelocModel {
         self.opts.cg.relocation_model.unwrap_or(self.target.relocation_model)
     }
diff --git a/src/test/ui/asm/inline-syntax.arm.stderr b/src/test/ui/asm/inline-syntax.arm.stderr
@@ -1,14 +1,74 @@
-error: att syntax is the default syntax on this target, and trying to use this directive may cause issues
-  --> $DIR/inline-syntax.rs:23:15
+error: unknown directive
+  --> $DIR/inline-syntax.rs:22:15
+   |
+LL |         asm!(".intel_syntax noprefix", "nop");
+   |               ^
+   |
+note: instantiated into assembly here
+  --> <inline asm>:1:2
+   |
+LL |     .intel_syntax noprefix
+   |     ^
+
+error: unknown directive
+  --> $DIR/inline-syntax.rs:25:15
+   |
+LL |         asm!(".intel_syntax aaa noprefix", "nop");
+   |               ^
+   |
+note: instantiated into assembly here
+  --> <inline asm>:1:2
+   |
+LL |     .intel_syntax aaa noprefix
+   |     ^
+
+error: unknown directive
+  --> $DIR/inline-syntax.rs:28:15
    |
 LL |         asm!(".att_syntax noprefix", "nop");
-   |               ^^^^^^^^^^^^^^^^^^^^ help: remove this assembler directive
+   |               ^
+   |
+note: instantiated into assembly here
+  --> <inline asm>:1:2
+   |
+LL |     .att_syntax noprefix
+   |     ^
 
-error: att syntax is the default syntax on this target, and trying to use this directive may cause issues
-  --> $DIR/inline-syntax.rs:26:15
+error: unknown directive
+  --> $DIR/inline-syntax.rs:31:15
    |
 LL |         asm!(".att_syntax bbb noprefix", "nop");
-   |               ^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this assembler directive
+   |               ^
+   |
+note: instantiated into assembly here
+  --> <inline asm>:1:2
+   |
+LL |     .att_syntax bbb noprefix
+   |     ^
+
+error: unknown directive
+  --> $DIR/inline-syntax.rs:34:15
+   |
+LL |         asm!(".intel_syntax noprefix; nop");
+   |               ^
+   |
+note: instantiated into assembly here
+  --> <inline asm>:1:2
+   |
+LL |     .intel_syntax noprefix; nop
+   |     ^
+
+error: unknown directive
+  --> $DIR/inline-syntax.rs:40:13
+   |
+LL |             .intel_syntax noprefix
+   |             ^
+   |
+note: instantiated into assembly here
+  --> <inline asm>:2:13
+   |
+LL |             .intel_syntax noprefix
+   |             ^
 
-error: aborting due to 2 previous errors
+error: aborting due to 6 previous errors
 
diff --git a/src/test/ui/asm/inline-syntax.rs b/src/test/ui/asm/inline-syntax.rs
@@ -1,9 +1,12 @@
 // needs-llvm-components: arm
 // revisions: x86_64 arm
 //[x86_64] compile-flags: --target x86_64-unknown-linux-gnu
+//[x86_64] check-pass
 //[arm] compile-flags: --target armv7-unknown-linux-gnueabihf
+//[arm] build-fail
 
 #![feature(no_core, lang_items, rustc_attrs)]
+#![crate_type = "rlib"]
 #![no_core]
 
 #[rustc_builtin_macro]
@@ -14,26 +17,30 @@ macro_rules! asm {
 #[lang = "sized"]
 trait Sized {}
 
-fn main() {
+pub fn main() {
     unsafe {
         asm!(".intel_syntax noprefix", "nop");
-        //[x86_64]~^ ERROR intel syntax is the default syntax on this target
+        //[x86_64]~^ WARN avoid using `.intel_syntax`
+        //[arm]~^^ ERROR unknown directive
         asm!(".intel_syntax aaa noprefix", "nop");
-        //[x86_64]~^ ERROR intel syntax is the default syntax on this target
+        //[x86_64]~^ WARN avoid using `.intel_syntax`
+        //[arm]~^^ ERROR unknown directive
         asm!(".att_syntax noprefix", "nop");
-        //[x86_64]~^ ERROR using the .att_syntax directive may cause issues
-        //[arm]~^^ att syntax is the default syntax on this target
+        //[x86_64]~^ WARN avoid using `.att_syntax`
+        //[arm]~^^ ERROR unknown directive
         asm!(".att_syntax bbb noprefix", "nop");
-        //[x86_64]~^ ERROR using the .att_syntax directive may cause issues
-        //[arm]~^^ att syntax is the default syntax on this target
+        //[x86_64]~^ WARN avoid using `.att_syntax`
+        //[arm]~^^ ERROR unknown directive
         asm!(".intel_syntax noprefix; nop");
-        //[x86_64]~^ ERROR intel syntax is the default syntax on this target
+        //[x86_64]~^ WARN avoid using `.intel_syntax`
+        //[arm]~^^ ERROR unknown directive
 
         asm!(
             r"
             .intel_syntax noprefix
             nop"
         );
-        //[x86_64]~^^^ ERROR intel syntax is the default syntax on this target
+        //[x86_64]~^^^ WARN avoid using `.intel_syntax`
+        //[arm]~^^^^ ERROR unknown directive
     }
 }
diff --git a/src/test/ui/asm/inline-syntax.x86_64.stderr b/src/test/ui/asm/inline-syntax.x86_64.stderr
@@ -1,50 +1,40 @@
-error: intel syntax is the default syntax on this target, and trying to use this directive may cause issues
-  --> $DIR/inline-syntax.rs:19:15
+warning: avoid using `.intel_syntax`, Intel syntax is the default
+  --> $DIR/inline-syntax.rs:22:15
    |
 LL |         asm!(".intel_syntax noprefix", "nop");
-   |               ^^^^^^^^^^^^^^^^^^^^^^ help: remove this assembler directive
+   |               ^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `#[warn(bad_asm_style)]` on by default
 
-error: intel syntax is the default syntax on this target, and trying to use this directive may cause issues
-  --> $DIR/inline-syntax.rs:21:15
+warning: avoid using `.intel_syntax`, Intel syntax is the default
+  --> $DIR/inline-syntax.rs:25:15
    |
 LL |         asm!(".intel_syntax aaa noprefix", "nop");
-   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this assembler directive
+   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: using the .att_syntax directive may cause issues, use the att_syntax option instead
-  --> $DIR/inline-syntax.rs:23:15
+warning: avoid using `.att_syntax`, prefer using `options(att_syntax)` instead
+  --> $DIR/inline-syntax.rs:28:15
    |
 LL |         asm!(".att_syntax noprefix", "nop");
    |               ^^^^^^^^^^^^^^^^^^^^
-   |
-help: remove the assembler directive and replace it with options(att_syntax)
-   |
-LL |         asm!("", "nop", options(att_syntax));
-   |              --       ^^^^^^^^^^^^^^^^^^^^^
 
-error: using the .att_syntax directive may cause issues, use the att_syntax option instead
-  --> $DIR/inline-syntax.rs:26:15
+warning: avoid using `.att_syntax`, prefer using `options(att_syntax)` instead
+  --> $DIR/inline-syntax.rs:31:15
    |
 LL |         asm!(".att_syntax bbb noprefix", "nop");
    |               ^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-help: remove the assembler directive and replace it with options(att_syntax)
-   |
-LL |         asm!("", "nop", options(att_syntax));
-   |              --       ^^^^^^^^^^^^^^^^^^^^^
 
-error: intel syntax is the default syntax on this target, and trying to use this directive may cause issues
-  --> $DIR/inline-syntax.rs:29:15
+warning: avoid using `.intel_syntax`, Intel syntax is the default
+  --> $DIR/inline-syntax.rs:34:15
    |
 LL |         asm!(".intel_syntax noprefix; nop");
-   |               ^^^^^^^^^^^^^^^^^^^^^^ help: remove this assembler directive
+   |               ^^^^^^^^^^^^^^^^^^^^^^
 
-error: intel syntax is the default syntax on this target, and trying to use this directive may cause issues
-  --> $DIR/inline-syntax.rs:34:14
+warning: avoid using `.intel_syntax`, Intel syntax is the default
+  --> $DIR/inline-syntax.rs:40:13
    |
-LL |               .intel_syntax noprefix
-   |  ______________^
-LL | |             nop"
-   | |_ help: remove this assembler directive
+LL |             .intel_syntax noprefix
+   |             ^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 6 previous errors
+warning: 6 warnings emitted
 

Original file line number	Diff line number	Diff line change
`@@ -793,13 +793,6 @@ impl Session {`
`793`	`793`	`}`
`794`	`794`	`}`
`795`	`795`
`796`		`- pub fn inline_asm_dialect(&self) -> rustc_ast::LlvmAsmDialect {`
`797`		`- match self.asm_arch {`
`798`		`- Some(InlineAsmArch::X86 \| InlineAsmArch::X86_64) => rustc_ast::LlvmAsmDialect::Intel,`
`799`		`- _ => rustc_ast::LlvmAsmDialect::Att,`
`800`		`- }`
`801`		`- }`
`802`		`-`
`803`	`796`	`pub fn relocation_model(&self) -> RelocModel {`
`804`	`797`	`self.opts.cg.relocation_model.unwrap_or(self.target.relocation_model)`
`805`	`798`	`}`