Re: CVS commit: src/common/lib/libc/string
On Mon, 14 Apr 2014, Joerg Sonnenberger wrote: Modified Files: src/common/lib/libc/string: bcopy.c Log Message: Using bcopy/memcpy with NULL arguments is valid as long as the size is also 0. No, it's undefined behaviour. C99 section 7.21.1: Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. and 7.1.4 says: If an argument to a function has an invalid value (such as ... a null pointer ...) ..., the behavior is undefined. and 7.21.2.1 The memcpy function does not give any explicit permission for use of null pointers. I don't object if the implementation wants to allow null pointers with zero size as an extension, but it should be clear that this is non-standard. --apb (Alan Barrett)
re: CVS commit: src/sys/arch/x68k/stand/boot_ustar
Tetsuya Isaki writes: Module Name: src Committed By: isaki Date: Mon Apr 14 14:24:27 UTC 2014 Modified Files: src/sys/arch/x68k/stand/boot_ustar: Makefile Log Message: Remove -mc68000 asm option for GCC4.8 (or new binutils?). With this option, new gcc complains that boot_ustar.S:21: Error: selected processor does not have all features of selected architecture. I'm not sure about the essence of this error, but this option maybe not needed because there is no need to consider about 68000 here. hmmm this option is now called -march=68000. i don't think any x68k are 68000 are they? all 020/030/040? perhaps using -mcpu=m68020 here might be best? i would test some and see if size or speed matters any. .mrg.
Re: CVS commit: src/sys/arch/x68k/stand/boot_ustar
On Tue, Apr 15, 2014 at 04:51:29PM +1000, matthew green wrote: hmmm this option is now called -march=68000. i don't think any x68k are 68000 are they? all 020/030/040? perhaps using -mcpu=m68020 here might be best? i would test some and see if size or speed matters any. It doesn't really matter for .S files, as long as the code does not mutate due to ifdefs - or am I missing something? Martin
re: CVS commit: src/sys/arch/x68k/stand/boot_ustar
Martin Husemann writes: On Tue, Apr 15, 2014 at 04:51:29PM +1000, matthew green wrote: hmmm this option is now called -march=68000. i don't think any x68k are 68000 are they? all 020/030/040? perhaps using -mcpu=m68020 here might be best? i would test some and see if size or speed matters any. It doesn't really matter for .S files, as long as the code does not mutate due to ifdefs - or am I missing something? i think it matters because as(1) might reject stuff otherwise, but i would imagine that the defaults for m68k are ok defaults for x68k port, so maybe this option going away is fine. .mrg.
Re: CVS commit: src/common/lib/libc/string
Hello, Joerg Sonnenberger jo...@netbsd.org wrote: |Module Name: src |Committed By: joerg |Date: Mon Apr 14 18:18:58 UTC 2014 | |Modified Files: | src/common/lib/libc/string: bcopy.c | |Log Message: |Using bcopy/memcpy with NULL arguments is valid as long as the size is |also 0. This is great interface design and easy to use, but it is not specified in neither POSIX nor ISO C (11). And worse i'm afraid that the ISO C people would actively veto this very behaviour given that wmemcpy(3) is standardized as If n is zero, the application shall ensure that ws1 and ws2 are valid pointers, and the function shall copy zero wide characters. --steffen
Re: CVS commit: src/sys/arch/x68k/stand/boot_ustar
mrg@ wrote: Martin Husemann writes: On Tue, Apr 15, 2014 at 04:51:29PM +1000, matthew green wrote: hmmm this option is now called -march=68000. I don't think this is correct. i don't think any x68k are 68000 are they? all 020/030/040? perhaps using -mcpu=m68020 here might be best? i would test some and see if size or speed matters any. In this case, it looks 68000 was intended to be supported. (see below) It doesn't really matter for .S files, as long as the code does not mutate due to ifdefs - or am I missing something? Probably as(1) might do some optimization or instruction replacement per the specified processor (addressing, branch ranges etc). i think it matters because as(1) might reject stuff otherwise, but i would imagine that the defaults for m68k are ok defaults for x68k port, so maybe this option going away is fine. Here is my guess: - NetBSD/x68k supports only X680x0 machines with MC68030 and higher processors. - Normal X68000 machines (i.e. all X680x0 except X68030) have MC68000, so 030 accelerators are required for the X68000 models, i.e. XVI, SUPER, EXPERT, PRO, and ACE. (BTW the normal X68030 has MC68EC030 so users also have to replace its CPU to with MC68030 to use NetBSD/x68k) - On the other hand, probably early x68k developers considered that some stupid users could try to boot NetBSD/x68k on X68000 without 030 and bootloaders should have some sanity checks. - All primary bootloaders except the boot_ustar have 020 checks: http://nxr.netbsd.org/xref/src/sys/arch/x68k/stand/boot_ufs/boot.S?r=1.10#65 but the boot_ustar (for boot floppy) doesn't due to size restriction, so -mc68000 was intentionally specified for it: http://mail-index.netbsd.org/source-changes/2001/10/01/0037.html - However, the secondary boot doesn't have proper 68000 check (yet?): http://nxr.netbsd.org/xref/src/sys/arch/x68k/stand/boot/srt0.S?r=1.2#60 Nowadays all (aged) X680x0 geeks know it won't work on normal X68000, so probably it's safe to remove it. (I'm not sure if -mc68000 actually emits different code though) Note a possible change in gcc 4.8 that causes this trouble is: http://gcc.gnu.org/viewcvs/gcc/trunk/gcc/config/m68k/m68k.c?annotate=173256#l482 I guess that gcc 4.5 passes only explicitly specified -m680x0 (which seems equivalent to -march=m680x0) options (i.e. as(1) will use its default -mcpu), but gcc 4.8 always passes both -march and -mcpu per the specified -m options to gcc(1) (m68k_cpu_entry is always set?) so passing only -Wa,-m[c]680x0 on gcc 4.8 makes as(1) complain arch vs cpu mismatch. That's the reason why passing both -march=m68030 and -mcpu=m68030 solves build failure on mvme68k 060 kernels with both gcc 4.5 and 4.8. In this boot_ustar case, passing only -Wa,-mcpu=m68000 fails with gcc 4.5, and passing only -Wa,-march=m68000 (or -Wa,-m68030) fails with gcc 4.8. --- Izumi Tsutsui
Re: CVS commit: src/common/lib/libc/string
P.S.: i wasn't subscribed to this list (until hopefully now), so i haven't seen that Alan Barrett already commented. But now that i read it, ISO C 2011 states the same (7.24.1). --steffen
Re: CVS commit: src/common/lib/libc/string
On Tue, Apr 15, 2014 at 08:06:57AM +0200, Alan Barrett wrote: On Mon, 14 Apr 2014, Joerg Sonnenberger wrote: Modified Files: src/common/lib/libc/string: bcopy.c Log Message: Using bcopy/memcpy with NULL arguments is valid as long as the size is also 0. No, it's undefined behaviour. C99 section 7.21.1: Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. and 7.1.4 says: If an argument to a function has an invalid value (such as ... a null pointer ...) ..., the behavior is undefined. and 7.21.2.1 The memcpy function does not give any explicit permission for use of null pointers. I don't object if the implementation wants to allow null pointers with zero size as an extension, but it should be clear that this is non-standard. I remember a discussion about this topic from the LLVM lists and the reasons for the standard language on this are extremely weak. IIRC the *only* justification was for some platforms with broken (trapping) prefetch instructions. Joerg
Re: CVS commit: src/sys/kern
Date: Tue, 15 Apr 2014 09:50:45 + From: Juergen Hannken-Illjes hann...@netbsd.org Fix a deadlock where one thread exits, enters fstrans_lwp_dtor() and wants fstrans_lock. This thread holds the proc_lock. This sounds fishy. lwp_exit does not hold proc_lock while calling lwp_finispecific, so there are no invariants covered by proc_lock that the lwp_specific destructors can rely on. I'm inclined to say that it is a bug for exit1 to hold proc_lock when it calls lwp_finispecific (and proc_finispecific). Can we just release it before and re-acquire it after calling lwp/proc_finispecific? Another thread holds fstrans_lock and runs pserialize_perform(). As the first thread holds the proc_lock, timeouts are blocked and the second thread blocks forever in kpause(). This also sounds fishy. How does T1's holding proc_lock cause T2 to block forever in kpause? I think I'm missing something in this analysis. kpause doesn't take proc_lock, does it?
Re: CVS commit: src/sys/arch/x68k/stand/boot_ustar
On 15 Apr, 2014, at 05:14 , Izumi Tsutsui tsut...@ceres.dti.ne.jp wrote: - NetBSD/x68k supports only X680x0 machines with MC68030 and higher processors. - Normal X68000 machines (i.e. all X680x0 except X68030) have MC68000, so 030 accelerators are required for the X68000 models, i.e. XVI, SUPER, EXPERT, PRO, and ACE. (BTW the normal X68030 has MC68EC030 so users also have to replace its CPU to with MC68030 to use NetBSD/x68k) - On the other hand, probably early x68k developers considered that some stupid users could try to boot NetBSD/x68k on X68000 without 030 and bootloaders should have some sanity checks. I'm pretty sure NetBSD could never run on a 68000 since the 68000 had no memory management unit. The 68010 and 68020 didn't have memory management units either, but Sun did proprietary MMUs for both (that's sun2 and sun3, respectively) and I think other companies may have done MMUs for the 68020. The 68030 was the first CPU in the series to come with an MMU built in. That generic x68x requires a 68030 makes sense since that's the first CPU where the code can count on knowing how the MMU works. A 68020 would have a manufacturer-specific MMU, while for the 68010 there is only sun2. There is no reason to restrict the instruction set to that of the 68000 in any case since NetBSD won't run on that. I once had a 68000 board that ran a 4.3 BSD kernel, but it was a 4.3 BSD kernel with all the process scheduling hacked out so that it ran exactly 1 compiled-in user space process, without memory protection. I used it as an ntp server. Dennis Ferguson
Re: CVS commit: src/sys/netinet
This should be pulled up to netbsd-6 On Apr 12, 2014, at 05:24, Greg Troxel g...@netbsd.org wrote: Module Name: src Committed By: gdt Date: Sat Apr 12 12:24:50 UTC 2014 Modified Files: src/sys/netinet: if_arp.c Log Message: revarprequest: Avoid leaking mbuf. In revarprequest, an mbuf could perhaps be leaked in an error path. My reading of the code is that this is not possible, because ar_pro is set to ETHERNET_IP, and ar_tha can only be null in the 1394 case. But, better to have the free call anyway; ar_tha does not have a documented interface contract :-) Pointed out by Maxime Villard. To generate a diff of this commit: cvs rdiff -u -r1.155 -r1.156 src/sys/netinet/if_arp.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/sys/netinet
Erik Fair f...@netbsd.org writes: On Apr 12, 2014, at 05:24, Greg Troxel g...@netbsd.org wrote: Module Name: src Committed By:gdt Date:Sat Apr 12 12:24:50 UTC 2014 Modified Files: src/sys/netinet: if_arp.c Log Message: revarprequest: Avoid leaking mbuf. In revarprequest, an mbuf could perhaps be leaked in an error path. My reading of the code is that this is not possible, because ar_pro is set to ETHERNET_IP, and ar_tha can only be null in the 1394 case. But, better to have the free call anyway; ar_tha does not have a documented interface contract :-) Pointed out by Maxime Villard. This should be pulled up to netbsd-6 I don't think so; I was able to convince myself by manual static analysis that the leak condition could never happen. So I think the gain/effort ratio isn't high enough to make it worthwhile. As I see it, the fix is more about avoiding a future leak than fixing a current one. pgpS0Z7aKtswx.pgp Description: PGP signature
