From 87fed9778e3d7d4e1a9c4ff9ff7d3aede639f761 Mon Sep 17 00:00:00 2001 From: asquared31415 <34665709+asquared31415@users.noreply.github.com> Date: Thu, 21 Dec 2023 17:32:58 +0000 Subject: [PATCH] Make named_asm_labels lint not trigger on unicode and trigger on format args --- compiler/rustc_lint/src/builtin.rs | 62 ++++++++++++---- tests/ui/asm/named-asm-labels.rs | 21 ++++++ tests/ui/asm/named-asm-labels.stderr | 103 ++++++++++++++++++++++++--- 3 files changed, 162 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 045ff38c0568c..34994ca2e8fc1 100644 --- 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 { @@ -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 index 160dbf617c4f6..24586b39aacc0 100644 --- 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 index c8380629e12ea..89c058499675c 100644 --- 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 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 `:` should be used in inline asm + = note: see the asm section of Rust By Example 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 `:` should be used in inline asm + = note: see the asm section of Rust By Example 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 `:` should be used in inline asm + = note: see the asm section of Rust By Example 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 `:` should be used in inline asm + = note: see the asm section of Rust By Example 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 `:` should be used in inline asm + = note: see the asm section of Rust By Example 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 `:` should be used in inline asm + = note: see the asm section of Rust By Example 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 `:` should be used in inline asm + = note: see the asm section of Rust By Example 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 `:` should be used in inline asm + = note: see the asm section of Rust By Example 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 `:` should be used in inline asm + = note: see the asm section of Rust By Example 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,7 +335,7 @@ LL | asm!(include_str!("named-asm-labels.s")); = note: see the asm section of Rust By Example 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"); | ^^^^^^ @@ -262,13 +343,13 @@ LL | asm!("warned: nop"); = help: only local labels of the form `:` should be used in inline asm = note: see the asm section of Rust By Example 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 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 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 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 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 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,7 +403,7 @@ LL | asm!("closure2: nop"); = note: see the asm section of Rust By Example 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"); | ^^^^^^^^ @@ -330,5 +411,5 @@ LL | asm!("closure3: nop"); = help: only local labels of the form `:` should be used in inline asm = note: see the asm section of Rust By Example for more information -error: aborting due to 35 previous errors; 1 warning emitted +error: aborting due to 44 previous errors; 1 warning emitted