Lint against named asm labels

compiler/rustc_builtin_macros/src/asm.rs

@@ -7,10 +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_session::lint;
+use rustc_session::lint::{self, BuiltinLintDiagnostics};
 use rustc_span::symbol::Ident;
 use rustc_span::symbol::{kw, sym, Symbol};
-use rustc_span::{InnerSpan, Span};
+use rustc_span::{InnerSpan, MultiSpan, Span};
 use rustc_target::asm::InlineAsmArch;
 use smallvec::smallvec;
 
@@ -397,7 +397,11 @@ fn parse_reg<'a>(
  Ok(result)
 }
 
-fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, args: AsmArgs) -> Option<ast::InlineAsm> {
+fn expand_preparsed_asm(
+ ecx: &mut ExtCtxt<'_>,
+ args: AsmArgs,
+ is_local_asm: bool,
+) -> Option<ast::InlineAsm> {
  let mut template = vec![];
  // Register operands are implicitly used since they are not allowed to be
  // referenced in the template string.
@@ -469,6 +473,72 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, args: AsmArgs) -> Option<ast::Inl
  }
  }
 
+ // Lint against the use of named labels in inline `asm!` but not `global_asm!`
+ if is_local_asm {
+ let find_label_span = |needle: &str| -> Option<Span> {
+ if let Some(snippet) = &template_snippet {
+ if let Some(pos) = snippet.find(needle) {
+ let end = pos
+ + &snippet[pos..]
+ .find(|c| c == ':')
+ .unwrap_or(snippet[pos..].len() - 1);
+ let inner = InnerSpan::new(pos, end);
+ return Some(template_sp.from_inner(inner));
+ }
+ }
+
+ None
+ };
+
+ let mut found_labels = Vec::new();
+
+ // A semicolon might not actually be specified as a separator for all targets, but it seems like LLVM accepts it always
+ let statements = template_str.split(|c| matches!(c, '\n' | ';'));
+ for statement in statements {
+ // If there's a comment, trim it from the statement
+ let statement = statement.find("//").map_or(statement, |idx| &statement[..idx]);
+ let mut start_idx = 0;
+ for (idx, _) in statement.match_indices(':') {
+ let possible_label = statement[start_idx..idx].trim();
+ let mut chars = possible_label.chars();
+ if let Some(c) = chars.next() {
+ // A label starts with an alphabetic character or . or _ and continues with alphanumeric characters, _, or $
+ if (c.is_alphabetic() || matches!(c, '.' | '_'))
+ && chars.all(|c| c.is_alphanumeric() || matches!(c, '_' | '$'))
+ {
+ found_labels.push(possible_label);
+ } else {
+ // If we encounter a non-label, there cannot be any further labels, so stop checking
+ break;
+ }
+ } else {
+ // Empty string means a leading ':' in this section, which is not a label
+ break;
+ }
+
+ start_idx = idx + 1;
+ }
+ }
+
+ if found_labels.len() > 0 {
+ let spans =
+ found_labels.into_iter().filter_map(find_label_span).collect::<Vec<Span>>();
+ // If there were labels but we couldn't find a span, combine the warnings and use the template span
+ let target_spans: MultiSpan =
+ if spans.len() > 0 { spans.into() } else { template_sp.into() };
+ ecx.parse_sess().buffer_lint_with_diagnostic(
+ lint::builtin::NAMED_ASM_LABELS,
+ target_spans,
+ ecx.current_expansion.lint_node_id,
+ "avoid using named labels in inline assembly",
+ BuiltinLintDiagnostics::NamedAsmLabel(
+ "only local labels of the form `<number>:` should be used in inline asm"
+ .to_string(),
+ ),
+ );
+ }
+ }
+
  // Don't treat raw asm as a format string.
  if args.options.contains(ast::InlineAsmOptions::RAW) {
  template.push(ast::InlineAsmTemplatePiece::String(template_str.to_string()));
@@ -670,7 +740,7 @@ pub fn expand_asm<'cx>(
 ) -> Box<dyn base::MacResult + 'cx> {
  match parse_args(ecx, sp, tts, false) {
  Ok(args) => {
- let expr = if let Some(inline_asm) = expand_preparsed_asm(ecx, args) {
+ let expr = if let Some(inline_asm) = expand_preparsed_asm(ecx, args, true) {
  P(ast::Expr {
  id: ast::DUMMY_NODE_ID,
  kind: ast::ExprKind::InlineAsm(P(inline_asm)),
@@ -697,7 +767,7 @@ pub fn expand_global_asm<'cx>(
 ) -> Box<dyn base::MacResult + 'cx> {
  match parse_args(ecx, sp, tts, true) {
  Ok(args) => {
- if let Some(inline_asm) = expand_preparsed_asm(ecx, args) {
+ if let Some(inline_asm) = expand_preparsed_asm(ecx, args, false) {
  MacEager::items(smallvec![P(ast::Item {
  ident: Ident::invalid(),
  attrs: Vec::new(),

compiler/rustc_lint/src/context.rs

@@ -758,6 +758,10 @@ pub trait LintContext: Sized {
  Applicability::MachineApplicable
  );
  }
+ BuiltinLintDiagnostics::NamedAsmLabel(help) => {
+ db.help(&help);
+ db.note("see the asm section of the unstable book <https://doc.rust-lang.org/nightly/unstable-book/library-features/asm.html#labels> for more information");
+ }
  }
  // Rewrap `db`, and pass control to the user.
  decorate(LintDiagnosticBuilder::new(db));

compiler/rustc_lint_defs/src/builtin.rs

@@ -2470,6 +2470,38 @@ declare_lint! {
  "incorrect use of inline assembly",
 }
 
+declare_lint! {
+ /// The `named_asm_labels` lint detects the use of named labels in the
+ /// inline `asm!` macro.
+ ///
+ /// ### Example
+ ///
+ /// ```rust,compile_fail
+ /// fn main() {
+ /// unsafe {
+ /// asm!("foo: bar");
+ /// }
+ /// }
+ /// ```
+ ///
+ /// {{produces}}
+ ///
+ /// ### Explanation
+ ///
+ /// LLVM is allowed to duplicate inline assembly blocks for any
+ /// reason, for example when it is in a function that gets inlined. Because
+ /// of this, GNU assembler [local labels] *must* be used instead of labels
+ /// with a name. Using named labels might cause assembler or linker errors.
+ ///
+ /// See the [unstable book] for more details.
+ ///
+ /// [local labels]: https://sourceware.org/binutils/docs/as/Symbol-Names.html#Local-Labels
+ /// [unstable book]: https://doc.rust-lang.org/nightly/unstable-book/library-features/asm.html#labels
+ pub NAMED_ASM_LABELS,
+ Deny,
+ "named labels in inline assembly",
+}
+
 declare_lint! {
  /// The `unsafe_op_in_unsafe_fn` lint detects unsafe operations in unsafe
  /// functions without an explicit unsafe block.
@@ -2958,6 +2990,7 @@ declare_lint_pass! {
  INLINE_NO_SANITIZE,
  BAD_ASM_STYLE,
  ASM_SUB_REGISTER,
+ NAMED_ASM_LABELS,
  UNSAFE_OP_IN_UNSAFE_FN,
  INCOMPLETE_INCLUDE,
  CENUM_IMPL_DROP_CAST,

compiler/rustc_lint_defs/src/lib.rs

@@ -305,6 +305,7 @@ pub enum BuiltinLintDiagnostics {
  ReservedPrefix(Span),
  TrailingMacro(bool, Ident),
  BreakWithLabelAndLoop(Span),
+ NamedAsmLabel(String),
 }
 
 /// Lints that are buffered up early on in the `Session` before the

src/test/codegen/asm-sanitize-llvm.rs

@@ -7,6 +7,7 @@
 #![no_core]
 #![feature(no_core, lang_items, rustc_attrs)]
 #![crate_type = "rlib"]
+#![allow(named_asm_labels)]
 
 #[rustc_builtin_macro]
 macro_rules! asm {

src/test/ui/asm/named-asm-labels.rs

@@ -0,0 +1,130 @@
+// only-x86_64
+
+#![feature(asm, global_asm)]
+
+#[no_mangle]
+pub static FOO: usize = 42;
+
+fn main() {
+ unsafe {
+ // Basic usage
+ asm!("bar: nop"); //~ ERROR avoid using named labels
+
+ // No following asm
+ asm!("abcd:"); //~ ERROR avoid using named labels
+
+ // Multiple labels on one line
+ asm!("foo: bar1: nop");
+ //~^ ERROR avoid using named labels
+
+ // Multiple lines
+ asm!("foo1: nop", "nop"); //~ ERROR avoid using named labels
+ asm!("foo2: foo3: nop", "nop");
+ //~^ ERROR avoid using named labels
+ asm!("nop", "foo4: nop"); //~ ERROR avoid using named labels
+ asm!("foo5: nop", "foo6: nop");
+ //~^ ERROR avoid using named labels
+ //~| ERROR avoid using named labels
+
+ // Statement separator
+ asm!("foo7: nop; foo8: nop");
+ //~^ ERROR avoid using named labels
+ asm!("foo9: nop; nop"); //~ ERROR avoid using named labels
+ asm!("nop; foo10: nop"); //~ ERROR avoid using named labels
+
+ // Escaped newline
+ asm!("bar2: nop\n bar3: nop");
+ //~^ ERROR avoid using named labels
+ asm!("bar4: nop\n nop"); //~ ERROR avoid using named labels
+ asm!("nop\n bar5: nop"); //~ ERROR avoid using named labels
+ asm!("nop\n bar6: bar7: nop");
+ //~^ ERROR avoid using named labels
+
+ // Raw strings
+ asm!(
+ r"
+ blah2: nop
+ blah3: nop
+ "
+ );
+ //~^^^^ ERROR avoid using named labels
+
+ asm!(
+ r###"
+ nop
+ nop ; blah4: nop
+ "###
+ );
+ //~^^^ ERROR avoid using named labels
+
+ // Non-labels
+ // should not trigger lint, but may be invalid asm
+ asm!("ab cd: nop");
+
+ // `blah:` does not trigger because labels need to be at the start
+ // of the statement, and there was already a non-label
+ asm!("1bar: blah: nop");
+
+ // Only `blah1:` should trigger
+ asm!("blah1: 2bar: nop"); //~ ERROR avoid using named labels
+
+ // Duplicate labels
+ asm!("def: def: nop"); //~ ERROR avoid using named labels
+ asm!("def: nop\ndef: nop"); //~ ERROR avoid using named labels
+ asm!("def: nop; def: nop"); //~ ERROR avoid using named labels
+
+ // Trying to break parsing
+ asm!(":");
+ asm!("\n:\n");
+ asm!("::::");
+
+ // 0x3A is a ':'
+ asm!("fooo\u{003A} nop"); //~ ERROR avoid using named labels
+ asm!("foooo\x3A nop"); //~ ERROR avoid using named labels
+
+ // 0x0A is a newline
+ asm!("fooooo:\u{000A} nop"); //~ ERROR avoid using named labels
+ asm!("foooooo:\x0A nop"); //~ ERROR avoid using named labels
+
+ // Intentionally breaking span finding
+ // equivalent to "ABC: nop"
+ asm!("\x41\x42\x43\x3A\x20\x6E\x6F\x70"); //~ ERROR avoid using named labels
+
+ // Non-label colons - should pass
+ // (most of these are stolen from other places)
+ asm!("{:l}", in(reg) 0i64);
+ asm!("{:e}", in(reg) 0f32);
+ asm!("mov rax, qword ptr fs:[0]");
+
+ // Comments
+ asm!(
+ r"
+ ab: nop // ab: does foo
+ // cd: nop
+ "
+ );
+ //~^^^^ ERROR avoid using named labels
+
+ // Tests usage of colons in non-label positions
+ asm!(":lo12:FOO"); // this is apparently valid aarch64
+ // is there an example that is valid x86 for this test?
+ asm!(":bbb nop");
+
+ // Test include_str in asm
+ asm!(include_str!("named-asm-labels.s")); //~ ERROR avoid using named labels
+
+ // Test allowing or warning on the lint instead
+ #[allow(named_asm_labels)]
+ {
+ asm!("allowed: nop"); // Should not emit anything
+ }
+
+ #[warn(named_asm_labels)]
+ {
+ asm!("warned: nop"); //~ WARNING avoid using named labels
+ }
+ }
+}
+
+// Don't trigger on global asm
+global_asm!("aaaaaaaa: nop");

src/test/ui/asm/named-asm-labels.s

@@ -0,0 +1,5 @@
+lab1: nop
+// do more things
+lab2: nop // does bar
+// a: b
+lab3: nop; lab4: nop