From owner-svn-src-head@freebsd.org Wed Aug 15 09:09:38 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id A1424107AE63; Wed, 15 Aug 2018 09:09:38 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id DAFB78167B; Wed, 15 Aug 2018 09:09:37 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail108.syd.optusnet.com.au (Postfix) with ESMTPS id B692B1A1DE2; Wed, 15 Aug 2018 19:09:34 +1000 (AEST) Date: Wed, 15 Aug 2018 19:09:34 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: John Baldwin cc: Matt Macy , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r337674 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys In-Reply-To: Message-ID: <20180815174659.X2616@besplex.bde.org> References: <201808120209.w7C297Ub018593@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=DZtnkrlW c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=x7bEGLp0ZPQA:10 a=obyNhTc-XXDjlMljpWoA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Aug 2018 09:09:38 -0000 On Wed, 15 Aug 2018, John Baldwin wrote: > On 8/12/18 3:09 AM, Matt Macy wrote: >> >> Log: >> Restore legacy dnode_phys layout on tier 2 arches >> >> Evidently gcc4 doesn't support anonymous union members Even gcc3 supports anonomous union members. Maybe even gcc1. They are a standard gcc feature unless turned off by requesting a stricter standard. Kernel builds are misconfigured by requesting the stricter standard C99 with -std=c99. clang is too broken to support this, but gcc4 -std=c99 resembles a C99 compiler so it doesn't allow anonymous unions. > It does. We use it in 'struct mbuf'. It does require > -fms-extensions which we enable for kernel builds. Perhaps ZFS is > missing this flag due to using custom build flags and/or rules? -fms-extensions is a wart on a bug. After misconfiguring kernel builds using -std=c99, -fms-extensions is misused to add some extensions. It gives many unwanted ms extensions together with extra gcc anonymous unions and some unneeded but harmless extensions for gcc anonymous unions. It doesn't give all gcc extensions AFAIK, so it is surprising that -std=c99 doesn't break more than it does. Very little code compiles with strict C99 compilers. It takes -pedantic as well as well as a -std directive to get nearer to strictness for the requested standard. Apparently not using pedantic, together with using the correct spelling for __asm() and __volatile, hides most of the problems. However, __extension__ is usually not used, so many of the problems should not be hidden. kern.mk enforces its broken standard using CSTD=c99 instead of CSTD?=c99, and then has a lot of garbage ifdefs to support the even more unusable standards k&r, c89, c90, c94 and c95. All these bugs are missing in userland. bsd.sys.mk defaults to the correct standard gnu99 but doesn't enforce it. It uses CSTD?=gnu99. This is followed the same ifdefs. These ifdefs are silly instead of garbage for userland. In theory, the user can request plain k&r, though support for it has rotted and is not needed for for FreeBSD sources. Even the c90 support is unlikely to work. There is no direct support for the more useful standard gnu89. Except for k&r, all the ifdef does is encrypt the standard name into iso-speak. E.g., -std=c99 becomes -std=iso9899:1999, to make CFLAGS and thus make -n output less readable. In the kern.mk, the forced -std=c99 turns into -std=iso-mumble, so might as well be hard-coded as the latter. The -fms-extensions wart is mishandled differently: - in system mk files, it is added to CFLAGS in kern.pre.mk and kmod.mk. - again the addition is unconditional, except it is only done for gcc. clang -std=c99 is apparently so broken that it allows all ms extensions, not just anonymous unions. - in kern.pre.mk, -fms-extensions is added on the same line as -fno-common and an actual gcc4-specific flag -finline-limit. -fno-common is not added anywhere for clang, and clang is not incompatible enough to force it, so clang restores old bugs involving common variables (separate compilation units can have "int x;" or even "int x[1];" and "int x[2];", and the error is not detected). - in kmod.mk, -fms-extensions is on a line by itself, and -fno-common is not broken for clang by mixing it into an unrelated gcc section. The gcc inline flags are on separate lines and merely have style bugs (excessive line splitting for related things, and inconsistent whitespace). Userland is again much better. The fms-extensions wart is missing from all user mk files. Ordinary gcc extensions given by gnu99 (and probably also gnu89) are apparently enough for the much larger userland. It is much larger since it needs to work for ports and also for any extensions in used in kernel headers. E.g., the extensive use of anonymous unions in is not under _KERNEL, so userland must understand it. And userland does understand it since it uses the correct standard gnu99. The wrong CSTD implemented in bsd.sys.mk too, but was quickly backed out since userland has larger test coverage so the bug was obvious. Some history of it there: r115902 in 2003-06-06: default to gnu99 and note than c99 doesn't work since it kills alloca r115942 in 2003-06-07: default to c99 on most arches r116316 in 2003-06-13: default to gnu99 on all arches except i386, since c99 kills alloca on amd65 r116330 in 2003-06-14: don't default to anything, as before this round of churning (only use CSTD if it is already set) r116342 in 2003-06-14: remove all CSTD support r161214 in 2006-08-11: restore CSTD support to almost what it was in r116330 r188801 in 2009-03-14: restore CSTD support to almost what it was in in r115902 (default to the correct standard of gnu89) The userland bugs in this were little more than style bugs except for using the wrong standard. I used to thing that it was a style bug for mk files to set the C standard at all, but it is now clear that unportable code in system headers needs to set the C standard to not actually standard. -std=c99 plus -fgnu99-extensions would be a better way of spelling this than -std-gnu99. The zfs bug might only show up for gcc, by using the broken -std=c99 since it is set very globally in kern.mk, but somehow not using the wart in special rules. But CDDL_CFLAGS uses ${CFLAGS} so it should have everything. Bruce