Skip to content

Don't lint against named labels in naked_asm! #140871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+37 −32
Open

Don't lint against named labels in naked_asm! #140871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2784,7 +2784,7 @@ impl InlineAsmOperand {
}
}

#[derive(Clone, Copy, Encodable, Decodable, Debug, HashStable_Generic)]
#[derive(Clone, Copy, Encodable, Decodable, Debug, HashStable_Generic, PartialEq, Eq)]
pub enum AsmMacro {
/// The `asm!` macro
Asm,
Expand Down
16 changes: 15 additions & 1 deletion compiler/rustc_lint/src/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2874,14 +2874,28 @@ impl<'tcx> LateLintPass<'tcx> for AsmLabels {
if let hir::Expr {
kind:
hir::ExprKind::InlineAsm(hir::InlineAsm {
asm_macro: AsmMacro::Asm | AsmMacro::NakedAsm,
asm_macro: asm_macro @ (AsmMacro::Asm | AsmMacro::NakedAsm),
template_strs,
options,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grab the def id here:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How exactly do I get a DefId`` here? I only have a HirIdsince this is aLateLintPass` which runs on HIR.

..
}),
..
} = expr
{
// Non-generic naked functions are allowed to define arbitrary
// labels.
if *asm_macro == AsmMacro::NakedAsm {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... then with that def id, use

cx.tcx.generics_of(def_id).requires_monomorphization(self.cx)

To avoid reinventing the wheel here.

let mono = cx.generics.is_none_or(|generics| {
generics
.params
.iter()
.all(|param| matches!(param.kind, GenericParamKind::Lifetime { .. }))
});
if mono {
return;
}
}

// asm with `options(raw)` does not do replacement with `{` and `}`.
let raw = options.contains(InlineAsmOptions::RAW);

Expand Down
19 changes: 14 additions & 5 deletions tests/ui/asm/named-asm-labels.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,10 @@ fn main() {
}
}

// Trigger on naked fns too, even though they can't be inlined, reusing a
// label or LTO can cause labels to break
Comment on lines -174 to -175
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this not a valid concern?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the original reasoning for closing #88169 was correct (even though it did come from me). Fundamentally there's no reason why you can't define global symbols in naked functions, the issue is only with local symbols (.Lfoo) which are local to the object file (rustc doesn't guarantee that each asm block is in a separate object file).

In any case #128004 changed naked functions to use global_asm! internally so there's no reason to treat it different (this lint is already disabled for global_asm!).

// Don't trigger on naked functions.
#[unsafe(naked)]
pub extern "C" fn foo() -> i32 {
naked_asm!(".Lfoo: mov rax, {}; ret;", "nop", const 1)
//~^ ERROR avoid using named labels
}

// Make sure that non-naked attributes *do* still let the lint happen
Expand All @@ -190,7 +188,18 @@ pub extern "C" fn bar() {
pub extern "C" fn aaa() {
fn _local() {}

naked_asm!(".Laaa: nop; ret;") //~ ERROR avoid using named labels
naked_asm!(".Laaa: nop; ret;")
}

#[unsafe(naked)]
pub extern "C" fn bbb<'a>(a: &'a u32) {
naked_asm!(".Lbbb: nop; ret;")
}

#[unsafe(naked)]
pub extern "C" fn ccc<T>(a: &T) {
naked_asm!(".Lccc: nop; ret;")
//~^ ERROR avoid using named labels
}

pub fn normal() {
Expand All @@ -200,7 +209,7 @@ pub fn normal() {
pub extern "C" fn bbb() {
fn _very_local() {}

naked_asm!(".Lbbb: nop; ret;") //~ ERROR avoid using named labels
naked_asm!(".Lbbb: nop; ret;")
}

fn _local2() {}
Expand Down
32 changes: 7 additions & 25 deletions tests/ui/asm/named-asm-labels.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,16 +475,7 @@ LL | #[warn(named_asm_labels)]
| ^^^^^^^^^^^^^^^^

error: avoid using named labels in inline assembly
--> $DIR/named-asm-labels.rs:178:17
|
LL | naked_asm!(".Lfoo: mov rax, {}; ret;", "nop", const 1)
| ^^^^^
|
= 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:185:20
--> $DIR/named-asm-labels.rs:183:20
|
LL | unsafe { asm!(".Lbar: mov rax, {}; ret;", "nop", const 1, options(noreturn)) }
| ^^^^^
Expand All @@ -493,25 +484,16 @@ 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:193:17
--> $DIR/named-asm-labels.rs:201:17
|
LL | naked_asm!(".Laaa: nop; ret;")
LL | naked_asm!(".Lccc: nop; ret;")
| ^^^^^
|
= 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:203:21
|
LL | naked_asm!(".Lbbb: nop; ret;")
| ^^^^^
|
= 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:212:15
--> $DIR/named-asm-labels.rs:221:15
|
LL | asm!("closure1: nop");
| ^^^^^^^^
Expand All @@ -520,7 +502,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:216:15
--> $DIR/named-asm-labels.rs:225:15
|
LL | asm!("closure2: nop");
| ^^^^^^^^
Expand All @@ -529,13 +511,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:226:19
--> $DIR/named-asm-labels.rs:235: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 56 previous errors; 1 warning emitted
error: aborting due to 54 previous errors; 1 warning emitted

Loading