Skip to content
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.

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

Incorrect reporting of asm labels in Nightly #128659

Open
corigan01 opened this issue Aug 4, 2024 · 8 comments
Open

Incorrect reporting of asm labels in Nightly #128659

corigan01 opened this issue Aug 4, 2024 · 8 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. D-imprecise-spans Diagnostics: spans don't point to exactly the erroneous code L-binary_asm_labels Lint: LLVM parses labels like 0, 1, 10, 11, etc. oddly T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@corigan01
Copy link

corigan01 commented Aug 4, 2024

Updating from 1.80.x-nightly to 1.82.x-nightly introduced a new compiler error in one of my inline assembly blocks falsely reporting that an int 0x13 shouldn't include numerical labels.

I tried this code:

    pub fn read(&self, disk: u16) {
        let packet_address = self as *const Self as u16;
        let status: u16;

        unsafe {
            core::arch::asm!("
                push si
                mov si, {packet:x}
                mov ax, 0x4200
                int 0x13
                jc 1f
                mov {status:x}, 0
                jmp 2f
                1:
                mov {status:x}, 1
                2:
                pop si
            ",
                in("dx") disk,
                packet = in(reg) packet_address,
                status = out(reg) status,
            );
        };

        // If the interrupt failed, we want to abort and tell the user
        if status == 1 {
            fail(b'D');
        }
    }

Compiler output:

error: avoid using labels containing only the digits `0` and `1` in inline assembly
  --> bootloader/stage-bootsector/src/disk.rs:38:23
   |
38 |                   int 0x13
   |  _______________________^
39 | |                 jc 1f
40 | |                 mov {status:x}, 0
   | |___________________________^ use a different label that doesn't start with `0` or `1`
   |
   = help: start numbering with `2` instead
   = note: an LLVM bug makes these labels ambiguous with a binary literal number on x86
   = note: see <https://github.com/llvm/llvm-project/issues/99547> for more information
   = note: `#[deny(binary_asm_labels)]` on by default

Meta

rustc --version --verbose:

rustc 1.82.0-nightly (64ebd39da 2024-08-03)

No backtrace was provided even when setting RUST_BACKTRACE=1

@corigan01 corigan01 added the C-bug Category: This is a bug. label Aug 4, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 4, 2024
@corigan01
Copy link
Author

Changing the labels in the assembly fixes the problem. I believe the problem is with the compiler indicating the int and mov are the cause of the issue instead of the jc and 1: labels.

push si
mov si, {packet:x}
mov ax, 0x4200
int 0x13                  ;<-- not error, but reported
jc 1f                     ;<-- actual error line
mov {status:x}, 0         ;<-- not error, but reported
jmp 2f
1:                        ;<-- actual error line
mov {status:x}, 1
2:
pop si

Maybe misalignment in asm macro's error reporting?

@workingjubilee
Copy link
Member

Thanks for reporting this! Yes, this is a recent lint since #126922 and seems to have a few bugs, which is expected since we uhhh don't usually lint on assembly code, but rather on Rust code. ^^;

cc @asquared31415

@workingjubilee workingjubilee added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. L-binary_asm_labels Lint: LLVM parses labels like 0, 1, 10, 11, etc. oddly labels Aug 4, 2024
@workingjubilee
Copy link
Member

also cc @tgross35 since you've done some work on this lint too.

@workingjubilee workingjubilee added A-diagnostics Area: Messages for errors, warnings, and lints D-imprecise-spans Diagnostics: spans don't point to exactly the erroneous code and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 4, 2024
@corigan01
Copy link
Author

Thanks for the response! I didn't catch that this is the same issue as #126922. I figured it had to do with the weird target and some weird spans in the asm macro being confused.

If it means anything, this is a really non-standard target as well: i368-unknown-code16

@tgross35
Copy link
Contributor

tgross35 commented Aug 4, 2024

Ah interesting. Seems like this needs to check more than just the first character

} else if matches!(start, '0' | '1') {

@workingjubilee
Copy link
Member

i368-unknown-code16

That shouldn't matter as it is still an x86 target, right? And thus still uses x86 assembly which will thus be parsed by LLVM's x86 assembly parser.

@asquared31415
Copy link
Contributor

@rustbot claim

The code that finds the span for the label assumes that the first occurrence of the label within the string literal is the start of the label. In this case it correctly finds the 1: label, but then the first 1 in the string is in the int 0x13, and it collects the span up to the next :. I sort of knew when writing this code for the original asm label lint that it was a little sketch, but could not find a reproducer at the time.

@asquared31415
Copy link
Contributor

The problem boils down to "turn a range of offsets in a string literal into a span in the source code", but it's not as simple as some offset math because escapes exist, so when processing, for example "\u{0031}", the label checking code correctly sees a "1", but the offsets of that do not 1:1 map to anything in the source code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. D-imprecise-spans Diagnostics: spans don't point to exactly the erroneous code L-binary_asm_labels Lint: LLVM parses labels like 0, 1, 10, 11, etc. oddly T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants