From owner-svn-src-head@freebsd.org Tue Aug 21 08:31:59 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 C4C75108D8A7; Tue, 21 Aug 2018 08:31:59 +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 1D54A793A5; Tue, 21 Aug 2018 08:31:58 +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 15F861A1B0D; Tue, 21 Aug 2018 18:31:49 +1000 (AEST) Date: Tue, 21 Aug 2018 18:31:48 +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: r338128 - in head: cddl/lib/libzpool cddl/usr.bin/ztest cddl/usr.sbin/zdb sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys sys/conf sys/modules/zfs In-Reply-To: <31e29ae2-99ea-ab62-c337-c1d7f0b4aab5@FreeBSD.org> Message-ID: <20180821180233.F960@besplex.bde.org> References: <201808210345.w7L3jADR035282@repo.freebsd.org> <31e29ae2-99ea-ab62-c337-c1d7f0b4aab5@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=FNpr/6gs c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=x7bEGLp0ZPQA:10 a=eWiB3hlGkldaFsU_ULYA: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: Tue, 21 Aug 2018 08:32:00 -0000 On Tue, 21 Aug 2018, John Baldwin wrote: > On 8/21/18 4:45 AM, Matt Macy wrote: >> ... >> Log: >> Make dnode definition uniform on !x86 >> >> gcc4 requires -fms-extensions to accept anonymous union members It does not. Anonymous unions have been a standard gcc extension since ~1989, but these are turned off by requesting a c99 compiler using CSTD=c99. Requesting this is broken, but clang is too broken to honor the request. >> Modified: >> head/cddl/lib/libzpool/Makefile >> head/cddl/usr.bin/ztest/Makefile >> head/cddl/usr.sbin/zdb/Makefile >> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dnode.h >> head/sys/conf/kern.pre.mk >> head/sys/modules/zfs/Makefile > > Are you really sure you need the CFLAGS changes in all these places? Userland > already defaults to a 'cstd' of 'gnu99' which allows anonymous unions by > default (whereas the kernel uses 'c99'), and kern.pre.mk already adds > -fms-extensions to CFLAGS earlier in the file (so that change is redundant). > kmod.mk also adds -fms-extensions already (so the ZFS change should be redundant). kern.pre.mk adds it just 3 lines earlier, so it is visibly added twice in the patch that adds it: XX Modified: head/sys/conf/kern.pre.mk XX ============================================================================== XX --- head/sys/conf/kern.pre.mk Tue Aug 21 03:33:54 2018 (r338127) XX +++ head/sys/conf/kern.pre.mk Tue Aug 21 03:45:09 2018 (r338128) XX @@ -89,6 +89,7 @@ CFLAGS_ARCH_PARAMS?=--param max-inline-insns-single=10 XX CFLAGS.gcc+= -fno-common -fms-extensions -finline-limit=${INLINE_LIMIT} ^^^^^^^^^^^^ ^^^^^^^^^^^^^^^ XX CFLAGS.gcc+= --param inline-unit-growth=${CFLAGS_PARAM_INLINE_UNIT_GROWTH} XX CFLAGS.gcc+= --param large-function-growth=${CFLAGS_PARAM_LARGE_FUNCTION_GROWTH} XX +CFLAGS.gcc+= -fms-extensions ^^^^^^^^^^^^ ^^^^^^^^^^^^^^^ XX .if defined(CFLAGS_ARCH_PARAMS) XX CFLAGS.gcc+=${CFLAGS_ARCH_PARAMS} XX .endif > As mentioned earlier, already uses anonymous unions, so nothing > would compile unless this already worked. > > I suspect the real issue is that ZFS when compiled into the kernel uses a > custom set of CFLAGS that might not be picking up the CFLAGS.gcc. > > In summary, all of the CFLAGS changes look wrong / redundant. Can you share > what build error you were actually seeing without the CFLAGS changes? cddl/lib/libzpool/Makefile, cddl/usr.bin/ztest/Makefile and cddl/usr.sbin/zdb/Makefile are most clearly broken. They hard-code CSTD=c99. Since user mk files don't clobber previous settings like kernel mk files do, this "works" to break the user default of gnu99, so it should ensure that anonymous unions are uncompilable with any C compiler. This commit adds the -fms-extensions wart to these Makefiles if the compiler is gcc, so anonymous unions should now be compilable by gcc but by no other C compiler. The only other changes to makefiles in this commit are the kernel ones, and these clearly have no effect since they just add -fms-extensions again. Bruce