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

u128 division not working on x86_64-unknown-uefi #86494

Closed
nicholasbishop opened this issue Jun 20, 2021 · 4 comments
Closed

u128 division not working on x86_64-unknown-uefi #86494

nicholasbishop opened this issue Jun 20, 2021 · 4 comments
Labels
C-bug Category: This is a bug.

Comments

@nicholasbishop
Copy link
Contributor

I've found that u128 division is not working on the x86_64-unknown-uefi target. Depending on the exact code I've seen it give either incorrect results or fail with an invalid opcode error.

Fairly minimal example:

#![no_std]
#![no_main]
#![feature(abi_efiapi)]

use log::info;
use uefi::prelude::*;

#[inline(never)]
fn hide_u128(n: u128) -> u128 {
    n
}

#[entry]
fn efi_main(_image: Handle, st: SystemTable<Boot>) -> Status {
    uefi_services::init(&st).unwrap().unwrap();
    let a = hide_u128(2);
    let b = hide_u128(1);
    info!("a/b={}", a / b);
    todo!();
}

The easiest way to actually run this is with QEMU; I've set up a example in this repo: https://github.com/nicholasbishop/uefi-div-bug. The run.py script will build it and run it under a QEMU UEFI environment.

With this particular example I get this error:

!!!! X64 Exception Type - 06(#UD - Invalid Opcode)  CPU Apic ID - 00000000 !!!!

With less minimal examples I've also seen it not fail with an invalid opcode, but instead just give a bogus result. Other u128 operations I tried (add, sub, mul) work fine.

My not-very-informed guess is that there's a bug in the __udivti3 implementation in compiler_builtins. I was curious if perhaps rust-lang/compiler-builtins#332 was related, but I couldn't figure out how to alter the version of compiler-builtins I tested against.

$ rustc +nightly --version
rustc 1.55.0-nightly (150fad30e 2021-06-19)
@nicholasbishop nicholasbishop added the C-bug Category: This is a bug. label Jun 20, 2021
@est31
Copy link
Member

est31 commented Jun 20, 2021

I was curious if perhaps rust-lang/compiler-builtins#332 was related, but I couldn't figure out how to alter the version of compiler-builtins I tested against.

The PR you linked should not be present in Rust compilers before 1.49.0, like 1.48.0. You can download it using rustup with the command rustup toolchain add 1.48.0. After that you can add +1.48.0 to rustc or cargo invocations to use that specific version. Edit: you'll also have to add RUSTC_BOOTSTRAP=1 to enable nightly features.

The uefi dependency as well as others might not accept that compiler version because it's too old. You might have to hardcode older versions.

@nagisa
Copy link
Member

nagisa commented Jun 21, 2021

This needs a disassembly at the point UD trap was fired. I suspect CPU features (SSE?) being used that are not available or initialized by firmware by default.

@AaronKutch
Copy link
Contributor

I'm trying to repro myself and am getting

PS C:\Users\aaron\Documents\github\uefi-div-bug> python3 ./run.py
rustc +nightly --version
rustc 1.61.0-nightly (76d770ac2 2022-04-02)
cargo +nightly build
    Finished dev [unoptimized + debuginfo] target(s) in 0.07s
cp target\x86_64-unknown-uefi\debug\uefi-div-bug.efi efi_partition\EFI/BOOT/BOOTX64.EFI
Traceback (most recent call last):
  File "C:\Users\aaron\Documents\github\uefi-div-bug\run.py", line 40, in <module>
    main()
  File "C:\Users\aaron\Documents\github\uefi-div-bug\run.py", line 25, in main
    run('cp', efi_app, os.path.join(efi_partition, 'EFI/BOOT/BOOTX64.EFI'))
  File "C:\Users\aaron\Documents\github\uefi-div-bug\run.py", line 10, in run
    subprocess.run(cmd, check=True)
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.1264.0_x64__qbz5n2kfra8p0\lib\subprocess.py", line 501, in run
    with Popen(*popenargs, **kwargs) as process:
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.1264.0_x64__qbz5n2kfra8p0\lib\subprocess.py", line 966, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.1264.0_x64__qbz5n2kfra8p0\lib\subprocess.py", line 1435, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
FileNotFoundError: [WinError 2] The system cannot find the file specified

My best guess is that this function in compiler-builtins is causing the problem:

unsafe fn u128_by_u64_div_rem(duo: u128, div: u64) -> (u64, u64) {
    let duo_lo = duo as u64;
    let duo_hi = (duo >> 64) as u64;
    let quo: u64;
    let rem: u64;
    unsafe {
        // divides the combined registers rdx:rax (`duo` is split into two 64 bit parts to do this)
        // by `div`. The quotient is stored in rax and the remainder in rdx.
        // FIXME: Use the Intel syntax once we drop LLVM 9 support on rust-lang/rust.
        core::arch::asm!(
            "div {0}",
            in(reg) div,
            inlateout("rax") duo_lo => quo,
            inlateout("rdx") duo_hi => rem,
            options(att_syntax, pure, nomem, nostack)
        );
    }
    (quo, rem)
}

I read that you shouldn't use floating point operations in UEFI, and I know that div is a weird instruction that can produce floating point exceptions. If this is the cause, then the fix is to make an exception in this line for uefi (I'm not sure what configuration option allows differentiating uefi from normal) and in this line so that impl_asymmetric is disabled and impl_trifecta is enabled. There is also 32-bit UEFI, so the same should probably be done for the 64-bit versions in that file as well

nicholasbishop added a commit to nicholasbishop/compiler-builtins that referenced this issue Jul 10, 2022
The `x86_64-unknown-uefi` target is Windows-like [1], and requires the
same altered ABI for some 128-bit integer intrinsics.

See also rust-lang/rust#86494.

[1]: https://github.com/rust-lang/rust/blob/master/compiler/rustc_target/src/spec/x86_64_unknown_uefi.rs
Ayush1325 pushed a commit to Ayush1325/compiler-builtins that referenced this issue Jul 25, 2022
The `x86_64-unknown-uefi` target is Windows-like [1], and requires the
same altered ABI for some 128-bit integer intrinsics.

See also rust-lang/rust#86494.

[1]: https://github.com/rust-lang/rust/blob/master/compiler/rustc_target/src/spec/x86_64_unknown_uefi.rs
nicholasbishop added a commit to nicholasbishop/compiler-builtins that referenced this issue Jul 28, 2022
The `x86_64-unknown-uefi` target is Windows-like [1], and requires the
same altered ABI for some 128-bit integer intrinsics.

See also rust-lang/rust#86494.

[1]: https://github.com/rust-lang/rust/blob/master/compiler/rustc_target/src/spec/x86_64_unknown_uefi.rs
Ayush1325 pushed a commit to Ayush1325/compiler-builtins that referenced this issue Jul 31, 2022
The `x86_64-unknown-uefi` target is Windows-like [1], and requires the
same altered ABI for some 128-bit integer intrinsics.

See also rust-lang/rust#86494.

[1]: https://github.com/rust-lang/rust/blob/master/compiler/rustc_target/src/spec/x86_64_unknown_uefi.rs
@nicholasbishop
Copy link
Contributor Author

This turned out to be an ABI problem, fixed in compiler_builtins: rust-lang/compiler-builtins#475
The version of compiler_builtins used by rust was updated to pull that fix in: #100218

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

4 participants