Re: CVS commit: src/tests/bin/sh
On Thu, Dec 28, 2023 at 11:08 PM Robert Elz wrote: > > [I could claim that the typo was deliberate, as part of > the test but that would be kind of absurd, sh does > no spell checking to test.] > > kre > Thanks for clarification. I definitely had few instances when I needed to revert spelling fixes, which were left on purpose for one reason or another, fortunately it is not the case here :).
Re: CVS commit: src/tests/bin/sh
Date:Thu, 28 Dec 2023 20:04:11 + From:"Andrius Varanavicius" Message-ID: <20231228200411.283ccf...@cvs.netbsd.org> | Modified Files: | src/tests/bin/sh: t_syntax.sh | Log Message: | s/synax/syntax/ in test description. Thanks for the correction, but, not that it matters, that was in the test code, not just a description. [I could claim that the typo was deliberate, as part of the test but that would be kind of absurd, sh does no spell checking to test.] kre
Re: CVS commit: src/tests/sbin/ifconfig
On 2023/10/18 17:25, Rin Okuyama wrote: Module Name:src Committed By: rin Date: Wed Oct 18 08:25:14 UTC 2023 Modified Files: src/tests/sbin/ifconfig: t_capabilities.sh Log Message: ifconfig/t_capabilities: Skip unless run_unsafe is configured to yes The test modifies if_capabilities for all available interfaces. This is not a behavior we expect for normal ATF runs. s/if_capabilities/if_capenable/ here. Thanks, rin
Re: CVS commit: src/tests/lib/libc/sys
Date:Mon, 1 Aug 2022 18:55:15 +0300 From:Valery Ushakov Message-ID: | The test uses __clone(), not clone() and a followup fix made __clone() | visible under plain _NETBSD_SOURCE again - which is the right thing, | IMO. So this change is not necessary (the test only uses __clone()). Yes, I saw, but the test really should be using clone() - that"s the thing which needs to work (for those odd apps which use it). kre
Re: CVS commit: src/tests/lib/libc/sys
On Mon, Aug 01, 2022 at 15:48:40 +, Robert Elz wrote: > Module Name: src > Committed By: kre > Date: Mon Aug 1 15:48:40 UTC 2022 > > Modified Files: > src/tests/lib/libc/sys: Makefile > > Log Message: > Provide _GNU_SOURCE for t_clone now that is required to make clone() > visible. The test uses __clone(), not clone() and a followup fix made __clone() visible under plain _NETBSD_SOURCE again - which is the right thing, IMO. So this change is not necessary (the test only uses __clone()). -uwe
Re: CVS commit: src/tests/libexec/ld.elf_so
On 21/06/2022 17:24, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Tue Jun 21 16:24:37 UTC 2022 Modified Files: src/tests/libexec/ld.elf_so: t_ifunc.c Log Message: sort; it is the same list as in h_ifunc_static.c; perhaps it should be a HAVE_ something? Thanks. I agree a HAVE_ something (in a arch/foo.h file) would be better. Nick
Re: CVS commit: src/tests/fs/vfs
Date:Sat, 5 Feb 2022 22:20:16 + From:David Brownlee Message-ID: | Oops, my earliest unix experience was on a BSD4.3 variant, so I was | spoiled by ffs and didn't realise the (in this context) helpful v7fs | behaviour with overlong filename components. To clarify ... I meant7th edition (and earlier) filesystems, not necessarily the thing we have called v7fs about which I know nothing ... thiugh I wondered when I saw your PR whether a name length problem might be what caused that. kre
Re: CVS commit: src/tests/fs/vfs
On Wed, 2 Feb 2022 at 17:24, Robert Elz wrote: > > Date:Wed, 2 Feb 2022 15:26:21 + > From:David Brownlee > Message-ID: > > > | So, we just need an optional flag when mounting v7fs to truncate any > | looked up filename component to 14 characters > > That's not, or shouldn't be, necessary - that always happened, the limit was > what was stored in the directory, not on the length of the pathname components > passed to namei. > > Further, v7fs (systems of that vintage) had no concept at all of a maximum > pathname length (provided there was available ram to store the string). > (Apologies for continuing further down this rabbit hole :) Oops, my earliest unix experience was on a BSD4.3 variant, so I was spoiled by ffs and didn't realise the (in this context) helpful v7fs behaviour with overlong filename components. As a quick test extracting rescue.tar.xz into a v7fs and chrooting into rescue/sh works, and rsyncing enough to get /bin/sh chrooting also works (extracting base.tar.xz runs into issues - PR bin/56690) Actually, there were enough other issues found in PR bin/56690 that I would have to regretfully recommend against v7fs for production NetBSD systems (ahem :) David
Re: CVS commit: src/tests/fs/vfs
Date:Wed, 2 Feb 2022 15:26:21 + From:David Brownlee Message-ID: | So, we just need an optional flag when mounting v7fs to truncate any | looked up filename component to 14 characters That's not, or shouldn't be, necessary - that always happened, the limit was what was stored in the directory, not on the length of the pathname components passed to namei. Further, v7fs (systems of that vintage) had no concept at all of a maximum pathname length (provided there was available ram to store the string). kre
Re: CVS commit: src/tests/fs/vfs
> On Feb 2, 2022, at 6:47 AM, Robert Elz wrote: > >Date:Wed, 2 Feb 2022 07:11:45 + >From:David Holland >Message-ID: > > | v7fs isn't a compat interface for old users, > > That's sad, I could do with something just for me! > > | it's a compat interface for old disk images :-) > > And makefs -t v7fs fits into that purpose how? > > So maybe it is for us truly old fogies (can we have v6fs as well? > Then I'd really feel at home.) Can I have a v7fs as root, and > boot from it? Does sysinst support it? I thought we maybe supported a system whose ROM boots from it? -- thorpej
Re: CVS commit: src/tests/fs/vfs
Hello, On Wed, 02 Feb 2022 21:47:25 +0700 Robert Elz wrote: > So maybe it is for us truly old fogies (can we have v6fs as well? Well, there is this thing... https://github.com/jaylogue/retro-fuse A user-space filesystem (FUSE) for accessing ancient Unix filesystems. retro-fuse provides a way to mount filesystems created by ancient Unix systems on modern OSes. The current version of retro-fuse supports mounting filesystems created by fifth, sixth and seventh-edition research Unix, as well as 2.9BSD and 2.11BSD. It can also initialize such filesystems. have fun Michael
Re: CVS commit: src/tests/fs/vfs
On Wed, 2 Feb 2022 at 14:47, Robert Elz wrote: > > Date:Wed, 2 Feb 2022 07:11:45 + > From:David Holland > Message-ID: > > | v7fs isn't a compat interface for old users, > > That's sad, I could do with something just for me! Sounds like we need a compat_kre(8) - assuming it would be more correct to provide the appropriate trailing slash behaviour for all filesystems in that mode? :-p > | it's a compat interface for old disk images :-) > > And makefs -t v7fs fits into that purpose how? > > So maybe it is for us truly old fogies (can we have v6fs as well? > Then I'd really feel at home.) Can I have a v7fs as root, and > boot from it? Maybe? - Throw together a bootxx_v7fs - Leave /dev with just MAKEDEV so the system will mount a mfs /dev - Convert symlinks to hard links or file copies when setting up - Then it's just the 14 character filename component limit: excluding drm firmware, kernel modules and X config most filename components are 14 characters or less - but there are a few libraries in /lib with longer filenames - lib/libcrypto.so.14 lib/libtermcap.so.0 lib/libterminfo.so.1 and lib/libpthread.so.1 are likely to be problematic So, we just need an optional flag when mounting v7fs to truncate any looked up filename component to 14 characters, then we're good to go. Actually, I'm a little concerned about how close it could be to being possible! :) > Does sysinst support it? It would be under the --spinal-tap option David
Re: CVS commit: src/tests/fs/vfs
Date:Wed, 2 Feb 2022 07:11:45 + From:David Holland Message-ID: | v7fs isn't a compat interface for old users, That's sad, I could do with something just for me! | it's a compat interface for old disk images :-) And makefs -t v7fs fits into that purpose how? So maybe it is for us truly old fogies (can we have v6fs as well? Then I'd really feel at home.) Can I have a v7fs as root, and boot from it? Does sysinst support it? kre
Re: CVS commit: src/tests/fs/vfs
On Wed, Feb 02, 2022 at 05:43:45AM +0700, Robert Elz wrote: > | Test mkdir(2) with one or more trailing slashes - this currently fails > | for v7fs. > > As it should I think, trailing slashes are not simply deleted in v7fs. > > [...] > > If this was ever changed, it would not truly be a v7fs any more. v7fs isn't a compat interface for old users, it's a compat interface for old disk images :-) -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/tests/fs/vfs
Date:Tue, 1 Feb 2022 18:27:24 + From:"Martin Husemann" Message-ID: <20220201182724.90f82f...@cvs.netbsd.org> | Test mkdir(2) with one or more trailing slashes - this currently fails | for v7fs. As it should I think, trailing slashes are not simply deleted in v7fs. If you do mkdir /path/to/dir/ in a v7 fs, then you're guaranteed (unless some other error happens earlier) either ENOENT if /path/to/dir doesn't already exist, ENOTDIR if it it does but isn't a directory, or EEXIST if it exists and is a directory. The thing to be created always follows the final slash, everything prior to that is path to look up, and must all exist and be directories (with appropriate permissions, etc).In this case the "thing" is "" which is (kind of) an alias for "." (without ever actually looking up "."). It can never not exist if the preceding path all exists. If this was ever changed, it would not truly be a v7fs any more. kre ps: I never understood the fascination with always writing directory names with a trailing / - it seems to come largely from filename completion where the '/' is added if the name found is a directory, so you can just go on typing anything that is to follow in the path (but could easily be removed again if no more components are added - just isn't) but it makes no sense for this to have happened with mkdir, filename completion can only find files that exist, not ones to be created, so it couldn't be that which adds the '/' after the directory name - some human must be doing that, but why? It seems to be to be just meaningless extra unneeded typing.
Re: CVS commit: src/tests/lib/libcurses
On Tue, Jan 25, 2022 at 03:23:05AM +, Brett Lymn wrote: > Module Name: src > Committed By: blymn > Date: Tue Jan 25 03:23:05 UTC 2022 > > Modified Files: > src/tests/lib/libcurses: debug_test t_curses.sh > src/tests/lib/libcurses/check_files: add_wch3.chk get_wstr.chk > getn_wstr.chk ins_wch1.chk ins_wch2.chk ins_wch3.chk mvins_wch.chk > wget_wstr.chk wgetn_wstr.chk wins_wch1.chk wins_wch2.chk > wins_wch3.chk wvline_set.chk > src/tests/lib/libcurses/tests: add_wch ins_wch overwrite > > Log Message: > Update of tests to account for output changes associated with wide char > fixes. Also, default all tests to using UTF8 instead of doing a special > dance for the wide character tests and fix debug_test to force set the > locale to UTF8 so tests under debug don't throw spurious mismatches > when a wide character test is run. This makes them all fail for me: Tests root: /usr/tests/lib/libcurses t_curses (1/1): 201 test cases add_wch: [0.174435s] Failed: Test case exited normally but failed to create the results file: Failed to open /tmp/atf-run.dxlAhY/tcr addbytes: [0.152203s] Failed: Test case exited normally but failed to create the results file: Failed to open /tmp/atf-run.dxlAhY/tcr [..] Martin
re: CVS commit: src/tests/lib/csu
Christos Zoulas writes: > In article , Joerg Sonnenberger > wrote: > >On Fri, Jun 04, 2021 at 10:20:24PM -, Christos Zoulas wrote: > >> In article , Joerg Sonnenberger > > wrote: > >> >On Thu, Jun 03, 2021 at 04:17:37PM -0400, Christos Zoulas wrote: > >> >> Module Name:src > >> >> Committed By: christos > >> >> Date: Thu Jun 3 20:17:37 UTC 2021 > >> >> > >> >> Modified Files: > >> >> src/tests/lib/csu: h_ifunc_static.c > >> >> > >> >> Log Message: > >> >> PR/56230: Jan-Benedict Glaw: arm oabi does not and will not have ifunc > >> >support. > >> > > >> >As I mentioned elsewhere, I disagree with this change. OABI is dead and > >> >only really support for legacy compatibility. All the OABI build > >> >variants should be removed and this is strongly a step in the wrong > >> >direction. > >> > >> I think it is better to do it after the 10 branch. It is not just removing > >> the build.sh targets, we need to remove the compat glue and fix the sets, > >> and perhaps something else I am missing. I will revert the two changes > >> once we remove all oabi support. > > > >I don't see how that's related. I'm not talking about removing the > >compat layer right now. Remove the build.sh targets and call it a day. > > CC:ed tech-toolchain. Does everyone agree to remove the oabi build targets? AFAICT, GCC removed the basic oabi targets recently, which is why they weren't updated. we have some dead code in our gcc/arm port in the GCC 10 tree. you can still target oabi with compiler options, but there are no configurations that default to it any more. at this point i'm also wanting to turn off the compat libs for everyone entirely -- if you have old oabi apps, you're welcome to use old libs to run them, same as eg, when we switched from a.out to ELF originally. this is *not* the kernel support to run these apps. ie, remove it from build.sh and share/mk/ ASAP -- before the netbsd 10 branch. .mrg.
Re: CVS commit: src/tests/lib/csu
In article , Joerg Sonnenberger wrote: >On Fri, Jun 04, 2021 at 10:20:24PM -, Christos Zoulas wrote: >> In article , Joerg Sonnenberger > wrote: >> >On Thu, Jun 03, 2021 at 04:17:37PM -0400, Christos Zoulas wrote: >> >> Module Name: src >> >> Committed By: christos >> >> Date: Thu Jun 3 20:17:37 UTC 2021 >> >> >> >> Modified Files: >> >> src/tests/lib/csu: h_ifunc_static.c >> >> >> >> Log Message: >> >> PR/56230: Jan-Benedict Glaw: arm oabi does not and will not have ifunc >> >support. >> > >> >As I mentioned elsewhere, I disagree with this change. OABI is dead and >> >only really support for legacy compatibility. All the OABI build >> >variants should be removed and this is strongly a step in the wrong >> >direction. >> >> I think it is better to do it after the 10 branch. It is not just removing >> the build.sh targets, we need to remove the compat glue and fix the sets, >> and perhaps something else I am missing. I will revert the two changes >> once we remove all oabi support. > >I don't see how that's related. I'm not talking about removing the >compat layer right now. Remove the build.sh targets and call it a day. CC:ed tech-toolchain. Does everyone agree to remove the oabi build targets? christos
Re: CVS commit: src/tests/lib/csu
On Fri, Jun 04, 2021 at 10:20:24PM -, Christos Zoulas wrote: > In article , Joerg Sonnenberger > wrote: > >On Thu, Jun 03, 2021 at 04:17:37PM -0400, Christos Zoulas wrote: > >> Module Name: src > >> Committed By: christos > >> Date: Thu Jun 3 20:17:37 UTC 2021 > >> > >> Modified Files: > >>src/tests/lib/csu: h_ifunc_static.c > >> > >> Log Message: > >> PR/56230: Jan-Benedict Glaw: arm oabi does not and will not have ifunc > >support. > > > >As I mentioned elsewhere, I disagree with this change. OABI is dead and > >only really support for legacy compatibility. All the OABI build > >variants should be removed and this is strongly a step in the wrong > >direction. > > I think it is better to do it after the 10 branch. It is not just removing > the build.sh targets, we need to remove the compat glue and fix the sets, > and perhaps something else I am missing. I will revert the two changes > once we remove all oabi support. I don't see how that's related. I'm not talking about removing the compat layer right now. Remove the build.sh targets and call it a day. Joerg
Re: CVS commit: src/tests/lib/csu
In article , Joerg Sonnenberger wrote: >On Thu, Jun 03, 2021 at 04:17:37PM -0400, Christos Zoulas wrote: >> Module Name: src >> Committed By:christos >> Date:Thu Jun 3 20:17:37 UTC 2021 >> >> Modified Files: >> src/tests/lib/csu: h_ifunc_static.c >> >> Log Message: >> PR/56230: Jan-Benedict Glaw: arm oabi does not and will not have ifunc >support. > >As I mentioned elsewhere, I disagree with this change. OABI is dead and >only really support for legacy compatibility. All the OABI build >variants should be removed and this is strongly a step in the wrong >direction. I think it is better to do it after the 10 branch. It is not just removing the build.sh targets, we need to remove the compat glue and fix the sets, and perhaps something else I am missing. I will revert the two changes once we remove all oabi support. christos
Re: CVS commit: src/tests/lib/csu
On Thu, Jun 03, 2021 at 04:17:37PM -0400, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Thu Jun 3 20:17:37 UTC 2021 > > Modified Files: > src/tests/lib/csu: h_ifunc_static.c > > Log Message: > PR/56230: Jan-Benedict Glaw: arm oabi does not and will not have ifunc > support. As I mentioned elsewhere, I disagree with this change. OABI is dead and only really support for legacy compatibility. All the OABI build variants should be removed and this is strongly a step in the wrong direction. Joerg
Re: CVS commit: src/tests/lib/libcurses/director
On Tue, Apr 06, 2021 at 00:47:00 +, Roland Illig wrote: > The previous "table" was an insult to any reader. It was unsorted, > listed the functions shuffled, and was not even formatted consistently. That's pretty rude. Please, tone down your commit "messages". -uwe
Re: GCC TSan (Re: CVS commit: src/tests/usr.bin)
On Tue, Sep 15, 2020 at 03:32:25PM +0200, Kamil Rytarowski wrote: > I've tried to mark the TSan parts that need porting as explicit failure, > soo we can reduce the risk of shipping unported runtime. That risc is quite low as currently the runtime is apparently not buildable on anything but amd64 ;-) Indeed once we are able to build runtime components we should also adjust the tested architectures. Would be great if we could easily query such things, but it is not even easy to conditionalize the tests on gcc 9 or newer. Martin
GCC TSan (Re: CVS commit: src/tests/usr.bin)
On 15.09.2020 07:03, Martin Husemann wrote: > On Mon, Sep 14, 2020 at 03:17:53PM +, Kamil Rytarowski wrote: >> Enable TSan tests for GCC and >32bit address space environments > > Since tsan does not work on all architectures, this is not a good idea. > It would be better to code it with an explicit list of architectures > supported. > > Martin > I've tried to mark the TSan parts that need porting as explicit failure, soo we can reduce the risk of shipping unported runtime. There are generally three such parts: - address space memory mappings - setjmp mapping of stack pointer - setjmp/longjmp assembly code I noted that not all ATF TSan tests pass for GCC amd64. I suspect that the runtime is still too old (even older than Clang-7 2 years ago, when we added the ATF tests). Another difference is that LLVM by default links the runtime statically and GCC dynamically. There is a crash with dl_iterate_phdr(3). This is possibly related to the crash with -pg. GCC requires to deliver .spec files for static linking of sanitizers. We need to generate libsanitizer.spec and install it to /usr/lib/. I will have a look into it. GCC9 also wants to install libasan_preinit.o, liblsan_preinit.o, libtsan_preinit.o. I'm going to prepare appropriate build rules for them. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/usr.bin
On Mon, Sep 14, 2020 at 03:17:53PM +, Kamil Rytarowski wrote: > Enable TSan tests for GCC and >32bit address space environments Since tsan does not work on all architectures, this is not a good idea. It would be better to code it with an explicit list of architectures supported. Martin
Re: CVS commit: src/tests/lib/libc/stdlib
On Sat, Jun 27, 2020 at 10:39:08AM +, m...@netbsd.org wrote: > > Add the default TNF copyright (2005), cf. PR misc/55419. > > I don't think we can generally do this. Can you clarify if you discussed > this with the author in commit messages? Well, the committer is christos, and I doubt he cares. But as noted in the test, this 15 year old code snippet is from another person. Like with many test cases, I suppose it was originally taken/modified from a PR or a mailing list. As I noted in PR misc/55419, the problem is generic. I doubt whether it is even possible to contact all people whose code appears in src/tests. But I guess at least all NetBSD people should add the meta-data if they have code in src/tests. - Jukka
Re: CVS commit: src/tests/lib/libc/stdlib
On Sat, Jun 27, 2020 at 10:19:43AM +, Jukka Ruohonen wrote: > Module Name: src > Committed By: jruoho > Date: Sat Jun 27 10:19:43 UTC 2020 > > Modified Files: > src/tests/lib/libc/stdlib: t_mbtowc.c > > Log Message: > Add the default TNF copyright (2005), cf. PR misc/55419. > > > To generate a diff of this commit: > cvs rdiff -u -r1.1 -r1.2 src/tests/lib/libc/stdlib/t_mbtowc.c > I don't think we can generally do this. Can you clarify if you discussed this with the author in commit messages? Thanks.
Re: CVS commit: src/tests/usr.sbin
On Wed, Jun 24, 2020 at 03:05:45PM +, Martin Husemann wrote: > Modified Files: > src/tests/usr.sbin/stdethers: Makefile > src/tests/usr.sbin/stdhosts: Makefile > > Log Message: > Add input files Ups. I try to be more careful, now that I again remember the nitty-gritties. - Jukka
Re: CVS commit: src/tests/lib/libc/sys
Committed. Thank you for your comment! rin On 2020/06/22 20:09, Kamil Rytarowski wrote: On 22.06.2020 04:51, Rin Okuyama wrote: Module Name:src Committed By: rin Date: Mon Jun 22 02:51:07 UTC 2020 Modified Files: src/tests/lib/libc/sys: t_ptrace_signal_wait.h t_ptrace_wait.h Log Message: Turn trigger_fpe() back to integer division by zero for a while until QEMU bug #1668041 is fixed: https://bugs.launchpad.net/qemu/+bug/1668041 by which floating-point division by zero is not trapped correctly both on amd64 and i386. Skip *_crash_fpe tests on powerpc, where integer division by zero is never trapped. The intention of this test is just detecting SIGFPE. If the si_code value is various on different architectures (like one traps on integers other one on floats), we shall just drop the asserts for si_code and adapt the trigger_fpe() function to cover promptly more ports. The qemu bug should be fixed nonetheless.
Re: CVS commit: src/tests/lib/libc/sys
On 22.06.2020 04:51, Rin Okuyama wrote: > Module Name: src > Committed By: rin > Date: Mon Jun 22 02:51:07 UTC 2020 > > Modified Files: > src/tests/lib/libc/sys: t_ptrace_signal_wait.h t_ptrace_wait.h > > Log Message: > Turn trigger_fpe() back to integer division by zero for a while > until QEMU bug #1668041 is fixed: > > https://bugs.launchpad.net/qemu/+bug/1668041 > > by which floating-point division by zero is not trapped correctly > both on amd64 and i386. > > Skip *_crash_fpe tests on powerpc, where integer division by zero > is never trapped. > The intention of this test is just detecting SIGFPE. If the si_code value is various on different architectures (like one traps on integers other one on floats), we shall just drop the asserts for si_code and adapt the trigger_fpe() function to cover promptly more ports. The qemu bug should be fixed nonetheless. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/lib/libc/sys
On 2020/06/17 17:42, Rin Okuyama wrote: Now, all *_crash_fpe tests pass for powerpc, and nothing changes for amd64 at least. Here, powerpc means powerpc/oea, more specifically Mac mini G4. At the moment, fenv.h doesn't work correctly on booke and ibm4xx, where FPU is emulated in software. For some processors of oea, m[ft]msr instructions used in fenv.h may be privileged, and need similar care as booke/ibm4xx. Thanks, rin
Re: CVS commit: src/tests/lib/libarchive
On Tue, 16 Jun 2020, Martin Husemann wrote: On Tue, Jun 16, 2020 at 09:12:40AM -0700, Paul Goyette wrote: It might be better to run the test in a rump-kernel rather than in a "live" environment The test this is about is a plain userland test: it extracts/compresses/decompresses various archive formats and compares results. Only thing "special" is that it is in big parts cpu bound, and multi-threaded. If NetBSD can not gracefully deal with that, something is very wrong (which since about a month it is). This PR is on the "must be fixed before branching netbsd-10" list, and I hope it will be fixed quickly. Ah, my bad. I thought it was the watch-dog that was being tested. I certainly agree that it needs to fixed ASAP. ++--+---+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | ++--+---+
Re: CVS commit: src/tests/lib/libarchive
On Tue, Jun 16, 2020 at 09:12:40AM -0700, Paul Goyette wrote: > It might be better to run the test in a rump-kernel rather than in a > "live" environment The test this is about is a plain userland test: it extracts/compresses/decompresses various archive formats and compares results. Only thing "special" is that it is in big parts cpu bound, and multi-threaded. If NetBSD can not gracefully deal with that, something is very wrong (which since about a month it is). This PR is on the "must be fixed before branching netbsd-10" list, and I hope it will be fixed quickly. Martin
Re: CVS commit: src/tests/lib/libarchive
On Tue, 16 Jun 2020, Greg Troxel wrote: Jason Thorpe writes: On Jun 16, 2020, at 8:43 AM, Martin Husemann wrote: No, that is definitively not OK, which is what the PR is about. It is not OK for a regular atf run to cause a reboot of the test machine though, so this is a temporary hack around the issue (and admitedly a very ugly hack). At the very least, the user-land watchdog tickler should wire itself down. My impression of the point is that it be normal, so that the system reboots if normal processes cannot be run.It seems like once whatever bug exists is fixed, wiring is probably not necessary anyway. It might be better to run the test in a rump-kernel rather than in a "live" environment ++--+---+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | ++--+---+
Re: CVS commit: src/tests/lib/libarchive
Jason Thorpe writes: >> On Jun 16, 2020, at 8:43 AM, Martin Husemann wrote: >> >> No, that is definitively not OK, which is what the PR is about. >> >> It is not OK for a regular atf run to cause a reboot of the test machine >> though, so this is a temporary hack around the issue (and admitedly a very >> ugly hack). > > At the very least, the user-land watchdog tickler should wire itself down. My impression of the point is that it be normal, so that the system reboots if normal processes cannot be run.It seems like once whatever bug exists is fixed, wiring is probably not necessary anyway.
Re: CVS commit: src/tests/lib/libarchive
> On Jun 16, 2020, at 8:43 AM, Martin Husemann wrote: > > No, that is definitively not OK, which is what the PR is about. > > It is not OK for a regular atf run to cause a reboot of the test machine > though, so this is a temporary hack around the issue (and admitedly a very > ugly hack). At the very least, the user-land watchdog tickler should wire itself down. -- thorpej
Re: CVS commit: src/tests/lib/libarchive
On Tue, Jun 16, 2020 at 03:38:26PM -, Christos Zoulas wrote: > So we are saying that it is ok for process running with regular priority, > to be able to starve another process at the same priority from getting > any runtime for 21 seconds in a uniprocessor kernel, and this does not > indicate any problem with the scheduler implementation? This would mean > that for a HZ=100 kernel in 2100 rescheduling opportunities, the watchdog > thread was never selected to run? No, that is definitively not OK, which is what the PR is about. It is not OK for a regular atf run to cause a reboot of the test machine though, so this is a temporary hack around the issue (and admitedly a very ugly hack). Martin
Re: CVS commit: src/tests/lib/libarchive
In article <20200616075907.ecafcf...@cvs.netbsd.org>, Martin Husemann wrote: >-=-=-=-=-=- > >Module Name: src >Committed By: martin >Date: Tue Jun 16 07:59:07 UTC 2020 > >Modified Files: > src/tests/lib/libarchive: t_libarchive.sh > >Log Message: >PR kern/55272: skip this test on uniprocessor machines, it is too dangerous >and can kill the host kernel if a userland watchdog is running So we are saying that it is ok for process running with regular priority, to be able to starve another process at the same priority from getting any runtime for 21 seconds in a uniprocessor kernel, and this does not indicate any problem with the scheduler implementation? This would mean that for a HZ=100 kernel in 2100 rescheduling opportunities, the watchdog thread was never selected to run? christos
Re: CVS commit: src/tests/lib/libc/sys
On Sat, Jun 06, 2020 at 06:11:21PM +, Jason R Thorpe wrote: > Module Name: src > Committed By: thorpej > Date: Sat Jun 6 18:11:21 UTC 2020 > > Modified Files: > src/tests/lib/libc/sys: t_lwp_create.c > > Log Message: > Add a test case to ensure that _lwp_create() fails with the > expected error code when a bad new-lwp-id pointer is passed. Was thinking of just this yesterday - thanks! It's a fragile bit of code and has been fixed 3x already this year. Andrew
Re: CVS commit: src/tests/rump/modautoload
I will test this with ASan and report back!
On 16.05.2020 16:15, Christos Zoulas wrote:
> That is a completely different issue here. There are no linker tricks.
> We want the module loader to include all the symbols any module
> can require, this is why we load all the libraries.
>
> While it is questionable if nofifofs is part of the base system or not,
> this is the way it was before. Anyway it is easy enough to have it
> both ways. If we ever grow a test that needs the real fifo stuff in
> an autoloaded module, we can deal with that then.
>
> christos
>
>> On May 16, 2020, at 9:46 AM, Kamil Rytarowski wrote:
>>
>> Signed PGP part
>> On 16.05.2020 14:54, Christos Zoulas wrote:
>>> Module Name:src
>>> Committed By: christos
>>> Date: Sat May 16 12:54:27 UTC 2020
>>>
>>> Modified Files:
>>> src/tests/rump/modautoload: Makefile
>>>
>>> Log Message:
>>> Do the same thing with linker flags instead of directly specifying the
>>> archives.
>>>
>>>
>>
>> Is there chance to rename the fifo symbols instead of using linker tricks?
>>
>> I'm also not entirely sure that this will be compatible with sanitizers
>> (and C++ with the ODR rule) at this point.
>>
>>> To generate a diff of this commit:
>>> cvs rdiff -u -r1.10 -r1.11 src/tests/rump/modautoload/Makefile
>>>
>>> Please note that diffs are not public domain; they are subject to the
>>> copyright notices on the relevant files.
>>>
>>>
>>> Modified files:
>>>
>>> Index: src/tests/rump/modautoload/Makefile
>>> diff -u src/tests/rump/modautoload/Makefile:1.10
>>> src/tests/rump/modautoload/Makefile:1.11
>>> --- src/tests/rump/modautoload/Makefile:1.10Sat May 16 08:44:42 2020
>>> +++ src/tests/rump/modautoload/Makefile Sat May 16 08:54:27 2020
>>> @@ -1,4 +1,4 @@
>>> -# $NetBSD: Makefile,v 1.10 2020/05/16 12:44:42 christos Exp $
>>> +# $NetBSD: Makefile,v 1.11 2020/05/16 12:54:27 christos Exp $
>>> #
>>>
>>> .include
>>> @@ -15,11 +15,9 @@ SRCS.t_modautoload+= t_modautoload.c
>>> # subdirectory -- otherwise the LDADD lines would get a little hairy.
>>> LDFLAGS+= -Wl,-E
>>> LDADD+= \
>>> --Wl,--whole-archive \
>>> -${DESTDIR}/usr/lib/librumpvfs_nofifofs.a \
>>> -${DESTDIR}/usr/lib/librumpvfs.a \
>>> -${DESTDIR}/usr/lib/librump.a \
>>> --Wl,--no-whole-archive
>>> +-Wl,--whole-archive -Wl,-Bstatic \
>>> + -lrumpvfs_nofifofs -lrumpvfs -lrump \
>>> +-Wl,-Bdynamic -Wl,--no-whole-archive
>>>
>>> LDADD+= -lrumpuser -lpthread
>>> DPADD+= ${LIBRUMPVFS} ${LIBRUMP} ${LIBRUMPUSER}
>>>
>>
>>
>>
>>
>
signature.asc
Description: OpenPGP digital signature
Re: CVS commit: src/tests/rump/modautoload
That is a completely different issue here. There are no linker tricks.
We want the module loader to include all the symbols any module
can require, this is why we load all the libraries.
While it is questionable if nofifofs is part of the base system or not,
this is the way it was before. Anyway it is easy enough to have it
both ways. If we ever grow a test that needs the real fifo stuff in
an autoloaded module, we can deal with that then.
christos
> On May 16, 2020, at 9:46 AM, Kamil Rytarowski wrote:
>
> Signed PGP part
> On 16.05.2020 14:54, Christos Zoulas wrote:
>> Module Name: src
>> Committed By:christos
>> Date:Sat May 16 12:54:27 UTC 2020
>>
>> Modified Files:
>> src/tests/rump/modautoload: Makefile
>>
>> Log Message:
>> Do the same thing with linker flags instead of directly specifying the
>> archives.
>>
>>
>
> Is there chance to rename the fifo symbols instead of using linker tricks?
>
> I'm also not entirely sure that this will be compatible with sanitizers
> (and C++ with the ODR rule) at this point.
>
>> To generate a diff of this commit:
>> cvs rdiff -u -r1.10 -r1.11 src/tests/rump/modautoload/Makefile
>>
>> Please note that diffs are not public domain; they are subject to the
>> copyright notices on the relevant files.
>>
>>
>> Modified files:
>>
>> Index: src/tests/rump/modautoload/Makefile
>> diff -u src/tests/rump/modautoload/Makefile:1.10
>> src/tests/rump/modautoload/Makefile:1.11
>> --- src/tests/rump/modautoload/Makefile:1.10 Sat May 16 08:44:42 2020
>> +++ src/tests/rump/modautoload/Makefile Sat May 16 08:54:27 2020
>> @@ -1,4 +1,4 @@
>> -# $NetBSD: Makefile,v 1.10 2020/05/16 12:44:42 christos Exp $
>> +# $NetBSD: Makefile,v 1.11 2020/05/16 12:54:27 christos Exp $
>> #
>>
>> .include
>> @@ -15,11 +15,9 @@ SRCS.t_modautoload+= t_modautoload.c
>> # subdirectory -- otherwise the LDADD lines would get a little hairy.
>> LDFLAGS+=-Wl,-E
>> LDADD+= \
>> --Wl,--whole-archive \
>> -${DESTDIR}/usr/lib/librumpvfs_nofifofs.a \
>> -${DESTDIR}/usr/lib/librumpvfs.a \
>> -${DESTDIR}/usr/lib/librump.a \
>> --Wl,--no-whole-archive
>> +-Wl,--whole-archive -Wl,-Bstatic \
>> +-lrumpvfs_nofifofs -lrumpvfs -lrump \
>> +-Wl,-Bdynamic -Wl,--no-whole-archive
>>
>> LDADD+= -lrumpuser -lpthread
>> DPADD+= ${LIBRUMPVFS} ${LIBRUMP} ${LIBRUMPUSER}
>>
>
>
>
>
signature.asc
Description: Message signed with OpenPGP
Re: CVS commit: src/tests/rump/modautoload
On 16.05.2020 14:54, Christos Zoulas wrote:
> Module Name: src
> Committed By: christos
> Date: Sat May 16 12:54:27 UTC 2020
>
> Modified Files:
> src/tests/rump/modautoload: Makefile
>
> Log Message:
> Do the same thing with linker flags instead of directly specifying the
> archives.
>
>
Is there chance to rename the fifo symbols instead of using linker tricks?
I'm also not entirely sure that this will be compatible with sanitizers
(and C++ with the ODR rule) at this point.
> To generate a diff of this commit:
> cvs rdiff -u -r1.10 -r1.11 src/tests/rump/modautoload/Makefile
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
>
> Modified files:
>
> Index: src/tests/rump/modautoload/Makefile
> diff -u src/tests/rump/modautoload/Makefile:1.10
> src/tests/rump/modautoload/Makefile:1.11
> --- src/tests/rump/modautoload/Makefile:1.10 Sat May 16 08:44:42 2020
> +++ src/tests/rump/modautoload/Makefile Sat May 16 08:54:27 2020
> @@ -1,4 +1,4 @@
> -#$NetBSD: Makefile,v 1.10 2020/05/16 12:44:42 christos Exp $
> +#$NetBSD: Makefile,v 1.11 2020/05/16 12:54:27 christos Exp $
> #
>
> .include
> @@ -15,11 +15,9 @@ SRCS.t_modautoload+= t_modautoload.c
> # subdirectory -- otherwise the LDADD lines would get a little hairy.
> LDFLAGS+=-Wl,-E
> LDADD+= \
> --Wl,--whole-archive \
> -${DESTDIR}/usr/lib/librumpvfs_nofifofs.a \
> -${DESTDIR}/usr/lib/librumpvfs.a \
> -${DESTDIR}/usr/lib/librump.a \
> --Wl,--no-whole-archive
> +-Wl,--whole-archive -Wl,-Bstatic \
> + -lrumpvfs_nofifofs -lrumpvfs -lrump \
> +-Wl,-Bdynamic -Wl,--no-whole-archive
>
> LDADD+= -lrumpuser -lpthread
> DPADD+= ${LIBRUMPVFS} ${LIBRUMP} ${LIBRUMPUSER}
>
signature.asc
Description: OpenPGP digital signature
Re: CVS commit: src/tests/lib/libc/sys
On 15.05.2020 00:43, Taylor R Campbell wrote: >> Date: Thu, 14 May 2020 23:36:28 +0200 >> From: Kamil Rytarowski >> >> If a signal would not reach the child process (as it is ignored or >> masked+SA_IGNOREd) and it is not a crash signal, it is dropped. As I >> checked, it's the design in UNIX to overlook SIGCHLD signals in UNIX. >> Race free signals could be maybe possible, but with some design rethinking. > > I don't understand -- are you saying that if I mask SIGCHLD, e.g. with > sigprocmask(SIG_BLOCK), then because sigprop[SIGCHLD] has SA_IGNORE > set, any SIGCHLD signals will be discarded while I have it masked? > That's it. But it will be discarded only when there is no SIGCHLD signal handler installed. That's the case in the test. A debugger catches regular signals only (except crash related ones) when they reach the debuggee, > This can't be right, so either I misunderstood what you're saying, or > something else is afoot with the test that is making it flaky, or > there's a bug in the kernel. > It's a design. If we install a signal handler for SIGCHLD in the traced the test is very stable and we note SIGCHLD always, so there is no bug. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/lib/libc/sys
> Date: Thu, 14 May 2020 23:36:28 +0200 > From: Kamil Rytarowski > > If a signal would not reach the child process (as it is ignored or > masked+SA_IGNOREd) and it is not a crash signal, it is dropped. As I > checked, it's the design in UNIX to overlook SIGCHLD signals in UNIX. > Race free signals could be maybe possible, but with some design rethinking. I don't understand -- are you saying that if I mask SIGCHLD, e.g. with sigprocmask(SIG_BLOCK), then because sigprop[SIGCHLD] has SA_IGNORE set, any SIGCHLD signals will be discarded while I have it masked? This can't be right, so either I misunderstood what you're saying, or something else is afoot with the test that is making it flaky, or there's a bug in the kernel.
Re: CVS commit: src/tests/lib/libc/sys
On Thu, May 14, 2020 at 11:36:28PM +0200, Kamil Rytarowski wrote: > On 14.05.2020 23:02, Joerg Sonnenberger wrote: > > On Thu, May 14, 2020 at 07:21:35PM +, Kamil Rytarowski wrote: > >> Module Name: src > >> Committed By: kamil > >> Date: Thu May 14 19:21:35 UTC 2020 > >> > >> Modified Files: > >>src/tests/lib/libc/sys: t_ptrace_fork_wait.h > >> > >> Log Message: > >> Ignore interception of the SIGCHLD signals. > >> > >> SIGCHLD once blocked is discarded by the kernel as it has the > >> SA_IGNORE property. During the fork(2) operation all signals can be > >> shortly blocked and missed (unless there is a registered signal > >> handler in the traced child). This leads to a race in this test if > >> there would be an intention to catch SIGCHLD. > > > > This doesn't sound right. sigprocmask and pthread_sigmask shouldn't > > affect whether the signal handler is called, only when. Temporarily > > masking a signal is different from setting it to SIG_IGN. > > > > Joerg > > > > I was looking into it and as there is no signal handler for SIGCHLD, the > signal is discarded. Sure, but that doesn't really have anything to do with signal blocking. That's the part that is confusing. Joerg
Re: CVS commit: src/tests/lib/libc/sys
On 14.05.2020 23:02, Joerg Sonnenberger wrote: > On Thu, May 14, 2020 at 07:21:35PM +, Kamil Rytarowski wrote: >> Module Name: src >> Committed By:kamil >> Date:Thu May 14 19:21:35 UTC 2020 >> >> Modified Files: >> src/tests/lib/libc/sys: t_ptrace_fork_wait.h >> >> Log Message: >> Ignore interception of the SIGCHLD signals. >> >> SIGCHLD once blocked is discarded by the kernel as it has the >> SA_IGNORE property. During the fork(2) operation all signals can be >> shortly blocked and missed (unless there is a registered signal >> handler in the traced child). This leads to a race in this test if >> there would be an intention to catch SIGCHLD. > > This doesn't sound right. sigprocmask and pthread_sigmask shouldn't > affect whether the signal handler is called, only when. Temporarily > masking a signal is different from setting it to SIG_IGN. > > Joerg > I was looking into it and as there is no signal handler for SIGCHLD, the signal is discarded. Another approach to fix the race would be to: 1. Add SIGCHLD signal handler in the traced child. 2. Explicitly masking SIGCHLD in the traced child. If a signal would not reach the child process (as it is ignored or masked+SA_IGNOREd) and it is not a crash signal, it is dropped. As I checked, it's the design in UNIX to overlook SIGCHLD signals in UNIX. Race free signals could be maybe possible, but with some design rethinking. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/lib/libc/sys
On Thu, May 14, 2020 at 07:21:35PM +, Kamil Rytarowski wrote: > Module Name: src > Committed By: kamil > Date: Thu May 14 19:21:35 UTC 2020 > > Modified Files: > src/tests/lib/libc/sys: t_ptrace_fork_wait.h > > Log Message: > Ignore interception of the SIGCHLD signals. > > SIGCHLD once blocked is discarded by the kernel as it has the > SA_IGNORE property. During the fork(2) operation all signals can be > shortly blocked and missed (unless there is a registered signal > handler in the traced child). This leads to a race in this test if > there would be an intention to catch SIGCHLD. This doesn't sound right. sigprocmask and pthread_sigmask shouldn't affect whether the signal handler is called, only when. Temporarily masking a signal is different from setting it to SIG_IGN. Joerg
Re: CVS commit: src/tests/lib/libc/sys
Date:Mon, 11 May 2020 13:47:45 +0200 From:Kamil Rytarowski Message-ID: <54178983-82d1-df3d-fd54-549a6c73f...@gmx.com> | The only purpose of the test is to check whether misaligned program | counter can crash the kernel (it can for NetBSD/sparc). Later, if a | process dies or runs is not important, thus it is being killed. That's all fine. | A process can disappear after dying and before reappearing as a zombie. There's a state between running and dead (zombie), that's correct - but it really doesn't matter, once the process ceases to be alive, it is beyond killing any more. | This is not a bug, but a predicted race. Yes, that's what I said, and that's fine too. | Doing the kill once (and missing the process) is still possibly enough, | but correcting it with SIGKILL does not cost. No, there is no problem with doing the SIGKILL, but if fails for the above reason, there's absolutely no point trying again. The process is gone, it isn't coming back. It doesn't need killing. But even if there was a reason to try again (there isn't), one more attempt would do - inserting an infinite loop is folly. But I see that you have fixed it, that's good, what's there now looks much better. Thanks. kre
Re: CVS commit: src/tests/lib/libc/sys
On 11.05.2020 13:35, Robert Elz wrote: > Date:Mon, 11 May 2020 11:03:15 + > From:"Kamil Rytarowski" > Message-ID: <2020050315.54b13f...@cvs.netbsd.org> > > | Do not fail when trying to kill a dying process > | > | A dying process can disappear for a while. Rather than aborting, retry > | sending SIGKILL to it. > > I don't understand this ... a process should never be able to > disappear and then reappear (not in any way). If a SIGKILL (or > ptrace(PT_KILL) fails with a "no such process" error, then repeating > it won't (or shouldn't) help - if it does, there's a kernel bug that > needs fixing (and it is OK for the test to fail until that happens.) > > Further, if the reason for this failure is that the process is > dying, you probably never needed the kill in the first place (and > no, I don't mean it should be deleted - the parent is unlikely to > know the state of the child, so killing it, if that is what is needed > is the right thing to do ... just that if the kill fails because you > were too late issuing it, it isn't an error, just a race that you lost, > and certainly shouldn't be repeated). > > But more than that, adding an infinite loop to the test, where you keep > doing the kill forever until it succeeds, or errno somehow stops being > ESRCH looks like a recipe for disaster. > > Just do the kill once, ignore the error if it is ESRCH (and probably > also ECHILD) report other errors as failures. > > kre > The only purpose of the test is to check whether misaligned program counter can crash the kernel (it can for NetBSD/sparc). Later, if a process dies or runs is not important, thus it is being killed. A process can disappear after dying and before reappearing as a zombie. This is not a bug, but a predicted race. We already discussed it in the past, whether to return the same process multiple times or overlook it for a while during the transition dying->zombie. Once an entity died it disappeared so the same is true for a process. Doing the kill once (and missing the process) is still possibly enough, but correcting it with SIGKILL does not cost. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/lib/libc/sys
Date:Mon, 11 May 2020 11:03:15 + From:"Kamil Rytarowski" Message-ID: <2020050315.54b13f...@cvs.netbsd.org> | Do not fail when trying to kill a dying process | | A dying process can disappear for a while. Rather than aborting, retry | sending SIGKILL to it. I don't understand this ... a process should never be able to disappear and then reappear (not in any way). If a SIGKILL (or ptrace(PT_KILL) fails with a "no such process" error, then repeating it won't (or shouldn't) help - if it does, there's a kernel bug that needs fixing (and it is OK for the test to fail until that happens.) Further, if the reason for this failure is that the process is dying, you probably never needed the kill in the first place (and no, I don't mean it should be deleted - the parent is unlikely to know the state of the child, so killing it, if that is what is needed is the right thing to do ... just that if the kill fails because you were too late issuing it, it isn't an error, just a race that you lost, and certainly shouldn't be repeated). But more than that, adding an infinite loop to the test, where you keep doing the kill forever until it succeeds, or errno somehow stops being ESRCH looks like a recipe for disaster. Just do the kill once, ignore the error if it is ESRCH (and probably also ECHILD) report other errors as failures. kre
Re: CVS commit: src/tests/lib/libc/sys
On 24.04.2020 05:25, Jason R Thorpe wrote: > Module Name: src > Committed By: thorpej > Date: Fri Apr 24 03:25:20 UTC 2020 > > Modified Files: > src/tests/lib/libc/sys: t_ptrace_wait.c t_ptrace_x86_wait.h > > Log Message: > Update for new LWP behavior -- as of 9.99.59, the LWP ID of a single-LWP > process is the PID, not 1. > Thanks. These tests shall not rely on specific LWP numbers and I will remove the asserts. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/lib/libc/sys
In article <5e528f7a-147a-23e7-46da-6b02d76e5...@gmx.com>, Kamil Rytarowski wrote: >-=-=-=-=-=- >-=-=-=-=-=- > >On 07.03.2020 15:53, Christos Zoulas wrote: >> Module Name: src >> Committed By:christos >> Date:Sat Mar 7 14:53:14 UTC 2020 >> >> Modified Files: >> src/tests/lib/libc/sys: t_ptrace_wait.c t_ptrace_wait.h >> >> Log Message: >> Try to fix the build. This is why all those inlines should really be in a >> separate file as regular function. The code is too large and hard to manage >> this way, and only increases in complexity as time goes by. >> >> > >What build configuration was broken? All of the evbarm ones since t_ptrace_sigchld.c was not including ieefp.h http://releng.netbsd.org/builds/HEAD/202003070040Z/evbarm-earmhfeb.build.failed christos
Re: CVS commit: src/tests/lib/libc/sys
On 07.03.2020 15:53, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Sat Mar 7 14:53:14 UTC 2020 > > Modified Files: > src/tests/lib/libc/sys: t_ptrace_wait.c t_ptrace_wait.h > > Log Message: > Try to fix the build. This is why all those inlines should really be in a > separate file as regular function. The code is too large and hard to manage > this way, and only increases in complexity as time goes by. > > What build configuration was broken? signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/lib/libc/gen
On 24.02.2020 20:32, Christos Zoulas wrote:
> In article <20200221222550.325a6f...@cvs.netbsd.org>,
> Kamil Rytarowski wrote:
>> -=-=-=-=-=-
>>
>> Module Name: src
>> Committed By:kamil
>> Date:Fri Feb 21 22:25:50 UTC 2020
>>
>> Modified Files:
>> src/tests/lib/libc/gen: t_siginfo.c
>>
>> Log Message:
>> Mark division by 0 as expected in sigfpe_int
>>
>> Disable ubsan instrumentation on the operation.
>
>> +#if defined(__clang__)
>> +__attribute__((no_sanitize("undefined")))
>> +#else
>> +__attribute__((no_sanitize_undefined))
>> +#endif
>> +static long int
>> +sigfpe_int_division(long int a, long int b)
>> +{
>> +
>> +return a / b;
>> +}
>
> Have you tested this? I recall I needed to make it a separate function...
>
> christos
>
It was tested and kind of worked, but I decided to fully disable the
tests and revert this change. The sanitizers can change the logic here
to avoid division by zero and interfere with crash signals. It's not
worth the effort to force them to be compatible with all the tests.
signature.asc
Description: OpenPGP digital signature
Re: CVS commit: src/tests/lib/libc/gen
In article <20200221222550.325a6f...@cvs.netbsd.org>,
Kamil Rytarowski wrote:
>-=-=-=-=-=-
>
>Module Name: src
>Committed By: kamil
>Date: Fri Feb 21 22:25:50 UTC 2020
>
>Modified Files:
> src/tests/lib/libc/gen: t_siginfo.c
>
>Log Message:
>Mark division by 0 as expected in sigfpe_int
>
>Disable ubsan instrumentation on the operation.
>+#if defined(__clang__)
>+__attribute__((no_sanitize("undefined")))
>+#else
>+__attribute__((no_sanitize_undefined))
>+#endif
>+static long int
>+sigfpe_int_division(long int a, long int b)
>+{
>+
>+ return a / b;
>+}
Have you tested this? I recall I needed to make it a separate function...
christos
Re: CVS commit: src/tests/lib/libc/gen
On 24.02.2020 20:29, Christos Zoulas wrote:
> In article <20200222191457.87687f...@cvs.netbsd.org>,
> Kamil Rytarowski wrote:
>> -=-=-=-=-=-
>>
>> Module Name: src
>> Committed By:kamil
>> Date:Sat Feb 22 19:14:57 UTC 2020
>>
>> Modified Files:
>> src/tests/lib/libc/gen: Makefile
>>
>> Log Message:
>> Update t_siginfo.c build rules
>>
>> Add logic for MKSANITIZER/MKLIBCSANITIZER checks.
>>
>>
>> To generate a diff of this commit:
>> cvs rdiff -u -r1.53 -r1.54 src/tests/lib/libc/gen/Makefile
>>
>> Please note that diffs are not public domain; they are subject to the
>> copyright notices on the relevant files.
>>
>>
>> -=-=-=-=-=-
>>
>> Modified files:
>>
>> Index: src/tests/lib/libc/gen/Makefile
>> diff -u src/tests/lib/libc/gen/Makefile:1.53
>> src/tests/lib/libc/gen/Makefile:1.54
>> --- src/tests/lib/libc/gen/Makefile:1.53 Fri Apr 26 19:17:05 2019
>> +++ src/tests/lib/libc/gen/Makefile Sat Feb 22 19:14:57 2020
>> @@ -1,4 +1,4 @@
>> -# $NetBSD: Makefile,v 1.53 2019/04/26 19:17:05 maya Exp $
>> +# $NetBSD: Makefile,v 1.54 2020/02/22 19:14:57 kamil Exp $
>>
>> .include
>>
>> @@ -39,6 +39,10 @@ TESTS_C+= t_time
>> TESTS_C+=t_ttyname
>> TESTS_C+=t_vis
>>
>> +.if ${MKSANITIZER:Uno} != "yes" && ${MKLIBCSANITIZER:Uno} != "yes"
>> +COPTS.t_siginfo.c+= -DENABLE_TESTS
>> +.endif
>> +
>> CPPFLAGS.t_siginfo.c+=-D__TEST_FENV
>> COPTS.t_fpsetround.c+=${${ACTIVE_CC} == "gcc":? -frounding-math :}
>
> This should be backwards. -DDISABLE_TESTS for the sanitizers and nothing
> in the regular build case. Isn't there a cpp macro for the sanitizers?
>
Not a global one, but I can add it in our headers and switch to it,
avoiding the logic in Makefiles.
I still need to switch h_segv.c.
> christos
>
signature.asc
Description: OpenPGP digital signature
Re: CVS commit: src/tests/lib/libc/gen
In article <20200222191457.87687f...@cvs.netbsd.org>,
Kamil Rytarowski wrote:
>-=-=-=-=-=-
>
>Module Name: src
>Committed By: kamil
>Date: Sat Feb 22 19:14:57 UTC 2020
>
>Modified Files:
> src/tests/lib/libc/gen: Makefile
>
>Log Message:
>Update t_siginfo.c build rules
>
>Add logic for MKSANITIZER/MKLIBCSANITIZER checks.
>
>
>To generate a diff of this commit:
>cvs rdiff -u -r1.53 -r1.54 src/tests/lib/libc/gen/Makefile
>
>Please note that diffs are not public domain; they are subject to the
>copyright notices on the relevant files.
>
>
>-=-=-=-=-=-
>
>Modified files:
>
>Index: src/tests/lib/libc/gen/Makefile
>diff -u src/tests/lib/libc/gen/Makefile:1.53
>src/tests/lib/libc/gen/Makefile:1.54
>--- src/tests/lib/libc/gen/Makefile:1.53 Fri Apr 26 19:17:05 2019
>+++ src/tests/lib/libc/gen/MakefileSat Feb 22 19:14:57 2020
>@@ -1,4 +1,4 @@
>-# $NetBSD: Makefile,v 1.53 2019/04/26 19:17:05 maya Exp $
>+# $NetBSD: Makefile,v 1.54 2020/02/22 19:14:57 kamil Exp $
>
> .include
>
>@@ -39,6 +39,10 @@ TESTS_C+= t_time
> TESTS_C+= t_ttyname
> TESTS_C+= t_vis
>
>+.if ${MKSANITIZER:Uno} != "yes" && ${MKLIBCSANITIZER:Uno} != "yes"
>+COPTS.t_siginfo.c+= -DENABLE_TESTS
>+.endif
>+
> CPPFLAGS.t_siginfo.c+=-D__TEST_FENV
> COPTS.t_fpsetround.c+=${${ACTIVE_CC} == "gcc":? -frounding-math :}
This should be backwards. -DDISABLE_TESTS for the sanitizers and nothing
in the regular build case. Isn't there a cpp macro for the sanitizers?
christos
Re: CVS commit: src/tests/modules
On 22.02.2020 15:54, Paul Goyette wrote: > On Sat, 22 Feb 2020, Kamil Rytarowski wrote: > > While there, it would be good to implement modctl(MODCTL_MODSTAT, > ) to check whether a specific module is loaded into the kernel > and retrieve modstat_t describing it. > > modstat_t m; > strlcpy(_name, "haxm", MAXMODNAME); > if (modctl(MODCTL_MODSTAT, ) == -1) > err(EXIT_FAILURE, "modctl: haxm"); > > I have got use-cases for these checks and I envision their wider usage > in future. We already have 3 use-cases in ATF tests. I can probably do this fairly quickly. But I'll have to look closer at the argument/result passing, especially WRT the module's list of "required" modules. >>> >>> Thinking a bit more, it's probably easiest just to retrieve the entire >>> list of modules with modctl(MODCTL_STAT, ...) and then scan the returned >>> list and compare against ms_name, as is done in modstat(8). >>> >>> Before I invest much time in this, I'd appreciate other opinions on >>> whether a new option is necessary/desirable. >>> >>> >> >> Performance is probably not critical so it sounds fine. >> >> I would like to have at least get_modstat_info() from t_modctl.c in >> libutil. > > Sure that seems reasonable to me. > > Assuming that noone else objects, please feel free to move it. I > think we should also update the test program to use the new libutil > version (rather than duplicating the code). Also update the libutil > man page? > > I guess that the return type of get_modstat_info() should be changed > to int rather than bool? And that it shouldn't directly print error > messages? :) > > I will do it and share a patch. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/modules
On Sat, 22 Feb 2020, Kamil Rytarowski wrote: While there, it would be good to implement modctl(MODCTL_MODSTAT, ) to check whether a specific module is loaded into the kernel and retrieve modstat_t describing it. modstat_t m; strlcpy(_name, "haxm", MAXMODNAME); if (modctl(MODCTL_MODSTAT, ) == -1) err(EXIT_FAILURE, "modctl: haxm"); I have got use-cases for these checks and I envision their wider usage in future. We already have 3 use-cases in ATF tests. I can probably do this fairly quickly.?? But I'll have to look closer at the argument/result passing, especially WRT the module's list of "required" modules. Thinking a bit more, it's probably easiest just to retrieve the entire list of modules with modctl(MODCTL_STAT, ...) and then scan the returned list and compare against ms_name, as is done in modstat(8). Before I invest much time in this, I'd appreciate other opinions on whether a new option is necessary/desirable. Performance is probably not critical so it sounds fine. I would like to have at least get_modstat_info() from t_modctl.c in libutil. Sure that seems reasonable to me. Assuming that noone else objects, please feel free to move it. I think we should also update the test program to use the new libutil version (rather than duplicating the code). Also update the libutil man page? I guess that the return type of get_modstat_info() should be changed to int rather than bool? And that it shouldn't directly print error messages? :) ++--+---+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | ++--+---+
Re: CVS commit: src/tests/modules
On 22.02.2020 15:32, Paul Goyette wrote: > On Sat, 22 Feb 2020, Paul Goyette wrote: > >>> While there, it would be good to implement modctl(MODCTL_MODSTAT, >>> ) to check whether a specific module is loaded into the kernel >>> and retrieve modstat_t describing it. >>> >>> modstat_t m; >>> strlcpy(_name, "haxm", MAXMODNAME); >>> if (modctl(MODCTL_MODSTAT, ) == -1) >>> err(EXIT_FAILURE, "modctl: haxm"); >>> >>> I have got use-cases for these checks and I envision their wider usage >>> in future. We already have 3 use-cases in ATF tests. >> >> I can probably do this fairly quickly. But I'll have to look closer >> at the argument/result passing, especially WRT the module's list of >> "required" modules. > > Thinking a bit more, it's probably easiest just to retrieve the entire > list of modules with modctl(MODCTL_STAT, ...) and then scan the returned > list and compare against ms_name, as is done in modstat(8). > > Before I invest much time in this, I'd appreciate other opinions on > whether a new option is necessary/desirable. > > Performance is probably not critical so it sounds fine. I would like to have at least get_modstat_info() from t_modctl.c in libutil. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/modules
On Sat, 22 Feb 2020, Paul Goyette wrote: While there, it would be good to implement modctl(MODCTL_MODSTAT, ) to check whether a specific module is loaded into the kernel and retrieve modstat_t describing it. modstat_t m; strlcpy(_name, "haxm", MAXMODNAME); if (modctl(MODCTL_MODSTAT, ) == -1) err(EXIT_FAILURE, "modctl: haxm"); I have got use-cases for these checks and I envision their wider usage in future. We already have 3 use-cases in ATF tests. I can probably do this fairly quickly. But I'll have to look closer at the argument/result passing, especially WRT the module's list of "required" modules. Thinking a bit more, it's probably easiest just to retrieve the entire list of modules with modctl(MODCTL_STAT, ...) and then scan the returned list and compare against ms_name, as is done in modstat(8). Before I invest much time in this, I'd appreciate other opinions on whether a new option is necessary/desirable. ++--+---+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | ++--+---+
Re: CVS commit: src/tests/modules
On Sat, 22 Feb 2020, Kamil Rytarowski wrote: I have got no opinion. Please rearrange the directories as needed. It's too much bother for now to move things around. But for future changes it would be good to put new "helper" modules in the same area as the tests being helped. While there, it would be good to implement modctl(MODCTL_MODSTAT, ) to check whether a specific module is loaded into the kernel and retrieve modstat_t describing it. modstat_t m; strlcpy(_name, "haxm", MAXMODNAME); if (modctl(MODCTL_MODSTAT, ) == -1) err(EXIT_FAILURE, "modctl: haxm"); I have got use-cases for these checks and I envision their wider usage in future. We already have 3 use-cases in ATF tests. I can probably do this fairly quickly. But I'll have to look closer at the argument/result passing, especially WRT the module's list of "required" modules. ++--+---+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | ++--+---+
Re: CVS commit: src/tests/modules
I have got no opinion. Please rearrange the directories as needed. While there, it would be good to implement modctl(MODCTL_MODSTAT, ) to check whether a specific module is loaded into the kernel and retrieve modstat_t describing it. modstat_t m; strlcpy(_name, "haxm", MAXMODNAME); if (modctl(MODCTL_MODSTAT, ) == -1) err(EXIT_FAILURE, "modctl: haxm"); I have got use-cases for these checks and I envision their wider usage in future. We already have 3 use-cases in ATF tests. On 22.02.2020 04:41, Paul Goyette wrote: > OK, I over-reacted and didn't completely read the original commit > message. > > The t_builtin.c stuff is indeed a test-of-module-functionality > so it does belong here. > > But some of the other stuff here does not belong, such as the > threadpool, fetchstore, and kcov stuff. As far as I can see, > those all belong somewhere else, probably in the tests/sys/... > hierarchy. > > Anyway, my apologies for over-reacting to this commit. And > thanks to riastradh@ for pointing this out (on IRC). > > > > On Fri, 21 Feb 2020, Paul Goyette wrote: > >> Really, the tests/modules directory should be only used for tests-that- >> relate-to-module-functionality. It should NOT be used for modules- >> that-support-tests-of-other-functionality. >> >> In the future, please do not put support modules here; put them in the >> samae place as the tests that they support. >> >> >> On Sat, 22 Feb 2020, Kamil Rytarowski wrote: >> >>> Module Name: src >>> Committed By: kamil >>> Date: Sat Feb 22 00:18:55 UTC 2020 >>> >>> Modified Files: >>> src/tests/modules: t_builtin.c >>> >>> Log Message: >>> Avoid undefined behavior in disabledstat >>> >>> t_builtin.c:174:16, member access within misaligned address >>> 0x741271c25004 >>> for type 'struct modstat_t' >>> >>> t_builtin.c:175:4, member access within misaligned address >>> 0x741271c251c4 >>> for type 'struct modstat_t' >>> >>> >>> To generate a diff of this commit: >>> cvs rdiff -u -r1.4 -r1.5 src/tests/modules/t_builtin.c >>> >>> Please note that diffs are not public domain; they are subject to the >>> copyright notices on the relevant files. >>> >>> >>> !DSPAM:5e5073af66043393299806! >>> >>> >> >> ++--+---+ >> | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | >> | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | >> | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | >> ++--+---+ >> > > ++--+---+ > | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | > | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | > | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | > ++--+---+ signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/modules
OK, I over-reacted and didn't completely read the original commit message. The t_builtin.c stuff is indeed a test-of-module-functionality so it does belong here. But some of the other stuff here does not belong, such as the threadpool, fetchstore, and kcov stuff. As far as I can see, those all belong somewhere else, probably in the tests/sys/... hierarchy. Anyway, my apologies for over-reacting to this commit. And thanks to riastradh@ for pointing this out (on IRC). On Fri, 21 Feb 2020, Paul Goyette wrote: Really, the tests/modules directory should be only used for tests-that- relate-to-module-functionality. It should NOT be used for modules- that-support-tests-of-other-functionality. In the future, please do not put support modules here; put them in the samae place as the tests that they support. On Sat, 22 Feb 2020, Kamil Rytarowski wrote: Module Name:src Committed By: kamil Date: Sat Feb 22 00:18:55 UTC 2020 Modified Files: src/tests/modules: t_builtin.c Log Message: Avoid undefined behavior in disabledstat t_builtin.c:174:16, member access within misaligned address 0x741271c25004 for type 'struct modstat_t' t_builtin.c:175:4, member access within misaligned address 0x741271c251c4 for type 'struct modstat_t' To generate a diff of this commit: cvs rdiff -u -r1.4 -r1.5 src/tests/modules/t_builtin.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. !DSPAM:5e5073af66043393299806! ++--+---+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | ++--+---+ ++--+---+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | ++--+---+
Re: CVS commit: src/tests/modules
Really, the tests/modules directory should be only used for tests-that- relate-to-module-functionality. It should NOT be used for modules- that-support-tests-of-other-functionality. In the future, please do not put support modules here; put them in the samae place as the tests that they support. On Sat, 22 Feb 2020, Kamil Rytarowski wrote: Module Name:src Committed By: kamil Date: Sat Feb 22 00:18:55 UTC 2020 Modified Files: src/tests/modules: t_builtin.c Log Message: Avoid undefined behavior in disabledstat t_builtin.c:174:16, member access within misaligned address 0x741271c25004 for type 'struct modstat_t' t_builtin.c:175:4, member access within misaligned address 0x741271c251c4 for type 'struct modstat_t' To generate a diff of this commit: cvs rdiff -u -r1.4 -r1.5 src/tests/modules/t_builtin.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. !DSPAM:5e5073af66043393299806! ++--+---+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | ++--+---+
Re: CVS commit: src/tests/lib/libc/gen
On 21.02.2020 23:53, matthew green wrote:
>> Disable ubsan instrumentation on the operation.
>
> +#if defined(__clang__)
> +__attribute__((no_sanitize("undefined")))
> +#else
> +__attribute__((no_sanitize_undefined))
> +#endif
>
> can we get a __disable_sanitizer or something i cdefs.h?
>
>
> .mrg.
>
I will do it.
signature.asc
Description: OpenPGP digital signature
re: CVS commit: src/tests/lib/libc/gen
> Disable ubsan instrumentation on the operation.
+#if defined(__clang__)
+__attribute__((no_sanitize("undefined")))
+#else
+__attribute__((no_sanitize_undefined))
+#endif
can we get a __disable_sanitizer or something i cdefs.h?
.mrg.
Re: CVS commit: src/tests/lib/libc/sys
On 18.02.2020 16:57, Christos Zoulas wrote: > In article <20200213152742.081a9f...@cvs.netbsd.org>, > MichaŠGórny wrote: >> -=-=-=-=-=- >> >> Module Name: src >> Committed By:mgorny >> Date:Thu Feb 13 15:27:41 UTC 2020 >> >> Modified Files: >> src/tests/lib/libc/sys: t_ptrace_wait.c >> >> Log Message: >> Enable combined breakpoint, watchpoint and signal tests > > Let's split this file up. The name does not reflect anymore what this > is testing and it has become more than 9000 lines long. Because of the > complexity it keeps breaking the build and because of the size it makes > fixing it awkward. Kamil/Michal, can you please work on this? > [ for example the llvm builds are currently broken ] > > Thanks, > > christos > I will do it. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/lib/libc/sys
In article <20200213152742.081a9f...@cvs.netbsd.org>, MichaŠGórny wrote: >-=-=-=-=-=- > >Module Name: src >Committed By: mgorny >Date: Thu Feb 13 15:27:41 UTC 2020 > >Modified Files: > src/tests/lib/libc/sys: t_ptrace_wait.c > >Log Message: >Enable combined breakpoint, watchpoint and signal tests Let's split this file up. The name does not reflect anymore what this is testing and it has become more than 9000 lines long. Because of the complexity it keeps breaking the build and because of the size it makes fixing it awkward. Kamil/Michal, can you please work on this? [ for example the llvm builds are currently broken ] Thanks, christos
Re: CVS commit: src/tests/lib/libc/sys
In article <20200213114904.ga30...@bec.de>, Joerg Sonnenberger wrote: >On Thu, Feb 13, 2020 at 10:50:19AM +0100, Joerg Sonnenberger wrote: >> On Wed, Feb 12, 2020 at 09:53:46PM -0500, Christos Zoulas wrote: >> > Module Name: src >> > Committed By: christos >> > Date: Thu Feb 13 02:53:46 UTC 2020 >> > >> > Modified Files: >> >src/tests/lib/libc/sys: t_ptrace_x86_wait.h >> > >> > Log Message: >> > Turn off optimization on a function which contains constant labels. >> > The optimizer splits it and we end up with 2 copies and duplicate symbols. >> >> Why not just turn them into local labels instead? > >Alternatively, suffixing them with %= would create unique labels. I was looking for that :-) christos
Re: CVS commit: src/tests/lib/libc/sys
In article <20200213095019.ga28...@bec.de>, Joerg Sonnenberger wrote: >On Wed, Feb 12, 2020 at 09:53:46PM -0500, Christos Zoulas wrote: >> Module Name: src >> Committed By:christos >> Date:Thu Feb 13 02:53:46 UTC 2020 >> >> Modified Files: >> src/tests/lib/libc/sys: t_ptrace_x86_wait.h >> >> Log Message: >> Turn off optimization on a function which contains constant labels. >> The optimizer splits it and we end up with 2 copies and duplicate symbols. > >Why not just turn them into local labels instead? You mean 1f etc? I was not sure if that would work in the symbol defined case. christos
Re: CVS commit: src/tests/lib/libc/sys
On Thu, Feb 13, 2020 at 10:50:19AM +0100, Joerg Sonnenberger wrote: > On Wed, Feb 12, 2020 at 09:53:46PM -0500, Christos Zoulas wrote: > > Module Name:src > > Committed By: christos > > Date: Thu Feb 13 02:53:46 UTC 2020 > > > > Modified Files: > > src/tests/lib/libc/sys: t_ptrace_x86_wait.h > > > > Log Message: > > Turn off optimization on a function which contains constant labels. > > The optimizer splits it and we end up with 2 copies and duplicate symbols. > > Why not just turn them into local labels instead? Alternatively, suffixing them with %= would create unique labels. Joerg
Re: CVS commit: src/tests/lib/libc/sys
On Wed, Feb 12, 2020 at 09:53:46PM -0500, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Thu Feb 13 02:53:46 UTC 2020 > > Modified Files: > src/tests/lib/libc/sys: t_ptrace_x86_wait.h > > Log Message: > Turn off optimization on a function which contains constant labels. > The optimizer splits it and we end up with 2 copies and duplicate symbols. Why not just turn them into local labels instead? Joerg
re: CVS commit: src/tests/lib/libarchive
"Martin Husemann" writes: > Module Name: src > Committed By: martin > Date: Tue Jan 28 18:18:32 UTC 2020 > > Modified Files: > src/tests/lib/libarchive: t_libarchive.sh > > Log Message: > Bump timeout to 3600 - the libarchive tests take quite a while to > complete (on a nearly 1 GHz dual armv7 machine it takes more than 700s) this seems awfully excessive. can we tone down the libarchive tests to something reasonable for netbsd? .mrg.
Re: CVS commit: src/tests/usr.bin
On 14.10.2019 05:47, Jason High wrote: Module Name:src Committed By: jhigh Date: Mon Oct 14 03:47:20 UTC 2019 Modified Files: src/tests/usr.bin: Makefile Added Files: src/tests/usr.bin/argon2: Atffile Makefile t_argon2.sh Log Message: adding argon2 tests diff -u /dev/null src/tests/usr.bin/argon2/Atffile:1.1 --- /dev/null Mon Oct 14 03:47:20 2019 +++ src/tests/usr.bin/argon2/AtffileMon Oct 14 03:47:20 2019 @@ -0,0 +1,7 @@ +Content-Type: application/X-atf-atffile; version="1" + +# Automatically generated by bsd.test.mk. + +prop: test-suite = "NetBSD" + +tp: t_argon2 This file shall not be added, please delete it. On the other hand we need to add entries in distrib sets.
Re: CVS commit: src/tests/kernel
While there, it does not build for me: /usr/src/tests/kernel/h_fexecve.c: In function 'main': /usr/src/tests/kernel/h_fexecve.c:48:6: error: implicit declaration of function 'fexecve'; did you mean 'execve'? [-Werror=implicit-function-declaration] if (fexecve(fd, args, NULL) == -1) ^~~ execve cc1: all warnings being treated as errors On 15.09.2019 22:37, Kamil Rytarowski wrote: > I have got no opinion, but merging them is good. Personally I prefer > src/libc/* path as fexecve(2) is a libc public symbol. > > On 15.09.2019 20:06, Christos Zoulas wrote: >> The tests are a different. Should we keep them both, or try to merge them? >> I think that merging them is probably better. It is also the case that >> perhaps >> we need to get rid of the kernel tests directory and move them to the >> respective bin and lib directories to avoid confusion? >> >> christos >> >>> On Sep 15, 2019, at 1:02 PM, Kamil Rytarowski wrote: >>> >>> Signed PGP part >>> On 15.09.2019 18:53, Christos Zoulas wrote: Module Name: src Committed By: christos Date: Sun Sep 15 16:53:58 UTC 2019 Modified Files: src/tests/kernel: Makefile Added Files: src/tests/kernel: h_fexecve.c t_fexecve.sh Log Message: Add tests for fexecve(2) >>> >>> For the reference, there were already tests in: >>> >>> ./lib/libc/c063/t_fexecve >>> >>> >>> >> > > signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/kernel
I have got no opinion, but merging them is good. Personally I prefer src/libc/* path as fexecve(2) is a libc public symbol. On 15.09.2019 20:06, Christos Zoulas wrote: > The tests are a different. Should we keep them both, or try to merge them? > I think that merging them is probably better. It is also the case that perhaps > we need to get rid of the kernel tests directory and move them to the > respective bin and lib directories to avoid confusion? > > christos > >> On Sep 15, 2019, at 1:02 PM, Kamil Rytarowski wrote: >> >> Signed PGP part >> On 15.09.2019 18:53, Christos Zoulas wrote: >>> Module Name:src >>> Committed By: christos >>> Date: Sun Sep 15 16:53:58 UTC 2019 >>> >>> Modified Files: >>> src/tests/kernel: Makefile >>> Added Files: >>> src/tests/kernel: h_fexecve.c t_fexecve.sh >>> >>> Log Message: >>> Add tests for fexecve(2) >> >> For the reference, there were already tests in: >> >> ./lib/libc/c063/t_fexecve >> >> >> > signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/kernel
The tests are a different. Should we keep them both, or try to merge them? I think that merging them is probably better. It is also the case that perhaps we need to get rid of the kernel tests directory and move them to the respective bin and lib directories to avoid confusion? christos > On Sep 15, 2019, at 1:02 PM, Kamil Rytarowski wrote: > > Signed PGP part > On 15.09.2019 18:53, Christos Zoulas wrote: >> Module Name: src >> Committed By:christos >> Date:Sun Sep 15 16:53:58 UTC 2019 >> >> Modified Files: >> src/tests/kernel: Makefile >> Added Files: >> src/tests/kernel: h_fexecve.c t_fexecve.sh >> >> Log Message: >> Add tests for fexecve(2) > > For the reference, there were already tests in: > > ./lib/libc/c063/t_fexecve > > >
Re: CVS commit: src/tests/kernel
On 15.09.2019 18:53, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Sun Sep 15 16:53:58 UTC 2019 > > Modified Files: > src/tests/kernel: Makefile > Added Files: > src/tests/kernel: h_fexecve.c t_fexecve.sh > > Log Message: > Add tests for fexecve(2) For the reference, there were already tests in: ./lib/libc/c063/t_fexecve signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/net
On Fri, Aug 23, 2019 at 3:51 PM Martin Husemann wrote: > > On Fri, Aug 23, 2019 at 01:41:58PM +0900, Ryota Ozaki wrote: > > A workaround for the issue is: > > cp /usr/bin/vmstat ./vmstat > > $HIJACKING ./vmstat > > rm -f ./vmstat > > > > It's awkward but it's reasonable for now. A proper fix would > > be to stop using kvm(3) for vmstat and drop the sgid bit from > > the binary. > > Wow, good catch! > > > I guess on most anita environments leak checks pass just in luck > > because the environment normally doesn't communicate with outside > > and there are no L2 caches. OTOH on baremetal environments there > > can be active L2 caches, which makes the leak checks fail. > > Sounds like a good explanation. https://gist.github.com/ozaki-r/37bec56f93f5a57dc98ed618e8db42a3 Could you try the patch? Running just one failed ATF test is enough to confirm. Thanks, ozaki-r
Re: CVS commit: src/tests/net
On Fri, Aug 23, 2019 at 1:53 PM matthew green wrote: > > > It's awkward but it's reasonable for now. A proper fix would > > be to stop using kvm(3) for vmstat and drop the sgid bit from > > the binary. > > please finish this work :-) it's been ongoing for a very > long time now... That task exists in our ToDo list for a long time too but there is no spare time to tackle it yet... ozaki-r
Re: CVS commit: src/tests/net
On Fri, Aug 23, 2019 at 01:41:58PM +0900, Ryota Ozaki wrote: > A workaround for the issue is: > cp /usr/bin/vmstat ./vmstat > $HIJACKING ./vmstat > rm -f ./vmstat > > It's awkward but it's reasonable for now. A proper fix would > be to stop using kvm(3) for vmstat and drop the sgid bit from > the binary. Wow, good catch! > I guess on most anita environments leak checks pass just in luck > because the environment normally doesn't communicate with outside > and there are no L2 caches. OTOH on baremetal environments there > can be active L2 caches, which makes the leak checks fail. Sounds like a good explanation. Martin
re: CVS commit: src/tests/net
> It's awkward but it's reasonable for now. A proper fix would > be to stop using kvm(3) for vmstat and drop the sgid bit from > the binary. please finish this work :-) it's been ongoing for a very long time now... .mrg.
Re: CVS commit: src/tests/net
On Thu, Aug 22, 2019 at 5:45 PM Ryota Ozaki wrote: > > On Tue, Aug 20, 2019 at 7:06 PM Ryota Ozaki wrote: > > > > On Tue, Aug 20, 2019 at 6:58 PM Martin Husemann wrote: > > > > > > On Tue, Aug 20, 2019 at 06:50:31PM +0900, Ryota Ozaki wrote: > > > > Hmm, okay, I'm going to disable the feature until the issue is > > > > addressed. > > > > > > Could it be page size related? > > > > I'm not sure. > > > > My fresh chroot environment for ATF tests on amd64 also fails so > > I guess I'm missing something important :-/ > > It seems that the official AFT tests work expectedly except for > evbarm-aarch64. The leak checker uses vmstat -m with rumphijack > to get pool statistics from a rump kernel, however, for failed > cases, rumphijack doesn't work correctly and it gets pool statistics > of the host kernel, resulting in test failures. I understood why vmstat with rumphijack doesn't work. Because vmstat has the sgid bit and it prevents rumphijack, i.e., LD_PRELOAD from working. (Thanks hikaru@ for the suggestion!) A workaround for the issue is: cp /usr/bin/vmstat ./vmstat $HIJACKING ./vmstat rm -f ./vmstat It's awkward but it's reasonable for now. A proper fix would be to stop using kvm(3) for vmstat and drop the sgid bit from the binary. > > I don't understand yet why rumphijack doesn't work for vmstat -m > in some cases and does work in other cases. I guess on most anita environments leak checks pass just in luck because the environment normally doesn't communicate with outside and there are no L2 caches. OTOH on baremetal environments there can be active L2 caches, which makes the leak checks fail. ozaki-r
Re: CVS commit: src/tests/net
On Tue, Aug 20, 2019 at 7:06 PM Ryota Ozaki wrote: > > On Tue, Aug 20, 2019 at 6:58 PM Martin Husemann wrote: > > > > On Tue, Aug 20, 2019 at 06:50:31PM +0900, Ryota Ozaki wrote: > > > Hmm, okay, I'm going to disable the feature until the issue is addressed. > > > > Could it be page size related? > > I'm not sure. > > My fresh chroot environment for ATF tests on amd64 also fails so > I guess I'm missing something important :-/ It seems that the official AFT tests work expectedly except for evbarm-aarch64. The leak checker uses vmstat -m with rumphijack to get pool statistics from a rump kernel, however, for failed cases, rumphijack doesn't work correctly and it gets pool statistics of the host kernel, resulting in test failures. I don't understand yet why rumphijack doesn't work for vmstat -m in some cases and does work in other cases. ozaki-r
Re: CVS commit: src/tests/net
On Tue, Aug 20, 2019 at 6:58 PM Martin Husemann wrote: > > On Tue, Aug 20, 2019 at 06:50:31PM +0900, Ryota Ozaki wrote: > > Hmm, okay, I'm going to disable the feature until the issue is addressed. > > Could it be page size related? I'm not sure. My fresh chroot environment for ATF tests on amd64 also fails so I guess I'm missing something important :-/ Anyway I disabled the feature in -current for now. ozaki-r
Re: CVS commit: src/tests/net
On Tue, Aug 20, 2019 at 06:50:31PM +0900, Ryota Ozaki wrote: > Hmm, okay, I'm going to disable the feature until the issue is addressed. Could it be page size related? Martin
Re: CVS commit: src/tests/net
On Tue, Aug 20, 2019 at 6:35 PM Martin Husemann wrote: > > On Tue, Aug 20, 2019 at 07:53:28AM +0200, Martin Husemann wrote: > > On Tue, Aug 20, 2019 at 11:21:08AM +0900, Ryota Ozaki wrote: > > > Is it an issue specific to sparc64? > > > > No, same happens on (big endian) arm. Endianess issue? > > I'll do a little endian arm run today. > > Nope, evbarm fails the same: > > net/if_bridge/t_rtable (484/833): 6 test cases > bridge_rtable_basic: [10.960250s] Failed: $target$reqs != $target$rels > (llentrypl10 != llentrypl6) > bridge_rtable_delete_member: [14.861957s] Failed: $target$reqs != > $target$rels (llentrypl10 != llentrypl6) > bridge_rtable_flush: [13.062212s] Failed: $target$reqs != $target$rels > (llentrypl10 != llentrypl6) > bridge_rtable_manyaddrs: [131.755625s] Failed: $target$reqs != > $target$rels (llentrypl10 != llentrypl6) > bridge_rtable_maxaddr: [11.661263s] Failed: $target$reqs != $target$rels > (llentrypl10 != llentrypl6) > bridge_rtable_timeout: [11.652307s] Failed: $target$reqs != $target$rels > (llentrypl10 != llentrypl6) > [194.004564s] Hmm, okay, I'm going to disable the feature until the issue is addressed. ozaki-r
Re: CVS commit: src/tests/net
On Tue, Aug 20, 2019 at 07:53:28AM +0200, Martin Husemann wrote: > On Tue, Aug 20, 2019 at 11:21:08AM +0900, Ryota Ozaki wrote: > > Is it an issue specific to sparc64? > > No, same happens on (big endian) arm. Endianess issue? > I'll do a little endian arm run today. Nope, evbarm fails the same: net/if_bridge/t_rtable (484/833): 6 test cases bridge_rtable_basic: [10.960250s] Failed: $target$reqs != $target$rels (llentrypl10 != llentrypl6) bridge_rtable_delete_member: [14.861957s] Failed: $target$reqs != $target$rels (llentrypl10 != llentrypl6) bridge_rtable_flush: [13.062212s] Failed: $target$reqs != $target$rels (llentrypl10 != llentrypl6) bridge_rtable_manyaddrs: [131.755625s] Failed: $target$reqs != $target$rels (llentrypl10 != llentrypl6) bridge_rtable_maxaddr: [11.661263s] Failed: $target$reqs != $target$rels (llentrypl10 != llentrypl6) bridge_rtable_timeout: [11.652307s] Failed: $target$reqs != $target$rels (llentrypl10 != llentrypl6) [194.004564s] Martin
Re: CVS commit: src/tests/net
On Tue, Aug 20, 2019 at 11:21:08AM +0900, Ryota Ozaki wrote: > Is it an issue specific to sparc64? No, same happens on (big endian) arm. Endianess issue? I'll do a little endian arm run today. Martin
Re: CVS commit: src/tests/net
On Mon, Aug 19, 2019 at 10:18 PM Martin Husemann wrote: > > On Mon, Aug 19, 2019 at 03:22:47AM +, Ryota Ozaki wrote: > > Module Name: src > > Committed By: ozaki-r > > Date: Mon Aug 19 03:22:47 UTC 2019 > > > > Modified Files: > > src/tests/net: net_common.sh > > > > Log Message: > > tests: check pool object leaks > > > > Currently only llentpl leaks can be detected. > > The message is cryptic ;-) Sorry, I'll describe how it works in the script. > > Also this breaks ~all the networking tests for me, failures jumped from > 19 to 462 on my sparc64 test run. Hmm, it works correctly for me on amd64 and it seems to work on anita/i386 too: http://releng.netbsd.org/b5reports/i386/commits-2019.08.html#build-2019.08.19.03.25.40 . Is it an issue specific to sparc64? ozaki-r
Re: CVS commit: src/tests/net
On Mon, Aug 19, 2019 at 03:22:47AM +, Ryota Ozaki wrote: > Module Name: src > Committed By: ozaki-r > Date: Mon Aug 19 03:22:47 UTC 2019 > > Modified Files: > src/tests/net: net_common.sh > > Log Message: > tests: check pool object leaks > > Currently only llentpl leaks can be detected. The message is cryptic ;-) Also this breaks ~all the networking tests for me, failures jumped from 19 to 462 on my sparc64 test run. Martin
Re: CVS commit: src/tests/lib/libc/sys
On 01.07.2019 14:59, Robert Elz wrote: > Date:Mon, 1 Jul 2019 11:55:57 +0200 > From:Joerg Sonnenberger > Message-ID: <20190701095557.ga55...@bec.de> > > | On Mon, Jul 01, 2019 at 02:04:38AM +, Kamil Rytarowski wrote: > > | > Cast note_hdr.n_namesz to ssize_t through size_t to avoid potential > | > signedness bit shifts. > | > | This change makes no sense. > > Certainly the size_t cast is pointless ... and I have no idea what > the comment in the commit message about bit shifts is about, this is > (was) a simple comparison of an unsigned with a signed, which the > cast to ssize_t turns into a signed comparison, which is as it should be. > There are no bit shifts. > > kre > > OK, it was cast of 32bit value to ssize_t that can be 64bit. The original type is uint32_t, not int32_t, so intermediate cast is not needed. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/lib/libc/sys
Date:Mon, 1 Jul 2019 11:55:57 +0200 From:Joerg Sonnenberger Message-ID: <20190701095557.ga55...@bec.de> | On Mon, Jul 01, 2019 at 02:04:38AM +, Kamil Rytarowski wrote: | > Cast note_hdr.n_namesz to ssize_t through size_t to avoid potential | > signedness bit shifts. | | This change makes no sense. Certainly the size_t cast is pointless ... and I have no idea what the comment in the commit message about bit shifts is about, this is (was) a simple comparison of an unsigned with a signed, which the cast to ssize_t turns into a signed comparison, which is as it should be. There are no bit shifts. kre
Re: CVS commit: src/tests/lib/libc/sys
On Mon, Jul 01, 2019 at 02:04:38AM +, Kamil Rytarowski wrote: > Module Name: src > Committed By: kamil > Date: Mon Jul 1 02:04:38 UTC 2019 > > Modified Files: > src/tests/lib/libc/sys: t_ptrace_wait.c > > Log Message: > Avoid GCC warning on NetBSD/i386 > > Cast note_hdr.n_namesz to ssize_t through size_t to avoid potential > signedness bit shifts. This change makes no sense. Joerg
Re: CVS commit: src/tests/lib/libcurses
On 25/06/2019 23:19, Brett Lymn wrote: Module Name:src Committed By: blymn Date: Tue Jun 25 22:19:29 UTC 2019 Modified Files: src/tests/lib/libcurses: t_curses.sh src/tests/lib/libcurses/tests: mvscanw Log Message: Fixed mvscanw test but leave disabled for the moment, the return for mvscanw is incorrect in libcurses, we need a major lib version bump to correct it. Can we do this before -9? Roy
Re: CVS commit: src/tests/lib/libc/atomic
On Feb 28, 1:05am, is...@pastel-flower.jp (Tetsuya Isaki) wrote: -- Subject: Re: CVS commit: src/tests/lib/libc/atomic | At Wed, 27 Feb 2019 10:32:11 -0500, | Christos Zoulas wrote: | > Module Name:src | > Committed By: christos | > Date: Wed Feb 27 15:32:11 UTC 2019 | > | > Modified Files: | > src/tests/lib/libc/atomic: t___sync_and.c | > | > Log Message: | > Make the _and_and_ have-nots compile. | | Sorry for build breakage. And thank you. | | I'm trying to add __sync_and_and_fetch_* to every libc | that got an error this time. | After confirming all build (tomorrow or later), I'll revert | your change. Is this ok? Yes, that's a much better way to fix things! Please remove my band-aid once you are done :-) christos
Re: CVS commit: src/tests/lib/libc/atomic
At Wed, 27 Feb 2019 10:32:11 -0500, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Wed Feb 27 15:32:11 UTC 2019 > > Modified Files: > src/tests/lib/libc/atomic: t___sync_and.c > > Log Message: > Make the _and_and_ have-nots compile. Sorry for build breakage. And thank you. I'm trying to add __sync_and_and_fetch_* to every libc that got an error this time. After confirming all build (tomorrow or later), I'll revert your change. Is this ok? Thanks, --- Tetsuya Isaki
Re: CVS commit: src/tests/lib/libc/misc
On 04.02.2019 09:50, Robert Elz wrote: > Date:Mon, 4 Feb 2019 05:02:46 +0100 > From:Kamil Rytarowski > Message-ID: <2eadaf71-d7d7-c285-bdec-78ddcd3a5...@gmx.com> > > > | If GCC is fine with it, we could try raise(!!(a * b) ? SIGSEGV : SIGBUS);= > > That's a kind of odd way of saying (a * b) != 0 ? ... > > kre > Yes, if it is acceptable for GCC, it's another option. signature.asc Description: OpenPGP digital signature
Re: CVS commit: src/tests/lib/libc/misc
Date:Mon, 4 Feb 2019 05:02:46 +0100 From:Kamil Rytarowski Message-ID: <2eadaf71-d7d7-c285-bdec-78ddcd3a5...@gmx.com> | If GCC is fine with it, we could try raise(!!(a * b) ? SIGSEGV : SIGBUS);= That's a kind of odd way of saying (a * b) != 0 ? ... kre
