svn commit: r287217 - head/usr.sbin/syslogd

[Bruce Evans]

On Sun, 30 Aug 2015, Jilles Tjoelker wrote:

> On Sun, Aug 30, 2015 at 11:53:00AM +0200, Ed Schouten wrote:
>> 2015-08-28 16:38 GMT+02:00 Joerg Sonnenberger <joerg at britannica.bec.de>:
>>> But the compiler can't tell if it is the *intention* that the function
>>> never returns. The warning behavior exists because that can easily
>>> change with macros etc.
>
>> I think it's important to keep in mind what this keyword was designed for.
>
>> The idea behind this attribute (and the C11 _Noreturn keyword) is to
>> allow for propagation of optimisation state across compilation units.
>> You use it to annotate functions in header files, so that the compiler
>> does not need to handle function return at the call site. This
>> knowledge can be automatically be inferred if the function is static.
>
> Although you are right philosophically, in practice there are some
> compilers and static analyzers that do benefit from noreturn attributes
> on static functions. In particular, gcc 4.2.1 generates better code and
> fewer warnings if static functions are annotated with noreturn if
> appropriate, and some versions of the Clang/LLVM static analyzer do not
> do any cross-procedural analysis at all and depend on noreturn
> attributes on static functions to avoid false positives.

If this were important, then __dead2 would be used a lot for important
functions, not for usage() and die().  It is in fact so important that
__dead2 is used a whole 86 times in $(find /usr/src -name *.c):
- 10 uses for non-static functions where there is actually a reason to
   use __dead2, but about half of these 10 are due to missing staticization
- 30 uses for static usage().  All of these uses have the correct syntax
   with __dead2 at the end (some versions of gcc only allow attributes
   near the end)
- 46 other uses.  Mostly for functions like panic(), exit() or err().
   18 of these have syntax errors with __dead2 not at the end (or near
   the end, preceding other attributes).  There are 10
   'static __dead2 void's, 7 'static void __dead2's, and 1
   '__dead2 static void'.

Non-FreeBSD spellings of __dead2 are more popular.  $(find /usr/src -name
*.c) has 200 lines matching 'oreturn'.  About 73 of these use the hard-coded
gccism __attribute(()).  The C11ish _Noreturn is used just 4 times (3 times
in rlogin for static functions that don't need it and 1 time in
libstdthreads).  Outside of contrib, there are just 30 lines matching
'oreturn': 11 in crypto/heimdal, 2 in routed/rtqery, 8 in tools/regression
for static usage(), 4 _Noreturns as above, 1 in xlint, 1 in pkill, 2 in
comments and 1 unrelated match.

> In practice this means that many compiler warnings need to be disabled
> for the affected compilers.

An example is the LLVM static checker in 2008, but hopefully not later
versions if this or any compiler ever used in FreeBSD.  errexit() in
echo/echo.c always compiled at WARNS=6 with gcc, but was changed in
2008 to define (sic) it as __dead2 with the excuse that this reduces
warnings with the static checker.  The change is quite broken:
- it has the syntax error 'static __dead2 void' -
__dead2 is applied to the definition of the function.  It doesn't
   take -funit-at-a-time to see when a static function which is defined
   before it is used doesn't return.  This application point also makes
   the syntax problems larger.  It is now natural to place the attribute
   declaration where it is, and there is a good chance that the old
   versions of gcc that didn't like it there also didn't like it being
   at the end.
The errexit() function has many style bugs:
- no prototype before the function.  This is not a large style bug.
   The function is unsorted before main() so that it can be its own
   protoype.  But this leaves no natural place to put the attribute.
- initialization in declaration - missing blank line after declarations
- garbage reason for existence of the function.  4.4BSD doesn't have
   the function or its style bugs, but just uses printf().  /bin/echo
   was "optimized" in FreeBSD to avoid using printf().  But with dynamic
   linkage, the space optimization is almost null, and with libc bloat
   for static linkage, the optimization doesn't work (/bin/echo has
   size 11K in my version of 5.2 where the bloat is smaller, but 415K
   in -current).
The main() function in echo/echo.c has rotted similarly.  To avoid
using printf() in a loop, it uses complicated allocation and writev().
This optimization is about 5% faster for 10000 args created by $(jot
10000 0), but most uses of echo are with only a couple of args.  With
10 args created by $(jot 10 0), the "optimization" is about 2% slower.

Bruce