Skip to content

Commit d9123d2

Browse files
authored
Unrolled build for rust-lang#126922
Rollup merge of rust-lang#126922 - asquared31415:asm_binary_label, r=estebank add lint for inline asm labels that look like binary fixes rust-lang#94426 Due to a bug/feature in LLVM, labels composed of only the digits `0` and `1` can sometimes be confused with binary literals, even if a binary literal would not be valid in that position. This PR adds detection for such labels and also as a drive-by change, adds a note to cases such as `asm!(include_str!("file"))` that the label that it found came from an expansion of a macro, it wasn't found in the source code. I expect this PR to upset some people that were using labels `0:` or `1:` without issue because they never hit the case where LLVM got it wrong, but adding a heuristic to the lint to prevent this is not feasible - it would involve writing a whole assembly parser for every target that we have assembly support for. [zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/238009-t-compiler.2Fmeetings/topic/.5Bweekly.5D.202024-06-20/near/445870628) r? ``@estebank``
2 parents c6727fc + 87856c4 commit d9123d2

File tree

8 files changed

+397
-91
lines changed

8 files changed

+397
-91
lines changed

Diff for: compiler/rustc_lint/messages.ftl

+13-4
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,6 @@ lint_builtin_allow_internal_unsafe =
5252
lint_builtin_anonymous_params = anonymous parameters are deprecated and will be removed in the next edition
5353
.suggestion = try naming the parameter or explicitly ignoring it
5454
55-
lint_builtin_asm_labels = avoid using named labels in inline assembly
56-
.help = only local labels of the form `<number>:` should be used in inline asm
57-
.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
58-
5955
lint_builtin_clashing_extern_diff_name = `{$this}` redeclares `{$orig}` with a different signature
6056
.previous_decl_label = `{$orig}` previously declared here
6157
.mismatch_label = this signature doesn't match the previous declaration
@@ -403,6 +399,19 @@ lint_incomplete_include =
403399
404400
lint_inner_macro_attribute_unstable = inner macro attributes are unstable
405401
402+
lint_invalid_asm_label_binary = avoid using labels containing only the digits `0` and `1` in inline assembly
403+
.label = use a different label that doesn't start with `0` or `1`
404+
.note = an LLVM bug makes these labels ambiguous with a binary literal number
405+
.note = see <https://bugs.llvm.org/show_bug.cgi?id=36144> for more information
406+
407+
lint_invalid_asm_label_format_arg = avoid using named labels in inline assembly
408+
.help = only local labels of the form `<number>:` should be used in inline asm
409+
.note1 = format arguments may expand to a non-numeric value
410+
.note2 = see the asm section of Rust By Example <https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels> for more information
411+
lint_invalid_asm_label_named = avoid using named labels in inline assembly
412+
.help = only local labels of the form `<number>:` should be used in inline asm
413+
.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
414+
lint_invalid_asm_label_no_span = the label may be declared in the expansion of a macro
406415
lint_invalid_crate_type_value = invalid `crate_type` value
407416
.suggestion = did you mean
408417

Diff for: compiler/rustc_lint/src/builtin.rs

+102-29
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,13 @@ use crate::{
3030
BuiltinExplicitOutlivesSuggestion, BuiltinFeatureIssueNote, BuiltinIncompleteFeatures,
3131
BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures, BuiltinKeywordIdents,
3232
BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc,
33-
BuiltinMutablesTransmutes, BuiltinNamedAsmLabel, BuiltinNoMangleGeneric,
34-
BuiltinNonShorthandFieldPatterns, BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds,
35-
BuiltinTypeAliasGenericBounds, BuiltinTypeAliasGenericBoundsSuggestion,
36-
BuiltinTypeAliasWhereClause, BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit,
33+
BuiltinMutablesTransmutes, BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns,
34+
BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, BuiltinTypeAliasGenericBounds,
35+
BuiltinTypeAliasGenericBoundsSuggestion, BuiltinTypeAliasWhereClause,
36+
BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit,
3737
BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub, BuiltinUnsafe,
3838
BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
39-
BuiltinWhileTrue, SuggestChangingAssocTypes,
39+
BuiltinWhileTrue, InvalidAsmLabel, SuggestChangingAssocTypes,
4040
},
4141
EarlyContext, EarlyLintPass, LateContext, LateLintPass, Level, LintContext,
4242
};
@@ -45,7 +45,7 @@ use rustc_ast::tokenstream::{TokenStream, TokenTree};
4545
use rustc_ast::visit::{FnCtxt, FnKind};
4646
use rustc_ast::{self as ast, *};
4747
use rustc_ast_pretty::pprust::{self, expr_to_string};
48-
use rustc_errors::{Applicability, LintDiagnostic, MultiSpan};
48+
use rustc_errors::{Applicability, LintDiagnostic};
4949
use rustc_feature::{deprecated_attributes, AttributeGate, BuiltinAttribute, GateIssue, Stability};
5050
use rustc_hir as hir;
5151
use rustc_hir::def::{DefKind, Res};
@@ -69,7 +69,6 @@ use rustc_target::abi::Abi;
6969
use rustc_trait_selection::infer::{InferCtxtExt, TyCtxtInferExt};
7070
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;
7171
use rustc_trait_selection::traits::{self, misc::type_allowed_to_implement_copy};
72-
use tracing::debug;
7372

