Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor #82270 as lint instead of an error #83437

Merged
merged 1 commit into from
Mar 26, 2021
+179 −118
Merged

Refactor #82270 as lint instead of an error #83437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 30 additions & 65 deletions compiler/rustc_builtin_macros/src/asm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
Expand Down Expand Up @@ -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()));
Expand All @@ -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",
);
}
}

Expand Down Expand Up @@ -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
}
}
46 changes: 46 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 0 additions & 7 deletions compiler/rustc_session/src/session.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
74 changes: 67 additions & 7 deletions src/test/ui/asm/inline-syntax.arm.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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

25 changes: 16 additions & 9 deletions src/test/ui/asm/inline-syntax.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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]
Expand All @@ -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
}
}
50 changes: 20 additions & 30 deletions src/test/ui/asm/inline-syntax.x86_64.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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