Auto merge of #87324 - asquared31415:named-asm-labels, r=Amanieu

Lint against named asm labels

This adds a deny-by-default lint to prevent the use of named labels in inline `asm!`.  Without a solution to #81088 about whether the compiler should rewrite named labels or a special syntax for labels, a lint against them should prevent users from writing assembly that could break for internal compiler reasons, such as inlining or anything else that could change the number of actual inline assembly blocks emitted.

This does **not** resolve the issue with rewriting labels, that still needs a decision if the compiler should do any more work to try to make them work.

Author: bors

Diff for: 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(),

Diff for: 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));

Diff for: compiler/rustc_lint_defs/src/builtin.rs

@@ -2468,6 +2468,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.
@@ -2988,6 +3020,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,

Diff for: 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

Diff for: 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 {

Diff for: 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");

Diff for: 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