tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

_DIAGASSERT(), noreturn attribute and static analysis



Hello,

I've been trying out the clang static analyzer, recently, and as it
is, NetBSD assert.h definitions don't play well with static
analysis. Basically, the analyzer always predicts the opposite of the
assertion predicate whenever it comes across an assert() or
_DIAGASSERT() call that is actually compiled (with NDEBUG undefined or
with _DIAGNOSTIC defined, respectively), which is pretty bad.

As far as NetBSD is concerned, it all boils down to the __assert(),
__assert13(), __diagassert() and __diagassert13() routines not being
declared __dead in assert.h. However, __diagassert() and
__diagassert13() are not dead, actually; but they ought to be
considered dead as far as analysis is concerned. LLVM has a special
attribute for that, it's called analyzer_noreturn.

The point of my post is: I think we should be nice to LLVM. :) (And
besides, having the static analyzer not spout loads of false positives
on NetBSD code without having to use an alternate system header would
be nice too!)

My suggestion is to either support analyzer_noreturn through
a #define, say __terminal in constrast to __dead, in sys/cdefs.h, the
same way we support other GCC-specific attributes, or conditionally
define __diagassert13() as __dead when, say, __lint__ is defined, and
have LLVM -D__lint__ when it does its analysis (which is actually not
quite straightforward, with their scripts as they are...).

The first solution would add something like this to sys/cdefs.h:

        #ifndef __has_feature
        #define __has_feature(x) 0
        #endif

        #ifndef __terminal
        #if __has_feature(attribute_analyzer_noreturn)
        #define __terminal __attribute__((analyzer_noreturn))
        #elif defined(__lint__)
        #define __terminal __dead
        #else
        #define __terminal
        #endif
        #endif  /* !__terminal */

This also defines __terminal to __dead as a fallback, in case we want
to run another static analyzer on it, since given the current state of
static analysis, such mispredictions are bound to happen if we don't
flag these functions appropriately.

Anyway, I just thought I'd bring that to your attention. But maybe we
don't want to support anymore compilers/tools than we already do in
sys/cdefs.h or something. What do you think?

--
Nhat Minh
minh%NetBSD.org@localhost


Home | Main Index | Thread Index | Old Index