GNOME Bugzilla – Bug 700268
Add support for using the clang analyzer
Last modified: 2018-05-24 15:18:43 UTC
In order to use the clang analyzer we need to mark up functions that are fatal as far as the analyzer is concerned. For example, a g_warning() should not be reached by well behaving code. Therefore when analyzing the analyzer should treat g_warning() as an abort(), ie: a function that does not return. We cannot use the 'standard' GCC noreturn attribute in these cases because then the compiler will generate incorrect code here. So we use the analyzer attribute to indicate this property to the analyzer but not the compiler. The G_ANALYZER_NORETURN attribute can be used to tag functions which don't return as far as the analyzer is concerned. The G_ANALYZER_ANALYZING is a define that is set to 1 when we an analyzer is statically analyzing the code. This patch sets it appropriately for the clang analyzer. However support for other analyzers can be added.
Created attachment 244132 [details] [review] Mark up warnings/critical functions for clang analyzer The clang code analyzer needs to know that functions like g_warning g_critical an g_return_if_fail should be seen by the analyzer in the same way as g_assert(). That is the analyzer should think they are fatal.
I disagree for g_warning(). I use g_warning() in libraries for "some unexpected error condition occurred outside of the program". Things like people speaking broken D-Bus protocols, etc. I consider this code reachable and (ideally) testable. g_critical() on the other hand, is as good as an abort in my opinion. g_warn_if_fail() and g_warn_if_reached() are really annoying here. These should really be firing criticals. Even the docs say: /** * g_warn_if_reached: * * Logs a critical warning.
(In reply to comment #2) > I disagree for g_warning(). I use g_warning() in libraries for "some > unexpected error condition occurred outside of the program". Things like > people speaking broken D-Bus protocols, etc. I consider this code reachable > and (ideally) testable. > > g_critical() on the other hand, is as good as an abort in my opinion. Interesting. I haven't used it that way, but it seems like that's a good distinction. > g_warn_if_fail() and g_warn_if_reached() are really annoying here. These > should really be firing criticals. Even the docs say: > > /** > * g_warn_if_reached: > * > * Logs a critical warning. Should I add a patch which changes this?
Created attachment 249182 [details] [review] Mark up warnings/critical functions for clang analyzer The clang code analyzer needs to know that functions like g_warning g_critical an g_return_if_fail should be seen by the analyzer in the same way as g_assert(). That is the analyzer should think they are fatal.
I've pushed this without the annotation on g_warning, since we didn't have agreement on that macro.
These macros break compilation of various glib using tools (e.g. Pango, GTK+) on OS X 10.7 and above, where clang is the default compiler. Bug #710044 has the build output from a Pango attempt. I have narrowed this down to only occuring when C++ is used - C code is perfectly OK. But whenever C++ code includes gmessages.h, this definition causes the compiler to fail: static void g_error (const gchar *format, ...) G_ANALYZER_NORETURN { va_list args; va_start (args, format); g_logv (G_LOG_DOMAIN, G_LOG_LEVEL_ERROR, format, args); va_end (args); for(;;) ; } with: /Users/paul/gtk/inst/include/glib-2.0/glib/gmessages.h:192:34: error: expected ';' after top level declarator ...) G_ANALYZER_NORETURN ^ ; I think that it is possible/likely that clang only allows these attributes on declarations, not definitions, when compiling C++, or has some other reservation about attaching them to static function definitions. It is possible that they have been used in the belief that clang would only be used for analysis, based on the #ifdef that would force the use of an alternate definition of g_error() and friends. That might be true on Linux, but is not the case on newer OS X platforms. Either way, the use of these macros in gmesssages.h prevent building pango and gtk2 on OS X 10.7 or later.
I removed the active definition of G_ANALYZER_ANALYZING and G_ANALYZER_NORETURN and things build without issues. These macros cannot be "turned on" simply because __has_feature(attribute_analyzer_noreturn) is true. Glib needs a configure-time option or some separate technique to enable these macros so that building with clang can work. Either that or a different way of declaring/defining the g_error() function in gmessages.h, but then the problem will just be punted till the next time someone does something similar.
re-opening.
(In reply to comment #6) > /Users/paul/gtk/inst/include/glib-2.0/glib/gmessages.h:192:34: error: expected > ';' after top level declarator > ...) G_ANALYZER_NORETURN That's bug 708793 and it's fixed in git master
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/701.