rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.24k stars 12.71k forks source link

RISC-V: Inline assembly ecall assumes return value in incorrect register #65290

Closed alistair23 closed 5 years ago

alistair23 commented 5 years ago

The following 32-bit RISC-V Rust code

pub unsafe fn allow_ptr(major: usize, minor: usize, slice: *mut u8, len: usize) -> isize {
    let res;
    asm!("li    a0, 3
          ecall"
         : "=r" (res)
         : "{x11}" (major), "{x12}" (minor), "{x13}" (slice), "{x14}" (len)
         : "memory"
         : "volatile");
    res
}

Compiles to this:

pub unsafe fn allow_ptr(major: usize, minor: usize, slice: *mut u8, len: usize) -> isize {
20431596:       7179                    addi    sp,sp,-48
20431598:       d606                    sw      ra,44(sp)
2043159a:       d422                    sw      s0,40(sp)
2043159c:       1800                    addi    s0,sp,48
2043159e:       feb42423                sw      a1,-24(s0)
204315a2:       fea42223                sw      a0,-28(s0)
204315a6:       fec42623                sw      a2,-20(s0)
204315aa:       fed42823                sw      a3,-16(s0)
    let res;
    asm!("li    a0, 3
204315ae:       feb42023                sw      a1,-32(s0)
204315b2:       85aa                    mv      a1,a0
204315b4:       fe042503                lw      a0,-32(s0)
204315b8:       fcc42e23                sw      a2,-36(s0)
204315bc:       862a                    mv      a2,a0
204315be:       fdc42703                lw      a4,-36(s0)
204315c2:       fcd42c23                sw      a3,-40(s0)
204315c6:       86ba                    mv      a3,a4
204315c8:       fd842703                lw      a4,-40(s0)
204315cc:       450d                    li      a0,3
204315ce:       00000073                ecall
204315d2:       feb42a23                sw      a1,-12(s0)
         : "=r" (res)
         : "{x11}" (major), "{x12}" (minor), "{x13}" (slice), "{x14}" (len)
         : "memory"
         : "volatile");
    res
}
204315d6:       852e                    mv      a0,a1
204315d8:       5422                    lw      s0,40(sp)
204315da:       50b2                    lw      ra,44(sp)
204315dc:       6145                    addi    sp,sp,48
204315de:       8082                    ret

Using:

rustc 1.40.0-nightly (f3c9cece7 2019-10-07)

As you can see, the compiled code assumes that the return value of the ecall is saved into a1 (x11). For the RISC-V ABI this is incorrect as the first return value should be saved into a0 see here.

Changing the output line to:

: "={x10}" (res)

fixes the problem.

nagisa commented 5 years ago

The constraint "=r" allows the compiler to pick any register. This is true regardless of the architecture. Register selection never looks at the assembly code to make decisions on what register should be picked; rather, the register allocation algorithm considers the surrounding code and the the constraints only.

As thus, closing as invalid, but if you feel I’m wrong or that I overlooked something, complain, and I will reopen.

alistair23 commented 5 years ago

Looking at the Linux kernel C code you might be right. They get away with using the +r as they have used register variables in C.

#define SBI_CALL(which, arg0, arg1, arg2, arg3) ({      \
    register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0);   \
    register uintptr_t a1 asm ("a1") = (uintptr_t)(arg1);   \
    register uintptr_t a2 asm ("a2") = (uintptr_t)(arg2);   \
    register uintptr_t a3 asm ("a3") = (uintptr_t)(arg3);   \
    register uintptr_t a7 asm ("a7") = (uintptr_t)(which);  \
    asm volatile ("ecall"                   \
              : "+r" (a0)               \
              : "r" (a1), "r" (a2), "r" (a3), "r" (a7)  \
              : "memory");              \
    a0;                         \
})