Date: Wed, 5 Jun 2019 11:31:22 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Gleb Smirnoff <glebius@freebsd.org> Cc: Dmitry Chagin <dchagin@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r348419 - in head: crypto/heimdal/lib/ipc share/man/man4 sys/compat/linux sys/kern sys/sys usr.sbin/mountd Message-ID: <20190605095458.S1083@besplex.bde.org> In-Reply-To: <20190604225128.GL21836@FreeBSD.org> References: <201905301424.x4UEORXr098755@repo.freebsd.org> <20190604225128.GL21836@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 4 Jun 2019, Gleb Smirnoff wrote: > On Thu, May 30, 2019 at 02:24:27PM +0000, Dmitry Chagin wrote: > D> Author: dchagin > D> ... > D> Log: > D> Complete LOCAL_PEERCRED support. Cache pid of the remote process in the > D> struct xucred. Do not bump XUCRED_VERSION as struct layout is not changed. > D> > D> PR: 215202 > D> Reviewed by: tijl > D> MFC after: 1 week > D> Differential Revision: https://reviews.freebsd.org/D20415 > ... > D> Modified: head/sys/sys/ucred.h > D> ============================================================================== > D> --- head/sys/sys/ucred.h Thu May 30 14:21:51 2019 (r348418) > D> +++ head/sys/sys/ucred.h Thu May 30 14:24:26 2019 (r348419) > D> @@ -87,10 +87,14 @@ struct xucred { > D> uid_t cr_uid; /* effective user id */ > D> short cr_ngroups; /* number of groups */ > D> gid_t cr_groups[XU_NGROUPS]; /* groups */ > D> - void *_cr_unused1; /* compatibility with old ucred */ > D> + union { > D> + void *_cr_unused1; /* compatibility with old ucred */ > D> + pid_t _pid; > D> + } _cr; > D> }; > D> #define XUCRED_VERSION 0 > D> > D> +#define cr_pid _cr._pid > > Why don't use C99 in 2019 instead of preprocessor define? Should be > > union { > void *_cr_unused1; /* compatibility with old ucred */ > pid_t cr_pid; > }; Anonymous unions are a gnu89 extension. Only broken C99 compilers like clang -std=c99 accept them. The kernel makefiles (kern.mk, kern.pre.mk and kmod.mk) are still very broken in this area: - kern.mk forces CSTD=c99. This prevents users using their preferred standard or even the correct standard. These bugs are missing in bsd.sys.mk. It uses the correct standard gnu99, and assigns it using ?= to not prevent users using their preferred standard or even the correct standard. But see the commit logs fo bsd.sys.mk. It took several rounds of churning to get this almost right. Using c99 for the standard is even wronger in the kernel than in userland, since the kernel uses nonstandard extensions relatively much more often than userland. - after this forcing, there is an ifdef copied (except for removing k&r support) from bsd.kern.mk. Since CSTD is forced, in the kernel this is an obfuscated way of adding the obfuscated standard name iso9899:1999. De-obfuscating 13 lines of this gives CFLAGS+= -std=iso9899:1999 - clang dishonors this request to be a C99 compiler and doesn't even warn about anonymous unions in the kernel. It takes -pedantic as well as -std=c99 to make clang resemble a C99 compiler. With -pedantic, it tells you that "anonymous unions are a C11 extension [-Wc11-extensions]". - anonymous unions are actually a gnu99 extension. But is closer to resembling a C99 compiler when requested to be one. It doesn't take -pedantic to make it warn about anonymous unions in the kernel. - you "fixed" this for gcc in r278913 by adding -fms-extensions for gcc only. This is in kern.pre.mk and kmod.mk. MS apparently added this extension 10-20 years after gnu added it. -fms-extensions gives subtly different (mostly stronger) semantics for anonymous unions. The kernel only needs old gnu semantics. -ms-extensions gives many other unwanted NS extensions. -current still has this bug. - r338128 adds -ms-extensions to CFLAGS for gcc on a different line. This bug is missing in kmod.mk. -current still has this bug. Nearby bugs: - -fms-extensions was added in a wrong place in r278913. It was added after -fno-common, which was already in a wrong place. In FreeBSD-7, -fno-common was added before inline limits, but on the same line as the first inline limit. Inline limits are very gcc-4.2.1-specific, so they needed ifdefs. There are now some ifdefs. Not nearly enough -- only 1 for mips and ones for gcc implemented using CFLAGS.gcc. The bug is now -fno-common is ifdefed for gcc using CFLAGS.gcc, so it is broken for clang (in FreeBSD-7, it was ifdefed for icc using an explicit ifdef. - I once fixed large parts of the kernel (at least all header files) to compile with -pedantic. The fixes included marking extensions using __extension__, expecially in header files. -pedantic is unusuable now even 1 file at a time, due to too many syntax errors and unmarked extensions in headers. E.g., compiling init_main.c now gives 2345 errors in 17312 lines after increasing -ferror-limit from 20 to 1000000. With the default limit of 20, the following errors are detected: XX In file included from ../../../kern/init_main.c:55: XX In file included from ../../../sys/epoch.h:41: XX In file included from ../../../sys/pcpu.h:50: XX ./machine/pcpu.h:49:1: error: _Static_assert is a C11-specific feature [-Werror,-Wc11-extensions] XX _Static_assert(sizeof(struct monitorbuf) == 128, "2x cache line"); XX ^ XX In file included from ../../../kern/init_main.c:56: XX In file included from ../../../sys/eventhandler.h:37: XX In file included from ../../../sys/mutex.h:44: XX In file included from ../../../sys/lockstat.h:41: XX ../../../sys/sdt.h:430:26: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROVIDER_DECLARE(sdt); XX ^ XX In file included from ../../../kern/init_main.c:56: XX In file included from ../../../sys/eventhandler.h:37: XX In file included from ../../../sys/mutex.h:44: XX ../../../sys/lockstat.h:43:31: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROVIDER_DECLARE(lockstat); XX ^ XX ../../../sys/lockstat.h:45:51: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , adaptive__acquire); XX ^ XX ../../../sys/lockstat.h:46:51: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , adaptive__release); XX ^ XX ../../../sys/lockstat.h:47:48: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , adaptive__spin); XX ^ XX ../../../sys/lockstat.h:48:49: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , adaptive__block); XX ^ XX ../../../sys/lockstat.h:50:47: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , spin__acquire); XX ^ XX ../../../sys/lockstat.h:51:47: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , spin__release); XX ^ XX ../../../sys/lockstat.h:52:44: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , spin__spin); XX ^ XX ../../../sys/lockstat.h:54:45: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , rw__acquire); XX ^ XX ../../../sys/lockstat.h:55:45: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , rw__release); XX ^ XX ../../../sys/lockstat.h:56:43: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , rw__block); XX ^ XX ../../../sys/lockstat.h:57:42: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , rw__spin); XX ^ XX ../../../sys/lockstat.h:58:45: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , rw__upgrade); XX ^ XX ../../../sys/lockstat.h:59:47: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , rw__downgrade); XX ^ XX ../../../sys/lockstat.h:61:45: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , sx__acquire); XX ^ XX ../../../sys/lockstat.h:62:45: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , sx__release); XX ^ XX ../../../sys/lockstat.h:63:43: error: extra ';' outside of a function [-Werror,-Wextra-semi] XX SDT_PROBE_DECLARE(lockstat, , , sx__block); XX ^ XX fatal error: too many errors emitted, stopping now [-ferror-limit=] XX 20 errors generated. Many of the other errors are for zero-sized arrays automatically generated in sysproto.h. With gcc, there seem to be still over 2000 errors, but it only takes 2327 lines to report them, and there are only 52 lines left after removing the ones for zero-sized arrays. The others are much as above: - 25 lines for "extra ';' outside of function - 4 lines for gnu statement-expressions (lots of these for bzero()) - 3 lines for forward references to enums - 1 line for an unnamed struct/union. Unfortunately, -pedantic seems to be broken for both clang and gcc in the following way: it seems to kill all gnu extensions, so even with the correct standard -std=-gnu99 (and also with -ms-extensions twice for gcc), adding -pedantic gives the same errors as with -std=c99. I thought that zero-sized arrays were in C99. Actually, they aren't even in C17. It is my design in sysproto.h that generates thousands of them :-(. The forward references to enums are most interesting. These are all auto- generated bugs in syscallsubr.h. The header that declares 'enum idtype' is not encluded, so the size of the enum is not known. 'enum idtype' is forward-declared to break detection of the bug when this enum is used in 2 prototypes, but -pedantic reports all 3 instances of 'enum idtype' as errors. This can only work for compilers that make all enum types have the same size. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20190605095458.S1083>