svn commit: r337674 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys

[Bruce Evans]

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 <sys/mbuf.h> 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