Rollup merge of #119195 - asquared31415:named_asm_labels_fix, r=Amanieu

matthiaskrgr · web-flow · commit 4c9143aea577 · 2024-01-03T22:08:49.000+01:00
Make named_asm_labels lint not trigger on unicode and trigger on format args

Someone showed me some cursed code that used format args to create named labels, and rustc wasn't linting on that.  Additionally while fixing that, I noticed that Unicode alphabetic characters were being used as part of labels, when they are not actually permitted in labels.

r? `@Amanieu`
diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs
@@ -2734,10 +2734,13 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
     #[allow(rustc::diagnostic_outside_of_impl)]
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
         if let hir::Expr {
-            kind: hir::ExprKind::InlineAsm(hir::InlineAsm { template_strs, .. }),
+            kind: hir::ExprKind::InlineAsm(hir::InlineAsm { template_strs, options, .. }),
             ..
         } = expr
         {
+            // asm with `options(raw)` does not do replacement with `{` and `}`.
+            let raw = options.contains(InlineAsmOptions::RAW);
+
             for (template_sym, template_snippet, template_span) in template_strs.iter() {
                 let template_str = template_sym.as_str();
                 let find_label_span = |needle: &str| -> Option<Span> {
@@ -2763,24 +2766,57 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
                 for statement in statements {
                     // If there's a comment, trim it from the statement
                     let statement = statement.find("//").map_or(statement, |idx| &statement[..idx]);
+
+                    // In this loop, if there is ever a non-label, no labels can come after it.
                     let mut start_idx = 0;
-                    for (idx, _) in statement.match_indices(':') {
+                    'label_loop: for (idx, _) in statement.match_indices(':') {
                         let possible_label = statement[start_idx..idx].trim();
                         let mut chars = possible_label.chars();
-                        let Some(c) = chars.next() else {
-                            // Empty string means a leading ':' in this section, which is not a label
-                            break;
+
+                        let Some(start) = chars.next() else {
+                            // Empty string means a leading ':' in this section, which is not a label.
+                            break 'label_loop;
                         };
-                        // 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;
+
+                        // Whether a { bracket has been seen and its } hasn't been found yet.
+                        let mut in_bracket = false;
+
+                        // A label starts with an ASCII alphabetic character or . or _
+                        // A label can also start with a format arg, if it's not a raw asm block.
+                        if !raw && start == '{' {
+                            in_bracket = true;
+                        } else if !(start.is_ascii_alphabetic() || matches!(start, '.' | '_')) {
+                            break 'label_loop;
+                        }
+
+                        // Labels continue with ASCII alphanumeric characters, _, or $
+                        for c in chars {
+                            // Inside a template format arg, any character is permitted for the puproses of label detection
+                            // because we assume that it can be replaced with some other valid label string later.
+                            // `options(raw)` asm blocks cannot have format args, so they are excluded from this special case.
+                            if !raw && in_bracket {
+                                if c == '{' {
+                                    // Nested brackets are not allowed in format args, this cannot be a label.
+                                    break 'label_loop;
+                                }
+
+                                if c == '}' {
+                                    // The end of the format arg.
+                                    in_bracket = false;
+                                }
+                            } else if !raw && c == '{' {
+                                // Start of a format arg.
+                                in_bracket = true;
+                            } else {
+                                if !(c.is_ascii_alphanumeric() || matches!(c, '_' | '$')) {
+                                    // The potential label had an invalid character inside it, it cannot be a label.
+                                    break 'label_loop;
+                                }
+                            }
                         }
 
+                        // If all characters passed the label checks, this is likely a label.
+                        found_labels.push(possible_label);
                         start_idx = idx + 1;
                     }
                 }
diff --git a/tests/ui/asm/named-asm-labels.rs b/tests/ui/asm/named-asm-labels.rs
@@ -120,6 +120,27 @@ fn main() {
         // is there an example that is valid x86 for this test?
         asm!(":bbb nop");
 
+        // non-ascii characters are not allowed in labels, so should not trigger the lint
+        asm!("Ù: nop");
+        asm!("testÙ: nop");
+        asm!("_Ù_: nop");
+
+        // Format arguments should be conservatively assumed to be valid characters in labels
+        // Would emit `test_rax:` or similar
+        #[allow(asm_sub_register)]
+        {
+            asm!("test_{}: nop", in(reg) 10); //~ ERROR avoid using named labels
+        }
+        asm!("test_{}: nop", const 10); //~ ERROR avoid using named labels
+        asm!("test_{}: nop", sym main); //~ ERROR avoid using named labels
+        asm!("{}_test: nop", const 10); //~ ERROR avoid using named labels
+        asm!("test_{}_test: nop", const 10); //~ ERROR avoid using named labels
+        asm!("{}: nop", const 10); //~ ERROR avoid using named labels
+
+        asm!("{uwu}: nop", uwu = const 10); //~ ERROR avoid using named labels
+        asm!("{0}: nop", const 10); //~ ERROR avoid using named labels
+        asm!("{1}: nop", "/* {0} */", const 10, const 20); //~ ERROR avoid using named labels
+
         // Test include_str in asm
         asm!(include_str!("named-asm-labels.s")); //~ ERROR avoid using named labels
 
diff --git a/tests/ui/asm/named-asm-labels.stderr b/tests/ui/asm/named-asm-labels.stderr
@@ -245,7 +245,88 @@ LL |             ab: nop // ab: does foo
    = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
 
 error: avoid using named labels in inline assembly
-  --> $DIR/named-asm-labels.rs:124:14
+  --> $DIR/named-asm-labels.rs:132:19
+   |
+LL |             asm!("test_{}: nop", in(reg) 10);
+   |                   ^^^^^^^
+   |
+   = help: only local labels of the form `<number>:` should be used in inline asm
+   = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
+
+error: avoid using named labels in inline assembly
+  --> $DIR/named-asm-labels.rs:134:15
+   |
+LL |         asm!("test_{}: nop", const 10);
+   |               ^^^^^^^
+   |
+   = help: only local labels of the form `<number>:` should be used in inline asm
+   = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
+
+error: avoid using named labels in inline assembly
+  --> $DIR/named-asm-labels.rs:135:15
+   |
+LL |         asm!("test_{}: nop", sym main);
+   |               ^^^^^^^
+   |
+   = help: only local labels of the form `<number>:` should be used in inline asm
+   = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
+
+error: avoid using named labels in inline assembly
+  --> $DIR/named-asm-labels.rs:136:15
+   |
+LL |         asm!("{}_test: nop", const 10);
+   |               ^^^^^^^
+   |
+   = help: only local labels of the form `<number>:` should be used in inline asm
+   = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
+
+error: avoid using named labels in inline assembly
+  --> $DIR/named-asm-labels.rs:137:15
+   |
+LL |         asm!("test_{}_test: nop", const 10);
+   |               ^^^^^^^^^^^^
+   |
+   = help: only local labels of the form `<number>:` should be used in inline asm
+   = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
+
+error: avoid using named labels in inline assembly
+  --> $DIR/named-asm-labels.rs:138:15
+   |
+LL |         asm!("{}: nop", const 10);
+   |               ^^
+   |
+   = help: only local labels of the form `<number>:` should be used in inline asm
+   = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
+
+error: avoid using named labels in inline assembly
+  --> $DIR/named-asm-labels.rs:140:15
+   |
+LL |         asm!("{uwu}: nop", uwu = const 10);
+   |               ^^^^^
+   |
+   = help: only local labels of the form `<number>:` should be used in inline asm
+   = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
+
+error: avoid using named labels in inline assembly
+  --> $DIR/named-asm-labels.rs:141:15
+   |
+LL |         asm!("{0}: nop", const 10);
+   |               ^^^
+   |
+   = help: only local labels of the form `<number>:` should be used in inline asm
+   = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
+
+error: avoid using named labels in inline assembly
+  --> $DIR/named-asm-labels.rs:142:15
+   |
+LL |         asm!("{1}: nop", "/* {0} */", const 10, const 20);
+   |               ^^^
+   |
+   = help: only local labels of the form `<number>:` should be used in inline asm
+   = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
+
+error: avoid using named labels in inline assembly
+  --> $DIR/named-asm-labels.rs:145:14
    |
 LL |         asm!(include_str!("named-asm-labels.s"));
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -254,21 +335,21 @@ LL |         asm!(include_str!("named-asm-labels.s"));
    = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
 
 warning: avoid using named labels in inline assembly
-  --> $DIR/named-asm-labels.rs:134:19
+  --> $DIR/named-asm-labels.rs:155:19
    |
 LL |             asm!("warned: nop");
    |                   ^^^^^^
    |
    = help: only local labels of the form `<number>:` should be used in inline asm
    = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
 note: the lint level is defined here
-  --> $DIR/named-asm-labels.rs:132:16
+  --> $DIR/named-asm-labels.rs:153:16
    |
 LL |         #[warn(named_asm_labels)]
    |                ^^^^^^^^^^^^^^^^
 
 error: avoid using named labels in inline assembly
-  --> $DIR/named-asm-labels.rs:143:20
+  --> $DIR/named-asm-labels.rs:164:20
    |
 LL |     unsafe { asm!(".Lfoo: mov rax, {}; ret;", "nop", const 1, options(noreturn)) }
    |                    ^^^^^
@@ -277,7 +358,7 @@ LL |     unsafe { asm!(".Lfoo: mov rax, {}; ret;", "nop", const 1, options(noret
    = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
 
 error: avoid using named labels in inline assembly
-  --> $DIR/named-asm-labels.rs:149:20
+  --> $DIR/named-asm-labels.rs:170:20
    |
 LL |     unsafe { asm!(".Lbar: mov rax, {}; ret;", "nop", const 1, options(noreturn)) }
    |                    ^^^^^
@@ -286,7 +367,7 @@ LL |     unsafe { asm!(".Lbar: mov rax, {}; ret;", "nop", const 1, options(noret
    = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
 
 error: avoid using named labels in inline assembly
-  --> $DIR/named-asm-labels.rs:157:20
+  --> $DIR/named-asm-labels.rs:178:20
    |
 LL |     unsafe { asm!(".Laaa: nop; ret;", options(noreturn)) }
    |                    ^^^^^
@@ -295,7 +376,7 @@ LL |     unsafe { asm!(".Laaa: nop; ret;", options(noreturn)) }
    = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
 
 error: avoid using named labels in inline assembly
-  --> $DIR/named-asm-labels.rs:167:24
+  --> $DIR/named-asm-labels.rs:188:24
    |
 LL |         unsafe { asm!(".Lbbb: nop; ret;", options(noreturn)) }
    |                        ^^^^^
@@ -304,7 +385,7 @@ LL |         unsafe { asm!(".Lbbb: nop; ret;", options(noreturn)) }
    = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
 
 error: avoid using named labels in inline assembly
-  --> $DIR/named-asm-labels.rs:176:15
+  --> $DIR/named-asm-labels.rs:197:15
    |
 LL |         asm!("closure1: nop");
    |               ^^^^^^^^
@@ -313,7 +394,7 @@ LL |         asm!("closure1: nop");
    = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
 
 error: avoid using named labels in inline assembly
-  --> $DIR/named-asm-labels.rs:180:15
+  --> $DIR/named-asm-labels.rs:201:15
    |
 LL |         asm!("closure2: nop");
    |               ^^^^^^^^
@@ -322,13 +403,13 @@ LL |         asm!("closure2: nop");
    = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
 
 error: avoid using named labels in inline assembly
-  --> $DIR/named-asm-labels.rs:190:19
+  --> $DIR/named-asm-labels.rs:211:19
    |
 LL |             asm!("closure3: nop");
    |                   ^^^^^^^^
    |
    = help: only local labels of the form `<number>:` should be used in inline asm
    = note: see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
 
-error: aborting due to 35 previous errors; 1 warning emitted
+error: aborting due to 44 previous errors; 1 warning emitted
 
