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.

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

Allow unused arguments in asm! #73056

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/doc/unstable-book/src/library-features/asm.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ fn mul(a: u64, b: u64) -> u128 {
);
}

(hi as u128) << 64 + lo as u128
((hi as u128) << 64) + lo as u128
}
```

Expand Down Expand Up @@ -382,7 +382,9 @@ The macro will initially be supported only on ARM, AArch64, x86, x86-64 and RISC

The assembler template uses the same syntax as [format strings][format-syntax] (i.e. placeholders are specified by curly braces). The corresponding arguments are accessed in order, by index, or by name. However, implicit named arguments (introduced by [RFC #2795][rfc-2795]) are not supported.

As with format strings, named arguments must appear after positional arguments. Explicit register operands must appear at the end of the operand list, after any named arguments if any. Explicit register operands cannot be used by placeholders in the template string. All other operands must appear at least once in the template string, otherwise a compiler error is generated.
As with format strings, named arguments must appear after positional arguments. Explicit register operands must appear at the end of the operand list, after named arguments if any.

Explicit register operands cannot be used by placeholders in the template string. All other named and positional operands must appear at least once in the template string, otherwise the compiler will emit a warning.

The exact assembly code syntax is target-specific and opaque to the compiler except for the way operands are substituted into the template string to form the code passed to the assembler.

Expand Down
35 changes: 32 additions & 3 deletions src/librustc_ast_lowering/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ use rustc_ast::ptr::P as AstP;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_data_structures::thin_vec::ThinVec;
use rustc_errors::struct_span_err;
use rustc_errors::{pluralize, struct_span_err};
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_session::lint::builtin::UNUSED_ASM_ARGUMENTS;
use rustc_span::source_map::{respan, DesugaringKind, Span, Spanned};
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_target::asm;
Expand Down Expand Up @@ -179,7 +180,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let e = e.as_ref().map(|x| self.lower_expr(x));
hir::ExprKind::Ret(e)
}
ExprKind::InlineAsm(ref asm) => self.lower_expr_asm(e.span, asm),
ExprKind::InlineAsm(ref asm) => self.lower_expr_asm(e.span, e.id, asm),
ExprKind::LlvmInlineAsm(ref asm) => self.lower_expr_llvm_asm(asm),
ExprKind::Struct(ref path, ref fields, ref maybe_expr) => {
let maybe_expr = maybe_expr.as_ref().map(|x| self.lower_expr(x));
Expand Down Expand Up @@ -973,7 +974,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
result
}

fn lower_expr_asm(&mut self, sp: Span, asm: &InlineAsm) -> hir::ExprKind<'hir> {
fn lower_expr_asm(&mut self, sp: Span, id: NodeId, asm: &InlineAsm) -> hir::ExprKind<'hir> {
if self.sess.asm_arch.is_none() {
struct_span_err!(self.sess, sp, E0472, "asm! is unsupported on this target").emit();
}
Expand Down Expand Up @@ -1265,6 +1266,34 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
}

// Warn about operands that are not referenced in the template string.
let mut used = vec![false; operands.len()];
for p in &asm.template {
if let InlineAsmTemplatePiece::Placeholder { operand_idx, modifier: _, span: _ } = *p {
used[operand_idx] = true;
}
}
let unused_operands: Vec<_> = used
.into_iter()
.enumerate()
.filter(|&(idx, used)| {
// Register operands are implicitly used since they are not allowed to be
// referenced in the template string.
!used && !matches!(operands[idx].reg(), Some(asm::InlineAsmRegOrRegClass::Reg(_)))
})
.map(|(idx, _)| asm.operands[idx].1)
.collect();
if !unused_operands.is_empty() {
let msg =
format!("asm argument{} not used in template", pluralize!(unused_operands.len()));
self.resolver.lint_buffer().buffer_lint(
UNUSED_ASM_ARGUMENTS,
id,
unused_operands,
&msg,
);
}

let operands = self.arena.alloc_from_iter(operands);
let template = self.arena.alloc_from_iter(asm.template.iter().cloned());
let line_spans = self.arena.alloc_slice(&asm.line_spans[..]);
Expand Down
46 changes: 2 additions & 44 deletions src/librustc_builtin_macros/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,13 +384,6 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, sp: Span, args: AsmArgs) -> P<ast
return DummyResult::raw_expr(sp, true);
}

// Register operands are implicitly used since they are not allowed to be
// referenced in the template string.
let mut used = vec![false; args.operands.len()];
for pos in &args.reg_args {
used[*pos] = true;
}

let named_pos: FxHashSet<usize> = args.named_args.values().cloned().collect();
let mut arg_spans = parser.arg_places.iter().map(|span| template_span.from_inner(*span));
let mut template = vec![];
Expand Down Expand Up @@ -469,7 +462,6 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, sp: Span, args: AsmArgs) -> P<ast
}

if let Some(operand_idx) = operand_idx {
used[operand_idx] = true;
template.push(ast::InlineAsmTemplatePiece::Placeholder {
operand_idx,
modifier,
Expand All @@ -480,48 +472,14 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, sp: Span, args: AsmArgs) -> P<ast
}
}

let operands = args.operands;
let unused_operands: Vec<_> = used
.into_iter()
.enumerate()
.filter(|&(_, used)| !used)
.map(|(idx, _)| {
if named_pos.contains(&idx) {
// named argument
(operands[idx].1, "named argument never used")
} else {
// positional argument
(operands[idx].1, "argument never used")
}
})
.collect();
match unused_operands.len() {
0 => {}
1 => {
let (sp, msg) = unused_operands.into_iter().next().unwrap();
let mut err = ecx.struct_span_err(sp, msg);
err.span_label(sp, msg);
err.emit();
}
_ => {
let mut err = ecx.struct_span_err(
unused_operands.iter().map(|&(sp, _)| sp).collect::<Vec<Span>>(),
"multiple unused asm arguments",
);
for (sp, msg) in unused_operands {
err.span_label(sp, msg);
}
err.emit();
}
}

let line_spans = if parser.line_spans.is_empty() {
vec![template_sp]
} else {
parser.line_spans.iter().map(|span| template_span.from_inner(*span)).collect()
};

let inline_asm = ast::InlineAsm { template, operands, options: args.options, line_spans };
let inline_asm =
ast::InlineAsm { template, operands: args.operands, options: args.options, line_spans };
P(ast::Expr {
id: ast::DUMMY_NODE_ID,
kind: ast::ExprKind::InlineAsm(P(inline_asm)),
Expand Down
7 changes: 7 additions & 0 deletions src/librustc_session/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,12 @@ declare_lint! {
"unsafe operations in unsafe functions without an explicit unsafe block are deprecated",
}

declare_lint! {
pub UNUSED_ASM_ARGUMENTS,
Warn,
"inline asm arguments not used in the template string",
}
Amanieu marked this conversation as resolved.
Show resolved Hide resolved

declare_lint_pass! {
/// Does nothing as a lint pass, but registers some `Lint`s
/// that are used by other parts of the compiler.
Expand Down Expand Up @@ -604,6 +610,7 @@ declare_lint_pass! {
INLINE_NO_SANITIZE,
ASM_SUB_REGISTER,
UNSAFE_OP_IN_UNSAFE_FN,
UNUSED_ASM_ARGUMENTS,
]
}

Expand Down
10 changes: 7 additions & 3 deletions src/test/ui/asm/bad-template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,22 @@ fn main() {
//~^ ERROR invalid reference to argument at index 0
asm!("{1}", in(reg) foo);
//~^ ERROR invalid reference to argument at index 1
//~^^ ERROR argument never used
//~^^ WARN asm argument not used in template
asm!("{a}");
//~^ ERROR there is no argument named `a`
asm!("{}", a = in(reg) foo);
//~^ ERROR invalid reference to argument at index 0
//~^^ ERROR argument never used
//~^^ WARN asm argument not used in template
asm!("{1}", a = in(reg) foo);
//~^ ERROR invalid reference to argument at index 1
//~^^ ERROR named argument never used
//~^^ WARN asm argument not used in template
asm!("{}", in("eax") foo);
//~^ ERROR invalid reference to argument at index 0
asm!("{:foo}", in(reg) foo);
//~^ ERROR asm template modifier must be a single character
asm!("", in(reg) 0, in(reg) 1);
//~^ WARN asm arguments not used in template
#[allow(unused_asm_arguments)]
asm!("", in(reg) 0, in(reg) 1);
}
}
52 changes: 33 additions & 19 deletions src/test/ui/asm/bad-template.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@ LL | asm!("{1}", in(reg) foo);
|
= note: there is 1 argument

error: argument never used
--> $DIR/bad-template.rs:10:21
|
LL | asm!("{1}", in(reg) foo);
| ^^^^^^^^^^^ argument never used

error: there is no argument named `a`
--> $DIR/bad-template.rs:13:15
|
Expand All @@ -41,12 +35,6 @@ note: named arguments cannot be referenced by position
LL | asm!("{}", a = in(reg) foo);
| ^^^^^^^^^^^^^^^

error: named argument never used
--> $DIR/bad-template.rs:15:20
|
LL | asm!("{}", a = in(reg) foo);
| ^^^^^^^^^^^^^^^ named argument never used

error: invalid reference to argument at index 1
--> $DIR/bad-template.rs:18:15
|
Expand All @@ -55,12 +43,6 @@ LL | asm!("{1}", a = in(reg) foo);
|
= note: no positional arguments were given

error: named argument never used
--> $DIR/bad-template.rs:18:21
|
LL | asm!("{1}", a = in(reg) foo);
| ^^^^^^^^^^^^^^^ named argument never used

error: invalid reference to argument at index 0
--> $DIR/bad-template.rs:21:15
|
Expand All @@ -82,5 +64,37 @@ error: asm template modifier must be a single character
LL | asm!("{:foo}", in(reg) foo);
| ^^^

error: aborting due to 10 previous errors
warning: asm argument not used in template
--> $DIR/bad-template.rs:10:21
|
LL | asm!("{1}", in(reg) foo);
| ^^^^^^^^^^^
|
= note: `#[warn(unused_asm_arguments)]` on by default

