Date: Sat, 18 Nov 2017 21:42:21 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Warner Losh <imp@bsdimp.com> Cc: "Rodney W. Grimes" <rgrimes@freebsd.org>, Warner Losh <imp@freebsd.org>, src-committers <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org> Subject: Re: svn commit: r325954 - in head: . share/mk sys/conf usr.sbin/config Message-ID: <20171118190422.V949@besplex.bde.org> In-Reply-To: <CANCZdfod-=vqcZ-NoNmkBPE_--tTcEX6NvyZncR0Bqn%2BrwA=QQ@mail.gmail.com> References: <CANCZdfrrd9JKxN=ZSi17iK_D=rh5R66pkHH%2BmPmy7wENh90k6Q@mail.gmail.com> <201711180134.vAI1Y2ks064138@pdx.rh.CN85.dnsmgr.net> <CANCZdfod-=vqcZ-NoNmkBPE_--tTcEX6NvyZncR0Bqn%2BrwA=QQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 17 Nov 2017, Warner Losh wrote:
> On Fri, Nov 17, 2017 at 6:34 PM, Rodney W. Grimes <
> freebsd@pdx.rh.cn85.dnsmgr.net> wrote:
>
>> [ Charset UTF-8 unsupported, converting... ]
>>> Kib@ posted to arch that we were removing it, nobody objected, we
>> removed
>>> it. If it was a working feature that might have a few users, that's one
>>> thing. But I don't think make lint has actually worked in at least a
>> decade.
>>
>> Thats a sad state of affairs.
>
> Indeed.
The bugs are mostly in system sources outside of lint, and user errors.
>> When I try it today, I get dozens of warnings, and several syntax errors:
>>>
>>> types.h(289): syntax error [249]
This only happens with _KERNEL defined. It shows a bug in types.h. Debugged
below.
>>> nvme.h(79): syntax error [249]
>>> nvme.h(105): syntax error [249]
>>> nvme.h(137): syntax error [249]
>>> nvme.h(160): syntax error [249]
>>> nvme.h(160): cannot recover from previous errors [224]
nvme.h could easily have lots more bugs than sys/types.h, since it is not
designed to support C90 or applications or be very portable.
>>> since it flags all c99 and newer usages as syntax errors. We've been
This shouldn't break use of lint on K&R or C90 sources. But types.h is
broken. It no longer supports K&R, C90, or most other things that are
upposed to be controlled by the visibility ifdefs in it.
>> using
>>> c99 for the kernel since about 8.x, and in the base longer than that.
>> xlint
>>> hasn't been useful since we started doing this. The types.h change went
>> in:
>>> c217b5c12e71 ( mdf 2011-12-12 18:44:17 +0000 289)typedef _Bool
>> bool;
>>> which as far as I can make out was between 8.x being branched and 9.x
>> being
>>> branched. This feature has therefore been broken for 3 major releases
>>> without any announcement, or even any bug reports coming in. Not exactly
>> a
>>> feature that has an audience that needs to be informed of anything...
It was sys/types.h that was broken then. lint just detects the bug.
The bug in sys/types.h is that iff _KERNEL is defined, it uses _Bool without
declaring it unless
__STDC_VERSION__ < 199901L && __GNUC__ < 3 && !defined(__INTEL_COMPILER__).
stdbool.h uses the same ifdef for defining _Bool. However, stdbool.h
uses a different definition of bool that hides the bug unless bool is
actually used -- sttdef.h #defines bool as _Bool, while sys/types.h
typedefs bool as _Bool.
The ifdef is fragile and works and fails as follows:
- when __STDC_VERSION__ >= 199901L, _Bool must be defined by the compiler
and there is no problem.
- gcc-4.2.1 defaults to c90 so __STDC_VERSION__ is undefined and is equivalent
to 0 (modulo the bug of the existence of -Wundef). But __GNUC__ is 4, so
the ifdef is not satisified anyway. It is important to not typedef _Bool
since it is a builtin in gcc-4 even with c90. The __GNUC__ < 3 test is what
implements this.
- icc apparently defines __GNUC__ as < 3, but defines _Bool, so it is important
to not typedef _Bool for icc too. The !defined(__INTEL_COMPILER__) test is
what implements this. Most __INTEL_COMPILER__ tests are used because icc
doesn't emulate being gcc very well.
- clang is very incompatible and defaults to c11, so the ifdef is not
satisfied by default. When c90 is forced, clang is less incompatible and
the ifdef works like for gcc-4 (clang sets __GNUC__ to 4 despite its
incompatibility, and the incompatibility is not large enough to break this
test.)
Then in lint, there is a problem getting __STDC_VERSION__ and even __GNUC__
defined correctly. __GNUC__ should not be defined without the lint option
-g. lint uses cc -E... instead of cpp to reduce unportabilities, but this
turns out to be fairly unportable too, due to the clang incompatiblity.
The main bugs are that lint doesn't use -std at all, and clang is incompatible.
lint only supports c90 and c90 with some gcc extensions, so should use
-std=c89 or -std=gnu90 to restrict the headers to what it supports. The
default -std=c89 still works for gcc, but clang gives -std=c11.
lint's main CFLAGS passed to cc are:
-E -x c (OK so far)
#if 0
bogus null -D__attribute__(()) and -D__extension__(())
#else
-U__GNUC__ (perhaps even worse than the null definitions)
#endif
-Wp,-C -Wcomment (perhaps OK)
-D__LINT__ -Dlint (OK)
-p adds:
-Wtraditional -Wnosystem-headers
-s adds:
-trigraphs -Wtrigraphs -pedantic -D__STRICT_ANSI__
-t adds:
-traditional -D${MACHINE} -D${MACHINE_ARCH}
-d adds:
-nostdinc -idirafter
>> Something larger. Just because the src tree use of lint is failing
>> does not meant that someone out there is not using lint in some
>> other capacity. The depreication policy is there to notify them
>> that lint well be removed as a program in the next release.
>
> Without a working sys/types.h, nothing non-trivial will work.
Even C90 support is broken.
> Lint is totally totally broken. It's a rotting corpse that needs to be just
> gone. Since it has been totally broken for several major releases, I see no
Less broken than in 1993 for the 1990 language that its supports. Its support
was never complete, but FreeBSD fixed all the headers to support this
language. The headers are broken again, but not as badly as in 1993.
Full list of bugs found by lint on sys/types.h:
X _types.h(104): warning: c89 C does not support 'long long' [265]
Lint bug: doesn't give full pathnames. Who knows where _types.h is today?
Well, it is sys/_types.h.
Headers bug: the long long abomination is hard-coded as that in the declaration
of __max_align_t. Using the abomination is wronger than usual here.
sys/_types.h is supposed to be MI, but the abomination is MD. Except ABIs
defeat the point of having long long (a hack to avoid expanding long) by
forcing it to be precisely 64 bits, so it is not really MD. But since it
is not MD, it has little to do with alignment. Using __intmax_t would fix
all of these bugs.
I fixed the corresponding bug in the definition of __intmax_t itself
(actually __int64_t) using __attribute__(__mode__(__DI__)) for gcc and icc,
and long long otherwise. Except lint's #undef of __GNUC__ breaks detection
of gcc, and lint can't do anything good with __attribute__(()) anyway. lint
was later fixed using ugly LONGLONG markup. Recent axing didn't remove this
markup AFAIR. Later, the __attribute__(()) was removed.
X endian.h(122): warning: extra bits set to 0 in conversion of 'unsigned int' to 'unsigned long long', op & [309]
X endian.h(122): warning: conversion to 'unsigned int' due to prototype, arg #1 [259]
X endian.h(122): warning: conversion to 'unsigned int' due to prototype, arg #1 [259]
X types.h(355): warning: conversion to 'unsigned int' due to prototype, arg #1 [259]
X types.h(355): warning: conversion to 'unsigned int' due to prototype, arg #1 [259]
Non-useful lint warnings. lint doesn't trust prototypes. -Wconversion in
gcc does much the same, but is not the default.
X _types.h(62): warning: struct __timer never defined [233]
X _types.h(63): warning: struct __mq never defined [233]
Worse non-useful lint warnings. lint doesn't understand forward declarations
of structs. These are a recondite C90 feature needed mainly to support
prototypes.
X endian.h(111): warning: static function __bswap64_var unused [236]
A serious problem. The function is inline, but inline functions are not in
C90, so lint defines away the inline keyword (it is spelled __inline).
Plain static functions in header files are unusual but not wrong.
X _pthreadtypes.h(44): warning: struct pthread never defined [233]
X _pthreadtypes.h(45): warning: struct pthread_attr never defined [233]
X _pthreadtypes.h(46): warning: struct pthread_cond never defined [233]
X _pthreadtypes.h(47): warning: struct pthread_cond_attr never defined [233]
X _pthreadtypes.h(48): warning: struct pthread_mutex never defined [233]
X _pthreadtypes.h(49): warning: struct pthread_mutex_attr never defined [233]
X _pthreadtypes.h(51): warning: struct pthread_rwlock never defined [233]
X _pthreadtypes.h(52): warning: struct pthread_rwlockattr never defined [233]
X _pthreadtypes.h(53): warning: struct pthread_barrier never defined [233]
X _pthreadtypes.h(54): warning: struct pthread_barrier_attr never defined [233]
X _pthreadtypes.h(55): warning: struct pthread_spinlock never defined [233]
X _pthreadtypes.h(78): warning: struct pthread_barrierattr never defined [233]
X types.h(247): warning: struct cap_rights never defined [233]
Lots more of lint not understanding forward declarations.
X types.h(313): warning: static function __bitcount16 unused [236]
X types.h(352): warning: static function __bitcount64 unused [236]
X Lint pass2:
More of lint not understanding static inline functions.
That was without __KERNEL. There was only 1 real bug (misuse of the long
long abomination). Missing lint markup was a feature since it allowed lint
to detect the bug.
Extra errors with _KERNEL:
X types.h(289): syntax error [249]
X [... 2 more undefined structs]
That is only 1 extra and already debugged.
It was mostly a user error to use lint on non-C90 code. sys/types.h should
be C90 if _ANSI_SOURCE or _POSIX_SOURCE is defined. But defining them makes
no difference to lint warnings.
-Wno-system-headers is supposed to be the default, so adding it for -p should
have no effect, but it actually increases the number of warnings by 5. These
are all "bitwise operation on signed value possibly unportable" in
sys/endian.h and sys/types.h. Now it is clang than doesn't understand C90.
In __bswap16_var() and __bitcount16() we start with uint16_t and
unsign-extend to signed int for use in expressions, then do bitwise
operations. C90's broken as designed value-preserving extension rules
give the signed int, and clang doesn't understand there are no problems
with the sign bit because the expressions barely need more than 16 bits
so they don't go near the sign bit.
Also, clang doesn't support -Wtraditional and lint doesn't have enough
control to select a working cpp. Lint supports a -B option, but this
only allows using alternative executables for the lint passes and not
the cpp pass.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20171118190422.V949>