7473
use crate::nonstandard_style::{method_context, MethodLateContext};
7574

@@ -2728,10 +2727,52 @@ declare_lint! {
27282727
"named labels in inline assembly",
27292728
}
27302729

2731-
declare_lint_pass!(NamedAsmLabels => [NAMED_ASM_LABELS]);
2730+
declare_lint! {
2731+
/// The `binary_asm_labels` lint detects the use of numeric labels containing only binary
2732+
/// digits in the inline `asm!` macro.
2733+
///
2734+
/// ### Example
2735+
///
2736+
/// ```rust,compile_fail
2737+
/// # #![feature(asm_experimental_arch)]
2738+
/// use std::arch::asm;
2739+
///
2740+
/// fn main() {
2741+
/// unsafe {
2742+
/// asm!("0: jmp 0b");
2743+
/// }
2744+
/// }
2745+
/// ```
2746+
///
2747+
/// {{produces}}
2748+
///
2749+
/// ### Explanation
2750+
///
2751+
/// A [LLVM bug] causes this code to fail to compile because it interprets the `0b` as a binary
2752+
/// literal instead of a reference to the previous local label `0`. Note that even though the
2753+
/// bug is marked as fixed, it only fixes a specific usage of intel syntax within standalone
2754+
/// files, not inline assembly. To work around this bug, don't use labels that could be
2755+
/// confused with a binary literal.
2756+
///
2757+
/// See the explanation in [Rust By Example] for more details.
2758+
///
2759+
/// [LLVM bug]: https://bugs.llvm.org/show_bug.cgi?id=36144
2760+
/// [Rust By Example]: https://doc.rust-lang.org/nightly/rust-by-example/unsafe/asm.html#labels
2761+
pub BINARY_ASM_LABELS,
2762+
Deny,
2763+
"labels in inline assembly containing only 0 or 1 digits",
2764+
}
27322765

