Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 18 Oct 2018 14:52:53 -0700
From: Michael Forney <mforney@...rney.org>
To: musl@...ts.openwall.com
Subject: Re: riscv port for review

On 2018-10-11, Michael Forney <mforney@...rney.org> wrote:
> On 2018-09-27, Rich Felker <dalias@...c.org> wrote:
>> Pulled from here:
>> https://github.com/riscv/riscv-musl/commit/6a4f4a9c774608add4b02f95322518bd2f5f51ee
>>
>> Attached for review.
>
> I noticed that some fcntl.h constants are incorrect (O_DIRECTORY,
> O_NOFOLLOW, O_DIRECT, O_LARGEFILE, and O_TMPFILE). Linux doesn't seem
> to have a riscv-specific fcntl.h, so I think they just come from
> asm-generic.

While playing around with a riscv32-linux-musl toolchain and tinyemu,
I found a few more issues:

- riscv linux has no renameat syscall. I think __NR_renameat needs to
be removed from arch/riscv*/bits/syscall.h.in, and musl's rename and
renameat need to fallback to SYS_renameat2 if SYS_renameat is not
defined.

- arch/riscv32/bits/syscall.h.in defines syscall names as if it were
64-bit. I think the following changes are necessary
    __NR_fcntl -> __NR_fcntl64
    __NR_statfs -> __NR_statfs64
    __NR_fstatfs -> __NR_fstatfs64
    __NR_truncate -> __NR_truncate64
    __NR_ftruncate -> __NR_ftruncate64
    __NR_lseek -> __NR__llseek (and __NR_llseek?)
    __NR_fstatat -> __NR_fstatat64
    __NR_fstat -> __NR_fstat64
    __NR_mmap -> __NR_mmap2
    __NR_fadvise64 -> __NR_fadvise64_64

- Since riscv32 uses fcntl64, and musl's struct flock corresponds to
struct flock64 on 32-bit, F_GETLK, F_SETLK, and F_SETLKW should be
defined to the corresponding *64 values (12, 13, 14). This matches
arch/generic/bits/fcntl.h, so I think arch/riscv32/bits/fcntl.h should
just be completely removed as Rich suggested.
arch/riscv64/bits/fcntl.h needs to stay to define the non *64 values
(5, 6, 7).

- For detecting soft float, configure compares $ARCH to riscv and
riscv64, but ARCH is set to riscv32 or riscv64 above. Also ARCH is set
to riscv32 when $target is riscv* but not riscv64*. Should this be
riscv32*?

- There are several instances of preprocessor checks
__riscv_soft_float, but this does not seem to be defined by gcc.
Perhaps this is superseded by __riscv_flen or __riscv_float_abi_soft?

- The functions in src/math/riscv* don't fall back to the C
implementation for soft float.

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.