Re: [musl] Hexagon DSP support

[Rich Felker <dalias@libc.org>]

On Tue, May 05, 2020 at 06:37:41PM -0500, sidneym@codeaurora.org wrote:
> >- The definitions in bits/msg.h, bits/sem.h, and bits/shm.h are not
> >  time64-compatible. In fact bits/sem.h is a regression; you had it
> >  right before and broke it.
> As you point out below the core of the problem was the missing time64
> calls.  I've updated the structures to use what I hope or the
> correct types.

I think they're still wrong and you need to do like the existing
32-bit archs, defining IPC_STAT to 0x102 and putting separate unsigned
long *_lo/*_hi members in the right places for the kernel ABI and
adding the time_t members at the ends. This is almost surely the case
if _Alignof(long long) is 8 on hexagon, since the kernel's
include/uapi/asm-generic/msgbuf.h etc. have the lo/hi pairs misaligned
(struct ipc_perm is an odd number of 32-bit words). The fact that you
had to change qemu to make this work suggests that you still have it
wrong too -- you should not have to change qemu or the kernel to make
it work if you do it right.

> >- The definition in bits/stat.h is using the wrong types (you need to
> >  use libc type names for all the members; this is a policy
> >  requirement to ensure that none inadvertently mismatch). Note that
> >  there's not even really a need to use an arch-specific header here
> >  anymore; the kernel definition comes from arch/hexagon/kstat.h and
> >  the bits/stat.h can just be copied from aarch64 or something else
> >  with a clean layout.
> 
> I used OpenRISC's stat.h.

Is there a reason? It has old __st_*tim32 members but you don't need
them since there's no old-musl-ABI involved.

> src/api/ftw.c:44:1: error: switch condition has boolean value [-Werror,-Wswitch-bool]
> C(S_ISBLK(0))
> ^~~~~~~~~~~~~

You're still building libc-test with -Werror which will give bogus
results.

> src/api/sys_ipc.c:13:1: error: initializing 'uid_t *' (aka 'unsigned int *') with an expression of type 'int *' converts between pointers to integer types with different sign [-Werror,-Wpointer-sign]
> F(uid_t,uid)
> ^~~~~~~~~~~~
> src/api/sys_ipc.c:3:20: note: expanded from macro 'F'
> #define F(t,n) {t *y = &x.n;}
>                    ^   ~~~~

This is happening because struct ipc_perm's members have the wrong
types (but I think you should just delete your bits/ipc.h because the
generic one is correct and matches the kernel's ABI for hexagon).

> src/functional/ipc_msg.c:63: qid_ds.msg_qnum == 0 failed: got 16384, want 0
> src/functional/ipc_msg.c:67: (long long)qid_ds.msg_rtime == 0 failed: got 6823500725968961536, want 0
> src/functional/ipc_msg.c:69: qid_ds.msg_ctime >= t failed: got 69713, want >= 1588720066
> src/functional/ipc_msg.c:73: qid_ds.msg_qbytes > 0 failed: got 0, want > 0
> src/functional/ipc_msg.c:78: qid_ds.msg_qnum == 1 failed: got 16384, want 1
> src/functional/ipc_msg.c:79: qid_ds.msg_lspid == getpid() failed: got 0, want 240818
> src/functional/ipc_msg.c:81: msg_stime is 0 want >= 1588720066
> src/functional/ipc_msg.c:130: child exit status: 256
> FAIL src/functional/ipc_msg-static.exe [status 1]
> src/functional/ipc_msg.c:63: qid_ds.msg_qnum == 0 failed: got 16384, want 0
> src/functional/ipc_msg.c:67: (long long)qid_ds.msg_rtime == 0 failed: got 6823500725968961536, want 0
> src/functional/ipc_msg.c:69: qid_ds.msg_ctime >= t failed: got 0, want >= 1588720066
> src/functional/ipc_msg.c:73: qid_ds.msg_qbytes > 0 failed: got 0, want > 0
> src/functional/ipc_msg.c:78: qid_ds.msg_qnum == 1 failed: got 16384, want 1
> src/functional/ipc_msg.c:79: qid_ds.msg_lspid == getpid() failed: got 0, want 240786
> src/functional/ipc_msg.c:81: msg_stime is 0 want >= 1588720066
> src/functional/ipc_msg.c:130: child exit status: 256
> FAIL src/functional/ipc_msg.exe [status 1]

These indicate your struct ipc_perm/msqid_ds mismatch kernel, as
described above.

Otherwise I didn't see anything obviously wrong in tests. Having less
noise from -Werror and the above issues would make it easier to review
the report though and ensure I'm not missing anything.

Rich