Date: Mon, 7 Sep 2020 14:06:36 -0400 From: Rich Felker <dalias@...c.org> To: musl@...ts.openwall.com Subject: Re: riscv32 v2 On Mon, Sep 07, 2020 at 06:47:00AM -0400, Stefan O'Rear wrote: > On Fri, Sep 4, 2020, at 1:48 AM, Stefan O'Rear wrote: > > FAIL src/api/main.exe [status 1] > > FAIL src/functional/fcntl-static.exe [status 1] > > FAIL src/functional/fcntl.exe [status 1] > > FAIL src/functional/ipc_msg-static.exe [status 1] > > FAIL src/functional/ipc_msg.exe [status 1] > > FAIL src/functional/ipc_sem-static.exe [status 1] > > FAIL src/functional/ipc_sem.exe [status 1] > > FAIL src/functional/ipc_shm-static.exe [status 1] > > FAIL src/functional/ipc_shm.exe [status 1] > > FAIL src/functional/strptime-static.exe [status 1] > > FAIL src/functional/strptime.exe [status 1] > > FAIL src/math/fma.exe [status 1] > > FAIL src/math/fmaf.exe [status 1] > > FAIL src/math/powf.exe [status 1] > > FAIL src/regression/malloc-brk-fail-static.exe [status 1] > > FAIL src/regression/malloc-brk-fail.exe [status 1] > > FAIL src/regression/pthread_atfork-errno-clobber-static.exe [status 1] > > FAIL src/regression/pthread_atfork-errno-clobber.exe [status 1] > > > > The fcntl and sysvipc errors do not correspond to any error in x86_64 > > and potentially require investigation, although they could be kernel > > configuration issues. x86_64 has a different but overlapping set of > > math errors; qemu is known to not give bit-exact results for RISC-V > > floating point. The malloc, pthread, and src/api/main.exe failures > > match failures on x86_64. > > Attached patch reaches test failure parity between riscv32 and riscv64 > and will be included in v3. > > * gdb HEAD wants ELF_NFPREG, so I set it in bits/user.h to the value > gdb needs. (glibc does #define ELF_NFPREG NFPREG and expects gdb > to define NFPREG. I don't get this.) Ick. Indeed I think this is wrong/probably an oversight in glibc, and the way you've done it sounds better. > * Restore accidentally removed errno setting in waitpid, fixes a gdb > assertion failure. OK. As an aside (I haven't gotten to sending review for this yet, sorry) I think I'd prefer to name the function __sys_wait4 or similar to make it more clear that it's analogous to __sys_open etc. (a function or macro emulating the syscall) and not a namespace-safe version of wait4. (musl's having __wait be a futex wait makes this even more confusing, btw) Perhaps also leave the int cp argument to the function but make separate __sys_wait4 and __sys_wait4_cp macros to call it via so that there's not a mysterious boolean argument that doesn't correspond to an actual syscall argument. (This would also be parallel with how __sys_open is done.) > * Zero IPC_64 because the kernel only recognizes one set of IPC commands. OK. > * Copy the IPC_TIME64 bits from arch/arm/bits to trigger the musl code > for fixing time64 IPC_STAT results. I'm not super happy with this, > maybe there should be a new mechanism in musl for fixing IPC_STAT for > unconditionally-time64 architectures. If the riscv32 IPC syscalls don't actually provide in-place time64 but require translation, I think it's fairly appropriate as-is. >From the definitions in your patch, it looks like all the time fields are fixed-word-order (little endian) and possibly not aligned, so it seems like they can't be used in-place. Is this correct? > * riscv32 _does_ provide both F_GETLK and F_GETLK32; make sure we use > the right one. IIRC someone already suggested using the generic bits/fcntl.h, which would have solved this. I also have unpushed changed that let 64-bit archs share the generic bits/fcntl.h too, via #if on __LONG_MAX. Rich
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.