Change binary_asm_labels to only fire on x86 and x86_64

[tgross35]

In #126922, the binary_asm_labels lint was added which flags labels such as 0: and 1:. Before that change, LLVM was giving a confusing error on x86/x86_64 because of an incorrect interpretation.

However, targets other than x86 and x86_64 never had the error message and have not been a problem. This means that the lint was causing code that previously worked to start failing (e.g. compiler_builtins), rather than only providing a more clear messages where there has always been an error.

Adjust the lint to only fire on x86 and x86_64 assembly to avoid this regression.

Also update the help message.

Fixes: #127821

[rustbot]

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[tgross35]

Seems to work now, tested locally on both x86 and aarch64.

r? @Urgau

[estebank]

Ideally, the lint would not gate on the platform being compiled, but rather gated on whether the code could be run from x86. Namely, if the asm is behind a cfg flag that limits the code to anything other than x86. That being said, I believe that for now this is a reasonable compromise.

@bors r+

[bors]

📌 Commit 8410348 has been approved by estebank

It is now in the queue for this repository.

[tgross35]

Ideally, the lint would not gate on the platform being compiled, but rather gated on whether the code could be run from x86. Namely, if the asm is behind a cfg flag that limits the code to anything other than x86.

Do you mean that you would expect the following to lint even if building on aarch64 or other targets?

#[cfg(target_arch = "x86")]
fn foo() {
    unsafe { asm!("0: jmp 0b") };
}

Does that kind of machinery exist? I thought if it was behind a cfg then it doesn't even know the asm is there.

Thanks for the review

[estebank]

Does that kind of machinery exist? I thought if it was behind a cfg then it doesn't even know the asm is there.

We have a very basic collection of items that got cfg'd out for the purposes of telling users that an item that wasn't found is behind a feature flag. We collect those during expansion, which would make my request harder to accomplish, as you would need to implement the lint to be able to work with an ast::ExprKind::InlineAsm (as opposed to the current hir::ExprKind::InlineAsm) but there's precedent.

[tgross35]

For this specific lint, it seems like effort is probably better spent fixing LLVM than attempting to improve the ergonomics much more. But being able to leverage that for other inline asm sounds amazing, since it is so platform dependent and easy to make mistakes, we should be able to at least perform Rust's validation on it.

I'll update #127938 to reflect that if it sounds accurate.

[tgross35]

@bors r- failed rollup #127941 (comment)

[tgross35]

I didn't update the docs so it tried to run the assembly example on all platforms. Added a commit to fix this, looking at other tests I guess the best thing we can do is ignore the doctest and update the output manually. lint-docs tests pass locally on my aarch64 machine now.

I'll let somebody double check the third commit changes (@estebank), @rustbot review

[Urgau]

Changes looks good.

@bors r=estebank,Urgau

[bors]

📌 Commit 5686720 has been approved by estebank,Urgau

It is now in the queue for this repository.