warning: asm argument not used in template
--> $DIR/bad-template.rs:15:20
|
LL | asm!("{}", a = in(reg) foo);
| ^^^^^^^^^^^^^^^

warning: asm argument not used in template
--> $DIR/bad-template.rs:18:21
|
LL | asm!("{1}", a = in(reg) foo);
| ^^^^^^^^^^^^^^^

warning: asm arguments not used in template
--> $DIR/bad-template.rs:25:18
|
LL | asm!("", in(reg) 0, in(reg) 1);
| ^^^^^^^^^ ^^^^^^^^^

warning: asm arguments not used in template
--> $DIR/bad-template.rs:28:18
|
LL | asm!("", in(reg) 0, in(reg) 1);
| ^^^^^^^^^ ^^^^^^^^^

error: aborting due to 7 previous errors; 5 warnings emitted

2 changes: 1 addition & 1 deletion src/test/ui/asm/parse-error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ fn main() {
//~^ ERROR arguments are not allowed after options
asm!("{a}", a = const foo, a = const bar);
//~^ ERROR duplicate argument named `a`
//~^^ ERROR argument never used
//~^^ WARN asm argument not used in template
asm!("", a = in("eax") foo);
//~^ ERROR explicit register arguments cannot have names
asm!("{a}", in("eax") foo, a = const bar);
Expand Down
16 changes: 9 additions & 7 deletions src/test/ui/asm/parse-error.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,6 @@ LL | asm!("{a}", a = const foo, a = const bar);
| |
| previously here

error: argument never used
--> $DIR/parse-error.rs:44:36
|
LL | asm!("{a}", a = const foo, a = const bar);
| ^^^^^^^^^^^^^ argument never used

error: explicit register arguments cannot have names
--> $DIR/parse-error.rs:47:18
|
Expand Down Expand Up @@ -158,5 +152,13 @@ LL | asm!("{1}", in("eax") foo, const bar);
| |
| explicit register argument

error: aborting due to 24 previous errors
warning: asm argument not used in template
--> $DIR/parse-error.rs:44:36
|
LL | asm!("{a}", a = const foo, a = const bar);
| ^^^^^^^^^^^^^
|
= note: `#[warn(unused_asm_arguments)]` on by default

error: aborting due to 23 previous errors; 1 warning emitted