From 5dabc80796822c80223a2e51dcd8cd3c752dd6d9 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Wed, 24 Mar 2021 04:52:57 +0000 Subject: [PATCH] Refactor #82270 as lint instead of an error --- compiler/rustc_builtin_macros/src/asm.rs | 95 +++++++-------------- compiler/rustc_lint_defs/src/builtin.rs | 46 ++++++++++ compiler/rustc_session/src/session.rs | 7 -- src/test/ui/asm/inline-syntax.arm.stderr | 74 ++++++++++++++-- src/test/ui/asm/inline-syntax.rs | 25 ++++-- src/test/ui/asm/inline-syntax.x86_64.stderr | 50 +++++------ 6 files changed, 179 insertions(+), 118 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/asm.rs b/compiler/rustc_builtin_macros/src/asm.rs index 8d8b3f4f6aaac..e6afc81d0396a 100644 --- 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>, @@ -402,8 +401,6 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, sp: Span, args: AsmArgs) -> P, sp: Span, args: AsmArgs) -> P { - 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>(piece: S, syntax: &str) -> Option { - 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 index 005c4f9f6eaf7..cd4d01ddc058e 100644 --- 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 index fc57b6b8acedf..4f6111183607c 100644 --- 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 index 78b85dfde33e9..56e6572fc6703 100644 --- 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 + --> :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 + --> :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 + --> :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 + --> :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 + --> :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 + --> :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 index 9048282456eb2..78dde5a58e150 100644 --- 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 index 826657c98e154..5c03d3a002c5a 100644 --- 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