On 18.11.2019 11:40, Rin Okuyama wrote:
> On 2019/11/18 16:52, Kamil Rytarowski wrote:
>> On 18.11.2019 05:38, Rin Okuyama wrote:
>>> Thank you for your comment!
>>>
>>> On 2019/11/17 22:42, Kamil Rytarowski wrote:
>>>> Please check it also with picotrace/truss:
>>>>
>>>> http://pkgsrc.se/devel/picotrace
>>>
>>> netbsd32_signal.c needed to catch up with kern_sig.c so that syscall
>>> information is provided with SIGTRAP TRAP_SCE/TRAP_SCX. I committed
>>> the fix and picotrace/truss works fine now on COMPAT_NETBSD32.
>>>
>>
>> Thanks! I have submitted a mail how to further improve it.
>
> Thank you for your advice! I committed it!
>
>>> On 2019/11/17 22:42, Kamil Rytarowski wrote:
>>>> 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.
>>>
>>> I don't think this is a good idea in this case. If we want to have
>>> duplicate define of netbsd32_uint64, and to avoid an "#ifdef __x86_64__"
>>> mess in <compat/sys/siginfo.h>, we need to move NETBSD32_INT64_ALIGN to
>>> <machine/somewhere.h> other than <machine/netbsd32_machdep.h>. If so,
>>> why not <machine/types.h>?
>>>
>>
>> No need to move anything out of machine/netbsd32_machdep.h. It's
>> sufficient to define netbsd32_int64 with a custom non-conflicting name
>> or protect it with #ifdef before typedefing/defining twice.
>>
>>> Also, in my proposal, spaghetti dependencies are avoided in the end;
>>> everyone depends on <sys/types.h>, and not on each other.
>>>
>>
>> However I have no strong opinions here. I would personally avoid
>> compat32 definitions in sys/types.h.
>>
>> Compat code tends to need hacks, so it is sensible imho to restrict them
>> to compat headers (I am aware that it's not always followed).
>
> I agree with you on that compat codes should not be put into sys headers
> as far as possible. However, I still do not understand what you mean.
>
> (1) NETBSD32_INT64_ALIGN == __attribute__((__aligned__(4))) is in
> <machine/netbsd32_machdep.h>, and
>
> (2) <machine/netbsd32_machdep.h> needs <compat/sys/siginfo.h>
>
> Therefore, we cannot use NETBSD32_INT64_ALIGN in <compat/sys/siginfo.h>.
> Of course, we can typedef _netbsd32_uint64 in <compat/sys/siginfo.h> like:
>
> typedef uint64_t _netbsd32_uint64
> #ifdef __x86_64__
> __attribute__((__aligned__(4)))
> #endif
> ;
>
> Do you think it is OK? I guess that I miss some elementary things...
> Sorry in advance ;-).
>
> Thanks,
> rin
I was thinking about something along these lines:
http://netbsd.org/~kamil/patch-00196-siginfo_netbsd32_compat_uint64.txt
In future some compat of i386 could use 8-byte alignment for 64-bit types.
Not build tested.
Attachment:
signature.asc
Description: OpenPGP digital signature