[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200506005929.GG21576@brightrain.aerifal.cx>
Date: Tue, 5 May 2020 20:59:29 -0400
From: Rich Felker <dalias@...c.org>
To: sidneym@...eaurora.org
Cc: musl@...ts.openwall.com
Subject: Re: Hexagon DSP support
On Tue, May 05, 2020 at 06:37:41PM -0500, sidneym@...eaurora.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
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.