2733-
impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
2734-
#[allow(rustc::diagnostic_outside_of_impl)]
2766+
declare_lint_pass!(AsmLabels => [NAMED_ASM_LABELS, BINARY_ASM_LABELS]);
2767+
2768+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
2769+
enum AsmLabelKind {
2770+
Named,
2771+
FormatArg,
2772+
Binary,
2773+
}
2774+
2775+
impl<'tcx> LateLintPass<'tcx> for AsmLabels {
27352776
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
27362777
if let hir::Expr {
27372778
kind: hir::ExprKind::InlineAsm(hir::InlineAsm { template_strs, options, .. }),
@@ -2759,7 +2800,8 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
27592800
None
27602801
};
27612802

2762-
let mut found_labels = Vec::new();
2803+
// diagnostics are emitted per-template, so this is created here as opposed to the outer loop
2804+
let mut spans = Vec::new();
27632805

27642806
// A semicolon might not actually be specified as a separator for all targets, but
27652807
// it seems like LLVM accepts it always.
@@ -2782,16 +2824,21 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
27822824

27832825
// Whether a { bracket has been seen and its } hasn't been found yet.
27842826
let mut in_bracket = false;
2827+
let mut label_kind = AsmLabelKind::Named;
27852828

2786-
// A label starts with an ASCII alphabetic character or . or _
27872829
// A label can also start with a format arg, if it's not a raw asm block.
27882830
if !raw && start == '{' {
27892831
in_bracket = true;
2832+
label_kind = AsmLabelKind::FormatArg;
2833+
} else if matches!(start, '0' | '1') {
2834+
// Binary labels have only the characters `0` or `1`.
2835+
label_kind = AsmLabelKind::Binary;
27902836
} else if !(start.is_ascii_alphabetic() || matches!(start, '.' | '_')) {
2837+
// Named labels start with ASCII letters, `.` or `_`.
2838+
// anything else is not a label
27912839
break 'label_loop;
27922840
}
27932841

2794-
// Labels continue with ASCII alphanumeric characters, _, or $
27952842
for c in chars {
27962843
// Inside a template format arg, any character is permitted for the
27972844
// puproses of label detection because we assume that it can be
@@ -2812,34 +2859,60 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
28122859
} else if !raw && c == '{' {
28132860
// Start of a format arg.
28142861
in_bracket = true;
2862+
label_kind = AsmLabelKind::FormatArg;
28152863
} else {
2816-
if !(c.is_ascii_alphanumeric() || matches!(c, '_' | '$')) {
2864+
let can_continue = match label_kind {
2865+
// Format arg labels are considered to be named labels for the purposes
2866+
// of continuing outside of their {} pair.
2867+
AsmLabelKind::Named | AsmLabelKind::FormatArg => {
2868+
c.is_ascii_alphanumeric() || matches!(c, '_' | '$')
2869+
}
2870+
AsmLabelKind::Binary => matches!(c, '0' | '1'),
2871+
};
2872+
2873+
if !can_continue {
28172874
// The potential label had an invalid character inside it, it
28182875
// cannot be a label.
28192876
break 'label_loop;
28202877
}
28212878
}
28222879
}
28232880

2824-
// If all characters passed the label checks, this is likely a label.
2825-
found_labels.push(possible_label);
2881+
// If all characters passed the label checks, this is a label.
2882+
spans.push((find_label_span(possible_label), label_kind));
28262883
start_idx = idx + 1;
28272884
}
28282885
}
28292886

2830-
debug!("NamedAsmLabels::check_expr(): found_labels: {:#?}", &found_labels);
2831-
2832-
if found_labels.len() > 0 {
2833-
let spans = found_labels
2834-
.into_iter()
2835-
.filter_map(|label| find_label_span(label))
2836-
.collect::<Vec<Span>>();
2837-
// If there were labels but we couldn't find a span, combine the warnings and
2838-
// use the template span.
2839-
let target_spans: MultiSpan =
2840-
if spans.len() > 0 { spans.into() } else { (*template_span).into() };
2841-
2842-
cx.emit_span_lint(NAMED_ASM_LABELS, target_spans, BuiltinNamedAsmLabel);
2887+
for (span, label_kind) in spans {
2888+
let missing_precise_span = span.is_none();
2889+
let span = span.unwrap_or(*template_span);
2890+
match label_kind {
2891+
AsmLabelKind::Named => {
2892+
cx.emit_span_lint(
2893+
NAMED_ASM_LABELS,
2894+
span,
2895+
InvalidAsmLabel::Named { missing_precise_span },
2896+
);
2897+
}
2898+
AsmLabelKind::FormatArg => {
2899+
cx.emit_span_lint(
2900+
NAMED_ASM_LABELS,
2901+
span,
2902+
InvalidAsmLabel::FormatArg { missing_precise_span },
2903+
);
2904+
}
2905+
AsmLabelKind::Binary => {
2906+
// the binary asm issue only occurs when using intel syntax
2907+
if !options.contains(InlineAsmOptions::ATT_SYNTAX) {
2908+
cx.emit_span_lint(
2909+
BINARY_ASM_LABELS,
2910+
span,
2911+
InvalidAsmLabel::Binary { missing_precise_span, span },
2912+
)
2913+
}
2914+
}
2915+
};
28432916
}
28442917
}
28452918
}

