Skip to content

Commit

Permalink
Rollup merge of #119195 - asquared31415:named_asm_labels_fix, r=Amanieu
Browse files Browse the repository at this point in the history 
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```
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Jan 4, 2024
2 parents d134a4f + 87fed97 commit 13840b3
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
View file
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
View file
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
View file
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 13840b3

Please sign in to comment.