On 17.11.2019 04:34, Rin Okuyama wrote:
> Hi,
>
> In order to distangle circular dependency between
> <sys/compat/sys/siginfo.h> v.s. <machine/netbsd32_machdep.h>,
> I propose
>
> (1) Move NETBSD32_INT64_ALIGN from <machine/netbsd32_machdep.h> to
> <machine/types.h>
>
> (2) Move netbsd32_{,u}int64 from <sys/compat/netbsd32/netbsd32.h> to
> <sys/types.h>
>
> See attached patch for example on amd64.
>
What do you think about duplicating the defines of netbsd32_uint64
inside sys/compat/sys/siginfo.h + adding a comment about keeping it in
sync with netbsd32.h?
I think that avoiding spaghetti dependencies is a benefit.
We already duplicated there _ptrace_state, removing circular
dependencies between sys/ptrace.h and sys/siginfo.h.
> Background is:
>
> Now, gdb for i386 does not work again on amd64 (both on -current and
> netbsd-9) with:
>
> ptrace: Invalid arguments.
>
> This is because sizeof(struct netbsd32_ptrace_siginfo) is 128+4+4=136
> on amd64 whereas sizeof(struct ptrace_siginfo) is 128+4=132 on i386;
> netbsd32_ptrace_siginfo has uint64_t members via siginfo32_t via
> __ksiginfo32 since compat/sys/siginfo.h rev 1.5:
>
> http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/compat/sys/siginfo.h#rev1.5
>
> As a result, netbsd32_ptrace_siginfo requires 4-byte tail padding on
> amd64. However, tail padding does not appear on i386, since 64-bit
> objects only need 4-byte alignment on i386.
>
> Actually, gdb/i386 becomes sane with this hack:
>
Please check it also with picotrace/truss:
http://pkgsrc.se/devel/picotrace
> ----
> Index: sys/compat/sys/siginfo.h
> ===================================================================
> RCS file: /home/netbsd/src/sys/compat/sys/siginfo.h,v
> retrieving revision 1.8
> diff -p -u -r1.8 siginfo.h
> --- sys/compat/sys/siginfo.h 30 Sep 2019 21:13:33 -0000 1.8
> +++ sys/compat/sys/siginfo.h 12 Nov 2019 11:04:58 -0000
> @@ -34,6 +34,12 @@
>
> #ifdef _KERNEL
>
> +typedef uint64_t _args_t
> +#ifdef __x86_64__
> + __attribute__((__aligned__(4)))
> +#endif
> + ;
> +
> typedef union sigval32 {
> int sival_int;
> uint32_t sival_ptr;
> @@ -73,7 +79,7 @@ struct __ksiginfo32 {
> int _sysnum;
> int _retval[2];
> int _error;
> - uint64_t _args[8]; /* SYS_MAXSYSARGS */
> + _args_t _args[8]; /* SYS_MAXSYSARGS */
> } _syscall;
>
> struct {
> ----
>
> We provide netbsd32_uint64 for this purpose:
>
> typedef uint64_t netbsd32_uint64 NETBSD32_INT64_ALIGN;
>
> and NETBSD32_INT64_ALIGN is __attribute__((__aligned__(4))) on amd64.
> However, unfortunately, NETBSD32_INT64_ALIGN is defined in
> <machine/netbsd32_machdep.h>, and <machine/netbsd32_machdep.h> requires
> <sys/compat/siginfo.h>.
>
> Thoughts? Any comments or objections?
>
> Thanks,
> rin
Attachment:
signature.asc
Description: OpenPGP digital signature