Date: Tue, 21 Aug 2018 18:31:48 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: John Baldwin <jhb@freebsd.org> Cc: Matt Macy <mmacy@freebsd.org>, 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 Message-ID: <20180821180233.F960@besplex.bde.org> In-Reply-To: <31e29ae2-99ea-ab62-c337-c1d7f0b4aab5@FreeBSD.org> References: <201808210345.w7L3jADR035282@repo.freebsd.org> <31e29ae2-99ea-ab62-c337-c1d7f0b4aab5@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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, <sys/mbuf.h> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180821180233.F960>