Diff for: compiler/rustc_lint/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ late_lint_methods!(
225225
NoopMethodCall: NoopMethodCall,
226226
EnumIntrinsicsNonEnums: EnumIntrinsicsNonEnums,
227227
InvalidAtomicOrdering: InvalidAtomicOrdering,
228-
NamedAsmLabels: NamedAsmLabels,
228+
AsmLabels: AsmLabels,
229229
OpaqueHiddenInferredBound: OpaqueHiddenInferredBound,
230230
MultipleSupertraitUpcastable: MultipleSupertraitUpcastable,
231231
MapUnitFn: MapUnitFn,

Diff for: compiler/rustc_lint/src/lints.rs

+26-4
Original file line numberDiff line numberDiff line change
@@ -2047,10 +2047,32 @@ pub struct UnitBindingsDiag {
20472047
}
20482048

20492049
#[derive(LintDiagnostic)]
2050-
#[diag(lint_builtin_asm_labels)]
2051-
#[help]
2052-
#[note]
2053-
pub struct BuiltinNamedAsmLabel;
2050+
pub enum InvalidAsmLabel {
2051+
#[diag(lint_invalid_asm_label_named)]
2052+
#[help]
2053+
#[note]
2054+
Named {
2055+
#[note(lint_invalid_asm_label_no_span)]
2056+
missing_precise_span: bool,
2057+
},
2058+
#[diag(lint_invalid_asm_label_format_arg)]
2059+
#[help]
2060+
#[note(lint_note1)]
2061+
#[note(lint_note2)]
2062+
FormatArg {
2063+
#[note(lint_invalid_asm_label_no_span)]
2064+
missing_precise_span: bool,
2065+
},
2066+
#[diag(lint_invalid_asm_label_binary)]
2067+
#[note]
2068+
Binary {
2069+
#[note(lint_invalid_asm_label_no_span)]
2070+
missing_precise_span: bool,
2071+
// hack to get a label on the whole span, must match the emitted span
2072+
#[label]
2073+
span: Span,
2074+
},
2075+
}
20542076

20552077
#[derive(Subdiagnostic)]
20562078
pub enum UnexpectedCfgCargoHelp {

Diff for: tests/ui/asm/binary_asm_labels.rs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
//@ needs-asm-support
2+
//@ only-x86_64
3+
4+
// tests that labels containing only the digits 0 and 1 are rejected
5+
// uses of such labels can sometimes be interpreted as a binary literal
6+
7+
use std::arch::{asm, global_asm};
8+
9+
fn main() {
10+
unsafe {
11+
asm!("0: jmp 0b"); //~ ERROR avoid using labels containing only the digits
12+
asm!("1: jmp 1b"); //~ ERROR avoid using labels containing only the digits
13+
asm!("10: jmp 10b"); //~ ERROR avoid using labels containing only the digits
14+
asm!("01: jmp 01b"); //~ ERROR avoid using labels containing only the digits
15+
asm!("1001101: jmp 1001101b"); //~ ERROR avoid using labels containing only the digits
16+
}
17+
}

Diff for: tests/ui/asm/binary_asm_labels.stderr

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
error: avoid using labels containing only the digits `0` and `1` in inline assembly
2+
--> $DIR/binary_asm_labels.rs:11:15
3+
|
4+
LL | asm!("0: jmp 0b");
5+
| ^ use a different label that doesn't start with `0` or `1`
6+
|
7+
= note: an LLVM bug makes these labels ambiguous with a binary literal number
8+
= note: `#[deny(binary_asm_labels)]` on by default
9+
10+
error: avoid using labels containing only the digits `0` and `1` in inline assembly
11+
--> $DIR/binary_asm_labels.rs:12:15
12+
|
13+
LL | asm!("1: jmp 1b");
14+
| ^ use a different label that doesn't start with `0` or `1`
15+
|
16+
= note: an LLVM bug makes these labels ambiguous with a binary literal number
17+
18+
error: avoid using labels containing only the digits `0` and `1` in inline assembly
19+
--> $DIR/binary_asm_labels.rs:13:15
20+
|
21+
LL | asm!("10: jmp 10b");
22+
| ^^ use a different label that doesn't start with `0` or `1`
23+
|
24+
= note: an LLVM bug makes these labels ambiguous with a binary literal number
25+
26+
error: avoid using labels containing only the digits `0` and `1` in inline assembly
27+
--> $DIR/binary_asm_labels.rs:14:15
28+
|
29+
LL | asm!("01: jmp 01b");
30+
| ^^ use a different label that doesn't start with `0` or `1`
31+
|
32+
= note: an LLVM bug makes these labels ambiguous with a binary literal number
33+
34+
error: avoid using labels containing only the digits `0` and `1` in inline assembly
35+
--> $DIR/binary_asm_labels.rs:15:15
36+
|
37+
LL | asm!("1001101: jmp 1001101b");
38+
| ^^^^^^^ use a different label that doesn't start with `0` or `1`
39+
|
40+
= note: an LLVM bug makes these labels ambiguous with a binary literal number
41+
42+
error: aborting due to 5 previous errors
43+

0 commit comments

Comments
 (0)