Skip to content

Commit

Permalink
Make named_asm_labels lint not trigger on unicode and trigger on form…
Browse files Browse the repository at this point in the history
…at args
  • Loading branch information
asquared31415 committed Dec 21, 2023
1 parent 767453e commit 87fed97
Show file tree
Hide file tree
Showing 3 changed files with 162 additions and 24 deletions.
62 changes: 49 additions & 13 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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;
}
}
Expand Down
21 changes: 21 additions & 0 deletions tests/ui/asm/named-asm-labels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
103 changes: 92 additions & 11 deletions tests/ui/asm/named-asm-labels.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -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)) }
| ^^^^^
Expand All @@ -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)) }
| ^^^^^
Expand All @@ -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)) }
| ^^^^^
Expand All @@ -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)) }
| ^^^^^
Expand All @@ -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");
| ^^^^^^^^
Expand All @@ -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");
| ^^^^^^^^
Expand All @@ -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

0 comments on commit 87fed97

Please sign in to comment.