Don't lint against named labels in naked_asm!
#140871
Conversation
|
r? @davidtwco rustbot has assigned @davidtwco. Use |
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
rustbot
added
S-waiting-on-author
| // Trigger on naked fns too, even though they can't be inlined, reusing a | ||
| // label or LTO can cause labels to break |
There was a problem hiding this comment.
Why is this not a valid concern?
There was a problem hiding this comment.
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!).
|
@rustbot ready |
rustbot
added
S-waiting-on-review
|
Naked functions can be generic, in which case named labels will still cause issues. |
Naked functions are allowed to define global labels, just like `global_asm!`.
|
I've updated the PR to only allow labels in non-generic naked functions. |
| @@ -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, | |||
There was a problem hiding this comment.
How exactly do I get a DefId here? I only have a HirId since this is a LateLintPass which runs on HIR.
There was a problem hiding this comment.
Sorry, I thought that hir::InlineAsm was given a def id as a body owner.
Well, in any case, this check is not correct because it doesn't take into account enclosing items:
use std::arch::naked_asm;
trait Foo {
#[unsafe(naked)]
extern "C" fn bbb<'a>(a: &'a u32) {
naked_asm!(".Lbbb: nop; ret;")
}
}
You can get the def id of the enclosing body of this expression by using expr.hir_id.owner_id.def_id. Then use it accordingly below. Please add the snippet I shared above as a test.
| template_strs, | ||
| options, | ||
| .. | ||
| }), | ||
| .. | ||
| } = expr | ||
| { | ||
| // Non-generic naked functions are allowed to define arbitrary | ||
| // labels. | ||
| if *asm_macro == AsmMacro::NakedAsm { |
There was a problem hiding this comment.
... then with that def id, use
cx.tcx.generics_of(def_id).requires_monomorphization(self.cx)
To avoid reinventing the wheel here.
rustbot
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #143897) made this pull request unmergeable. Please resolve the merge conflicts. |
Naked functions are allowed to define global labels, just like
global_asm!.