Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
On 23.11.2019 17:29, Christos Zoulas wrote: > In article , > Christos Zoulas wrote: >> In article <468095c0-b973-7f23-1cfa-3dc19e3b8...@gmail.com>, >> Rin Okuyama wrote: >>> On 2019/11/22 10:56, Christos Zoulas wrote: In article <679493cf-3e85-f56d-85e4-dfaf7958a...@gmail.com>, Rin Okuyama wrote: >>> ... > This was my misunderstanding. These codes are used when tracer is 64-bit > and traced is 32-bit. Don't know whether this is useful though... Yes, and someone broke them because all the ptrace 64->32 calls for registers seem to return 0 now. Was that code changed recently? >>> ... >>> >>> I fixed it: >>> http://mail-index.netbsd.org/source-changes/2019/11/22/msg111068.html >>> >>> Now, gdb/amd64 seems to work for i386 binaries to some extent, at least. >> >> Thanks! I think that the gdb on head should working for i386 binaries >> on amd64. I am installing a new kernel and userland and I will test >> shortly. > > And it works fine for both static and dynamic binaries. We could also add > kernel debugging support for i386 kernels, but that needs changes in > the i386 header files: > > 1. Everywhere s/#include I think this is a good change anyway for all MD headers because they >expect to load their own arch stuff. MI stuff needs to stay " 2. change __vaddr_t in segments.h ld_descriptor to uint32_t >That should have no impact and it is a good change because it is a >packed structure and needs to be fixed size. > 3. Add some fixed field sizes in struct pcb instead of pointers with >#ifdef __x86_64__ (or copy struct pcb into gdb like kamil did). This >is just ugly, but I think it is better than copying the struct. I >could add a comment explaining why. > > Is it worth it? > I consider fixing this as required. > chritod > signature.asc Description: OpenPGP digital signature
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
In article , Christos Zoulas wrote: >In article <468095c0-b973-7f23-1cfa-3dc19e3b8...@gmail.com>, >Rin Okuyama wrote: >>On 2019/11/22 10:56, Christos Zoulas wrote: >>> In article <679493cf-3e85-f56d-85e4-dfaf7958a...@gmail.com>, >>> Rin Okuyama wrote: >>... This was my misunderstanding. These codes are used when tracer is 64-bit and traced is 32-bit. Don't know whether this is useful though... >>> >>> Yes, and someone broke them because all the ptrace 64->32 calls for >>> registers seem to return 0 now. Was that code changed recently? >>... >> >>I fixed it: >>http://mail-index.netbsd.org/source-changes/2019/11/22/msg111068.html >> >>Now, gdb/amd64 seems to work for i386 binaries to some extent, at least. > >Thanks! I think that the gdb on head should working for i386 binaries >on amd64. I am installing a new kernel and userland and I will test >shortly. And it works fine for both static and dynamic binaries. We could also add kernel debugging support for i386 kernels, but that needs changes in the i386 header files: 1. Everywhere s/#include
Re: PT32_[GS]ETXMMREGS for amd64 (Was: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h)
On 2019/11/22 15:58, Michał Górny wrote: On Fri, 2019-11-22 at 10:10 +0900, Rin Okuyama wrote: Hi, thank you for your review! On 2019/11/22 0:48, Kamil Rytarowski wrote: On 21.11.2019 10:49, Rin Okuyama wrote: ... I wrote a draft version of patch which adds PT32_[GS]ETXMMREGS support: http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch With this patch, XMM-related tests pass for COMPAT_NETBSD32 on amd64. Some remarks: (1) PT_[GS]ETXMMREGS ptrace(2) commands are added to . These are only used for COMPAT_NETBSD32, and not exposed to userland. This is correct. We don't want to export XMMREGS to amd64 users. Pleae remove /* */ from this part: +/* + "PT_GETXMMREGS", \ + "PT_SETXMMREGS" + */ Yes, I will. This will allow ktruss and related software to have meaningful form for compat32 ptracing. (2) COMPAT_NETBSD32 codes are called from process_machdep.c via module_hook framework. This may be too much though... I have no opinion here. Now, Paul and I are discussed how to improve the usage of module_hook. I will update this part in accordance with his advice. Comments? I will leave this to be reviewed by mgorny@. Adding him to CC. I see. Hi, mgorny@. Please look into it! http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch It seems correct at a first glance. The hook logic is unknown to me, so I can't comment on that. If I was to be picky, I'd prefer if changes in existing code that are not exactly relevant to adding this were split into a separate patches (e.g. changing int to bool, renaming valid* func). Thank you for your comments! Yes, I will split changes. I will commit them after finding out appropriate places where hook and its definition should be. Thanks, rin
Re: PT32_[GS]ETXMMREGS for amd64 (Was: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h)
On Fri, 2019-11-22 at 10:10 +0900, Rin Okuyama wrote: > Hi, thank you for your review! > > On 2019/11/22 0:48, Kamil Rytarowski wrote: > > On 21.11.2019 10:49, Rin Okuyama wrote: > ... > > > I wrote a draft version of patch which adds PT32_[GS]ETXMMREGS support: > > > > > > http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch > > > > > > With this patch, XMM-related tests pass for COMPAT_NETBSD32 on amd64. > > > > > > Some remarks: > > > > > > (1) PT_[GS]ETXMMREGS ptrace(2) commands are added to . > > > These are only used for COMPAT_NETBSD32, and not exposed to userland. > > > > > > > This is correct. > > We don't want to export XMMREGS to amd64 users. > > > > Pleae remove /* */ from this part: > > > > +/* > > + "PT_GETXMMREGS", \ > > + "PT_SETXMMREGS" > > + */ > > Yes, I will. > > > This will allow ktruss and related software to have meaningful form for > > compat32 ptracing. > > > > > (2) COMPAT_NETBSD32 codes are called from process_machdep.c via > > > module_hook framework. This may be too much though... > > > > > > > I have no opinion here. > > Now, Paul and I are discussed how to improve the usage of module_hook. > I will update this part in accordance with his advice. > > > > Comments? > > > > > > > I will leave this to be reviewed by mgorny@. Adding him to CC. > > I see. Hi, mgorny@. Please look into it! > > > http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch > > > > It seems correct at a first glance. The hook logic is unknown to me, so I can't comment on that. If I was to be picky, I'd prefer if changes in existing code that are not exactly relevant to adding this were split into a separate patches (e.g. changing int to bool, renaming valid* func). -- Best regards, Michał Górny
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
On 2019/11/22 10:56, Christos Zoulas wrote: In article <679493cf-3e85-f56d-85e4-dfaf7958a...@gmail.com>, Rin Okuyama wrote: ... This was my misunderstanding. These codes are used when tracer is 64-bit and traced is 32-bit. Don't know whether this is useful though... Yes, and someone broke them because all the ptrace 64->32 calls for registers seem to return 0 now. Was that code changed recently? ... I fixed it: http://mail-index.netbsd.org/source-changes/2019/11/22/msg111068.html Now, gdb/amd64 seems to work for i386 binaries to some extent, at least. Thanks, rin
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
In article <679493cf-3e85-f56d-85e4-dfaf7958a...@gmail.com>,
Rin Okuyama wrote:
>On 2019/11/20 20:12, Rin Okuyama wrote:
>> On 2019/11/19 22:59, Kamil Rytarowski wrote:
>...
>>>
>>> We still miss compat32 support for PT_GETXMMREGS and PT_SETXMMREGS, at
>>> some point of time we shall add it for completeness.
>>
>> Thank you!
>>
>> With amd64/netbsd32_machdep.c rev 1.130, all tests in t_ptrace* pass in
>> COMPAT_NETBSD32 on amd64, except for that involved with XMM registers.
>> I will examine how to implement PT32_[GS]ETXMMREGS.
>
>I wrote a draft version of patch which adds PT32_[GS]ETXMMREGS support:
>
>http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch
>
>With this patch, XMM-related tests pass for COMPAT_NETBSD32 on amd64.
>
>Some remarks:
>
>(1) PT_[GS]ETXMMREGS ptrace(2) commands are added to .
> These are only used for COMPAT_NETBSD32, and not exposed to userland.
>
>(2) COMPAT_NETBSD32 codes are called from process_machdep.c via
> module_hook framework. This may be too much though...
>
>Comments?
>
>> Also, it seems that some COMPAT_NETBSD32 related codes for amd64 need to
>> be cleaned up. For example, there remain COMPAT_NETBSD32 codes in
>> amd64/process_machdep.c, that are no longer used:
>>
>> https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#129
>> https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#183
>> ...
>>
>> I will examine them too.
>
>This was my misunderstanding. These codes are used when tracer is 64-bit
>and traced is 32-bit. Don't know whether this is useful though...
Yes, and someone broke them because all the ptrace 64->32 calls for
registers seem to return 0 now. Was that code changed recently?
christos
[8:55pm] 1404>cat hello.c
#include
#include
int
main(void)
{
printf("hello world\n");
sleep(1);
return 0;
}
[8:56pm] 1405>cc -g -m32 hello.c
[8:56pm] 1406>gdb ./a.out
GNU gdb (GDB) 8.3
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64--netbsd".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
PT32_[GS]ETXMMREGS for amd64 (Was: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h)
Hi, thank you for your review! On 2019/11/22 0:48, Kamil Rytarowski wrote: On 21.11.2019 10:49, Rin Okuyama wrote: ... I wrote a draft version of patch which adds PT32_[GS]ETXMMREGS support: http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch With this patch, XMM-related tests pass for COMPAT_NETBSD32 on amd64. Some remarks: (1) PT_[GS]ETXMMREGS ptrace(2) commands are added to . These are only used for COMPAT_NETBSD32, and not exposed to userland. This is correct. We don't want to export XMMREGS to amd64 users. Pleae remove /* */ from this part: +/* + "PT_GETXMMREGS", \ + "PT_SETXMMREGS" + */ Yes, I will. This will allow ktruss and related software to have meaningful form for compat32 ptracing. (2) COMPAT_NETBSD32 codes are called from process_machdep.c via module_hook framework. This may be too much though... I have no opinion here. Now, Paul and I are discussed how to improve the usage of module_hook. I will update this part in accordance with his advice. Comments? I will leave this to be reviewed by mgorny@. Adding him to CC. I see. Hi, mgorny@. Please look into it! http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch Also, it seems that some COMPAT_NETBSD32 related codes for amd64 need to be cleaned up. For example, there remain COMPAT_NETBSD32 codes in amd64/process_machdep.c, that are no longer used: https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#129 https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#183 ... I will examine them too. This was my misunderstanding. These codes are used when tracer is 64-bit and traced is 32-bit. Don't know whether this is useful though... Thanks, rin Maxime added it. If you want to change it, please consult with maxv@. OK. It must be useful for a tracer capable for both 64- and 32-bit processes (although, we also need to provide x87-style fpregs for 64-bit tracer in some way). Thanks, rin
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
On 21.11.2019 10:49, Rin Okuyama wrote: > On 2019/11/20 20:12, Rin Okuyama wrote: >> On 2019/11/19 22:59, Kamil Rytarowski wrote: > ... >>> >>> We still miss compat32 support for PT_GETXMMREGS and PT_SETXMMREGS, at >>> some point of time we shall add it for completeness. >> >> Thank you! >> >> With amd64/netbsd32_machdep.c rev 1.130, all tests in t_ptrace* pass in >> COMPAT_NETBSD32 on amd64, except for that involved with XMM registers. >> I will examine how to implement PT32_[GS]ETXMMREGS. > > I wrote a draft version of patch which adds PT32_[GS]ETXMMREGS support: > > http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch > > With this patch, XMM-related tests pass for COMPAT_NETBSD32 on amd64. > > Some remarks: > > (1) PT_[GS]ETXMMREGS ptrace(2) commands are added to . > These are only used for COMPAT_NETBSD32, and not exposed to userland. > This is correct. We don't want to export XMMREGS to amd64 users. Pleae remove /* */ from this part: +/* + "PT_GETXMMREGS", \ + "PT_SETXMMREGS" + */ This will allow ktruss and related software to have meaningful form for compat32 ptracing. > (2) COMPAT_NETBSD32 codes are called from process_machdep.c via > module_hook framework. This may be too much though... > I have no opinion here. > Comments? > I will leave this to be reviewed by mgorny@. Adding him to CC. >> Also, it seems that some COMPAT_NETBSD32 related codes for amd64 need to >> be cleaned up. For example, there remain COMPAT_NETBSD32 codes in >> amd64/process_machdep.c, that are no longer used: >> >> https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#129 >> >> https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#183 >> >> ... >> >> I will examine them too. > > This was my misunderstanding. These codes are used when tracer is 64-bit > and traced is 32-bit. Don't know whether this is useful though... > > Thanks, > rin Maxime added it. If you want to change it, please consult with maxv@. signature.asc Description: OpenPGP digital signature
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
On 2019/11/20 20:12, Rin Okuyama wrote: On 2019/11/19 22:59, Kamil Rytarowski wrote: ... We still miss compat32 support for PT_GETXMMREGS and PT_SETXMMREGS, at some point of time we shall add it for completeness. Thank you! With amd64/netbsd32_machdep.c rev 1.130, all tests in t_ptrace* pass in COMPAT_NETBSD32 on amd64, except for that involved with XMM registers. I will examine how to implement PT32_[GS]ETXMMREGS. I wrote a draft version of patch which adds PT32_[GS]ETXMMREGS support: http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch With this patch, XMM-related tests pass for COMPAT_NETBSD32 on amd64. Some remarks: (1) PT_[GS]ETXMMREGS ptrace(2) commands are added to . These are only used for COMPAT_NETBSD32, and not exposed to userland. (2) COMPAT_NETBSD32 codes are called from process_machdep.c via module_hook framework. This may be too much though... Comments? Also, it seems that some COMPAT_NETBSD32 related codes for amd64 need to be cleaned up. For example, there remain COMPAT_NETBSD32 codes in amd64/process_machdep.c, that are no longer used: https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#129 https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#183 ... I will examine them too. This was my misunderstanding. These codes are used when tracer is 64-bit and traced is 32-bit. Don't know whether this is useful though... Thanks, rin
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
On 2019/11/19 22:59, Kamil Rytarowski wrote: On 18.11.2019 13:08, Rin Okuyama wrote: On 2019/11/18 20:15, Kamil Rytarowski wrote: 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. Thank you for providing the patch. It looks fine for me. I committed it with changes below: - Take into account of __arm__ && __ARM_EABI__ for compat with arm-oabi - Undefine NETBSD32_SIGINFO_UINT64_ALIGN for clarity Thanks! rin Great work! Please try to run in compat32: cd /usr/tests/lib/libc/sys atf-run t_ptrace* | atf-report We still miss compat32 support for PT_GETXMMREGS and PT_SETXMMREGS, at some point of time we shall add it for completeness. Thank you! With amd64/netbsd32_machdep.c rev 1.130, all tests in t_ptrace* pass in COMPAT_NETBSD32 on amd64, except for that involved with XMM registers. I will examine how to implement PT32_[GS]ETXMMREGS. Also, it seems that some COMPAT_NETBSD32 related codes for amd64 need to be cleaned up. For example, there remain COMPAT_NETBSD32 codes in amd64/process_machdep.c, that are no longer used: https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#129 https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#183 ... I will examine them too. Thanks, rin
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
On 18.11.2019 13:08, Rin Okuyama wrote: > On 2019/11/18 20:15, Kamil Rytarowski wrote: >> 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. >> > > Thank you for providing the patch. It looks fine for me. I committed > it with changes below: > > - Take into account of __arm__ && __ARM_EABI__ for compat with arm-oabi > - Undefine NETBSD32_SIGINFO_UINT64_ALIGN for clarity > > Thanks! > rin Great work! Please try to run in compat32: cd /usr/tests/lib/libc/sys atf-run t_ptrace* | atf-report We still miss compat32 support for PT_GETXMMREGS and PT_SETXMMREGS, at some point of time we shall add it for completeness. signature.asc Description: OpenPGP digital signature
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
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
> v.s. ,
> I propose
>
> (1) Move NETBSD32_INT64_ALIGN from to
>
>
> (2) Move netbsd32_{,u}int64 from to
>
>
> 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 , we need to move NETBSD32_INT64_ALIGN to
>>> other than . If so,
>>> why not ?
>>>
>>
>> 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 , 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
> , and
>
> (2) needs
>
> Therefore, we cannot use NETBSD32_INT64_ALIGN in .
> Of course, we can typedef _netbsd32_uint64 in 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.
signature.asc
Description: OpenPGP digital signature
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
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
v.s. ,
I propose
(1) Move NETBSD32_INT64_ALIGN from to
(2) Move netbsd32_{,u}int64 from to
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 , we need to move NETBSD32_INT64_ALIGN to
other than . If so,
why not ?
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 , 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
, and
(2) needs
Therefore, we cannot use NETBSD32_INT64_ALIGN in .
Of course, we can typedef _netbsd32_uint64 in 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
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
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.
> 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
>>> v.s. ,
>>> I propose
>>>
>>> (1) Move NETBSD32_INT64_ALIGN from to
>>>
>>>
>>> (2) Move netbsd32_{,u}int64 from to
>>>
>>>
>>> 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 , we need to move NETBSD32_INT64_ALIGN to
> other than . If so,
> why not ?
>
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 , 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).
> Thanks,
> rin
signature.asc
Description: OpenPGP digital signature
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
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.
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
v.s. ,
I propose
(1) Move NETBSD32_INT64_ALIGN from to
(2) Move netbsd32_{,u}int64 from to
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 , we need to move NETBSD32_INT64_ALIGN to
other than . If so,
why not ?
Also, in my proposal, spaghetti dependencies are avoided in the end;
everyone depends on , and not on each other.
Thanks,
rin
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
On 17.11.2019 04:34, Rin Okuyama wrote:
> Hi,
>
> In order to distangle circular dependency between
> v.s. ,
> I propose
>
> (1) Move NETBSD32_INT64_ALIGN from to
>
>
> (2) Move netbsd32_{,u}int64 from to
>
>
> 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 - 1.8
> +++ sys/compat/sys/siginfo.h 12 Nov 2019 11:04:58 -
> @@ -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
> , and requires
> .
>
> Thoughts? Any comments or objections?
>
> Thanks,
> rin
signature.asc
Description: OpenPGP digital signature
netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
Hi,
In order to distangle circular dependency between
v.s. ,
I propose
(1) Move NETBSD32_INT64_ALIGN from to
(2) Move netbsd32_{,u}int64 from to
See attached patch for example on amd64.
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:
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.h30 Sep 2019 21:13:33 - 1.8
+++ sys/compat/sys/siginfo.h12 Nov 2019 11:04:58 -
@@ -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
, and requires
.
Thoughts? Any comments or objections?
Thanks,
rin
Index: sys/compat/netbsd32/netbsd32.h
===
RCS file: /home/netbsd/src/sys/compat/netbsd32/netbsd32.h,v
retrieving revision 1.128
diff -p -u -r1.128 netbsd32.h
--- sys/compat/netbsd32/netbsd32.h 7 Nov 2019 15:21:55 - 1.128
+++ sys/compat/netbsd32/netbsd32.h 17 Nov 2019 03:11:34 -
@@ -37,6 +37,7 @@
*/
#include /* precautionary upon removal from ucred.h */
+#include
#include
#include
#include
@@ -72,7 +73,11 @@ typedef int32_t netbsd32_key_t;
typedef int32_t netbsd32_intptr_t;
typedef uint32_t netbsd32_uintptr_t;
-/* netbsd32_[u]int64 are machine dependent and defined below */
+/*
+ * netbsd32_[u]int64 are machine dependent and defined in :
+ * 64 bit integers only have 4-byte alignment on some 32 bit ports,
+ * but always have 8-byte alignment on 64 bit systems.
+ */
/*
* machine dependant section; must define:
@@ -154,15 +159,6 @@ netbsd32_ptr32_incr(netbsd32_pointer_t *
#undef NETBSD32_POINTER_TYPE
/*
- * 64 bit integers only have 4-byte alignment on some 32 bit ports,
- * but always have 8-byte alignment on 64 bit systems.
- * NETBSD32_INT64_ALIGN may be __attribute__((__aligned__(4)))
- */
-typedef int64_t netbsd32_int64 NETBSD32_INT64_ALIGN;
-typedef uint64_t netbsd32_uint64 NETBSD32_INT64_ALIGN;
-#undef NETBSD32_INT64_ALIGN
-
-/*
* all pointers are netbsd32_pointer_t (defined in
)
*/
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.h30 Sep 2019 21:13:33 - 1.8
+++ sys/compat/sys/siginfo.h17 Nov 2019 03:17:02 -
@@ -34,6 +34,8 @@
#ifdef _KERNEL
+#include
+
typedef union sigval32 {
int sival_int;
uint32_t sival_ptr;
@@ -73,7 +75,7 @@ struct __ksiginfo32 {
int _sysnum;
int _retval[2];
int _error;
- uint64_t _args[8]; /* SYS_MAXSYSARGS */
+ netbsd32_uint64 _args[8]; /* SYS_MAXSYSARGS */
} _syscall;
struct {
Index: sys/sys/types.h
===
RCS file: /home/netbsd/src/sys/sys/types.h,v
retrieving revision 1.102
diff -p -u -r1.102 types.h
--- sys/sys/types.h 6 Nov 2018 16:26:44 - 1.102
+++ sys/sys/types.h 17 Nov 2019 03:22:49 -
@@ -348,6 +348,16 @@ struct uio;
#defineCLR(t, f) ((t) &= ~(f))
#endif
+#if defined(_KERNEL) && defined(_LP64)
+/*
+ * See comments in for more details.
+ * _NETBSD32_INT64_ALIGN may be __attribute__((__aligned__(4))).
+ */
+typedef int64_t netbsd32_int64 _NETBSD32_INT64_ALIGN;
+typedef uint64_t netbsd32_uint64
