Lint against named asm labels

Author: asquared31415

compiler/rustc_builtin_macros/src/asm.rs

@@ -7,7 +7,7 @@ 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};
@@ -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,70 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, args: AsmArgs) -> Option<ast::Inl
             }
         }
 
+        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 {
+                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 and continues with alphanumeric characters
+                        if c.is_alphabetic() {
+                            if chars.all(char::is_alphanumeric) {
+                                found_labels.push(possible_label);
+                            }
+                        }
+                    }
+
+                    start_idx = idx + 1;
+                }
+            }
+
+            if found_labels.len() > 0 {
+                let spans =
+                    found_labels.into_iter().filter_map(find_label_span).collect::<Vec<Span>>();
+                if spans.len() > 0 {
+                    for span in spans.into_iter() {
+                        ecx.parse_sess().buffer_lint_with_diagnostic(
+                            lint::builtin::NAMED_ASM_LABELS,
+                            span,
+                            ecx.current_expansion.lint_node_id,
+                            "do not use named labels in inline assembly",
+                            BuiltinLintDiagnostics::NamedAsmLabel("Only GAS local labels of the form `N:` where N is a number may be used in inline asm".to_string()),
+                        );
+                    }
+                } else {
+                    // If there were labels but we couldn't find a span, combine the warnings and use the template span
+                    ecx.parse_sess().buffer_lint_with_diagnostic(
+                        lint::builtin::NAMED_ASM_LABELS,
+                        template_span,
+                        ecx.current_expansion.lint_node_id,
+                        "do not use named labels in inline assembly",
+                        BuiltinLintDiagnostics::NamedAsmLabel("Only GAS local labels of the form `N:` where N is a number may 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 +738,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 +765,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's assembler 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.

compiler/rustc_lint_defs/src/lib.rs

@@ -274,15 +274,15 @@ impl<HCX> ToStableHashKey<HCX> for LintId {
 }
 
 // Duplicated from rustc_session::config::ExternDepSpec to avoid cyclic dependency
-#[derive(PartialEq, Debug)]
+#[derive(PartialEq)]
 pub enum ExternDepSpec {
     Json(Json),
     Raw(String),
 }
 
 // This could be a closure, but then implementing derive trait
 // becomes hacky (and it gets allocated).
-#[derive(PartialEq, Debug)]
+#[derive(PartialEq)]
 pub enum BuiltinLintDiagnostics {
     Normal,
     BareTraitObject(Span, /* is_global */ bool),
@@ -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/ui/asm/named_asm_labels.rs

@@ -0,0 +1,95 @@
+#![feature(asm, global_asm)]
+
+fn main() {
+    unsafe {
+        // Basic usage
+        asm!("bar: nop"); //~ ERROR do not use named labels
+
+        // No following asm
+        asm!("abcd:"); //~ ERROR do not use named labels
+
+        // Multiple labels on one line
+        asm!("foo: bar1: nop");
+        //~^ ERROR do not use named labels
+        //~| ERROR do not use named labels
+
+        // Multiple lines
+        asm!("foo1: nop", "nop"); //~ ERROR do not use named labels
+        asm!("foo2: foo3: nop", "nop");
+        //~^ ERROR do not use named labels
+        //~| ERROR do not use named labels
+        asm!("nop", "foo4: nop"); //~ ERROR do not use named labels
+        asm!("foo5: nop", "foo6: nop");
+        //~^ ERROR do not use named labels
+        //~| ERROR do not use named labels
+
+        // Statement separator
+        asm!("foo7: nop; foo8: nop");
+        //~^ ERROR do not use named labels
+        //~| ERROR do not use named labels
+        asm!("foo9: nop; nop"); //~ ERROR do not use named labels
+        asm!("nop; foo10: nop"); //~ ERROR do not use named labels
+
+        // Escaped newline
+        asm!("bar2: nop\n bar3: nop");
+        //~^ ERROR do not use named labels
+        //~| ERROR do not use named labels
+        asm!("bar4: nop\n nop"); //~ ERROR do not use named labels
+        asm!("nop\n bar5: nop"); //~ ERROR do not use named labels
+        asm!("nop\n bar6: bar7: nop");
+        //~^ ERROR do not use named labels
+        //~| ERROR do not use named labels
+
+        // Raw strings
+        asm!(
+            r"
+            blah2: nop
+            blah3: nop
+            "
+        );
+        //~^^^^ ERROR do not use named labels
+        //~^^^^ ERROR do not use named labels
+        asm!(
+            r###"
+            nop
+            nop ; blah4: nop
+            "###
+        );
+        //~^^^ ERROR do not use named labels
+
+        // Non-labels
+        // should not trigger lint, but may be invalid asm
+        asm!("ab cd: nop");
+
+        // Only `blah:` should trigger
+        asm!("1bar: blah: nop"); //~ ERROR do not use named labels
+
+        // Only `blah1:` should trigger
+        asm!("blah1: 2bar: nop"); //~ ERROR do not use named labels
+
+        // Duplicate labels
+        asm!("def: def: nop"); //~ ERROR do not use named labels
+        asm!("def: nop\ndef: nop"); //~ ERROR do not use named labels
+        asm!("def: nop; def: nop"); //~ ERROR do not use named labels
+
+        // Trying to break parsing
+        asm!(":");
+        asm!("\n:\n");
+        asm!("::::");
+
+        // 0x3A is a ':'
+        asm!("fooo\u{003A} nop"); //~ ERROR do not use named labels
+        asm!("foooo\x3A nop"); //~ ERROR do not use named labels
+
+        // 0x0A is a newline
+        asm!("fooooo:\u{000A} nop"); //~ ERROR do not use named labels
+        asm!("foooooo:\x0A nop"); //~ ERROR do not use named labels
+
+        // Intentionally breaking span finding
+        // equivalent to "ABC: nop"
+        asm!("\x41\x42\x43\x3A\x20\x6E\x6F\x70"); //~ ERROR do not use named labels
+    }
+}
+
+// Don't trigger on global asm
+global_asm!("aaaaaaaa: nop");