After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 741901 - Clang cannot know that g_error don't return
Clang cannot know that g_error don't return
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
2.43.x
Other FreeBSD
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
: 734128 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-12-23 12:15 UTC by Ting-Wei Lan
Modified: 2017-11-08 13:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gmacros: Only set G_ANALYZER_ANALYZING to 1 when clang static analyzer is in use (1.06 KB, patch)
2015-01-01 05:50 UTC, Ting-Wei Lan
none Details | Review
gmacros: Only set G_ANALYZER_ANALYZING to 1 when clang static analyzer is in use (1.07 KB, patch)
2015-01-01 12:42 UTC, Ting-Wei Lan
none Details | Review
gmessages: Add G_GNUC_NORETURN to g_error static function declaration (963 bytes, patch)
2015-02-18 17:25 UTC, Ting-Wei Lan
committed Details | Review
gmacros: Only set G_ANALYZER_ANALYZING to 1 when clang static analyzer is in use (1.10 KB, patch)
2015-03-27 15:05 UTC, Ting-Wei Lan
committed Details | Review

Description Ting-Wei Lan 2014-12-23 12:15:13 UTC
After -Werror=return-type was added to gnome-common a week ago, clang reports this type of error for a few modules:

vinagre/vinagre-dirs.c:207:1: error: control may reach end of non-void function [-Werror,-Wreturn-type]

In vinagre/vinagre-dirs.vala, the last line of the get_package_data_file function is error, which is translated to g_error, so it is not possible for this function to reach end and return an unknown value.


I found the declaration of g_error in the preprocessed file,:
static void g_error (const gchar *format, ...) __attribute__((analyzer_noreturn));

If I add _Noreturn to the declaration, the problem is fixed. I know we cannot simply do this in glib because C++ or older C compilers do not accept it.

Is there any reason that clang should use the static function? Or we should modify the #ifdef check so it can use the macro instead?
Comment 1 Ting-Wei Lan 2014-12-30 14:59:01 UTC
This simple program can be used to reproduce the problem:

#include <glib.h>                                                                
#include <stdio.h>                                                               
                                                                                 
static const char* get_string (int c) {                                          
    if (c > 1) {                                                                 
        return "Good";                                                           
    }                                                                            
    g_error ("Sorry");                                                           
}                                                                                
                                                                                 
int main (int argc, char *argv[]) {                                              
    puts (get_string (argc));                                                    
    return 0;                                                                    
}

Error message:

test.c:9:1: error: control may reach end of non-void function [-Werror,-Wreturn-type]
Comment 2 Ting-Wei Lan 2015-01-01 05:50:14 UTC
Created attachment 293536 [details] [review]
gmacros: Only set G_ANALYZER_ANALYZING to 1 when clang static analyzer is in use
Comment 3 Ting-Wei Lan 2015-01-01 12:42:07 UTC
Created attachment 293552 [details] [review]
gmacros: Only set G_ANALYZER_ANALYZING to 1 when clang static analyzer is in use

Updated patch which causes less warnings
Comment 4 Ting-Wei Lan 2015-02-15 10:12:31 UTC
Does someone have time to review the patch?
Comment 5 Stef Walter 2015-02-15 20:49:10 UTC
Comment on attachment 293552 [details] [review]
gmacros: Only set G_ANALYZER_ANALYZING to 1 when clang static analyzer is in use

Shouldn't we just use G_GNUC_NORETURN on g_error() since it's guaranteed to never return?
Comment 6 Ting-Wei Lan 2015-02-16 16:39:52 UTC
Yes, I think we can add G_GNUC_NORETURN, but the original problem is that G_ANALYZER_ANALYZING seems to be incorrect. Is there any reason that we should treat gcc and clang differently when the static analyzer is not used?
Comment 7 Ting-Wei Lan 2015-02-18 17:25:50 UTC
Created attachment 297121 [details] [review]
gmessages: Add G_GNUC_NORETURN to g_error static function declaration

This patch doesn't obsolete the previous patch. I still think that we should change G_ANALYZER_ANALYZING macro because either its name or its implementation is misleading.
Comment 8 Ting-Wei Lan 2015-03-27 15:05:36 UTC
Created attachment 300461 [details] [review]
gmacros: Only set G_ANALYZER_ANALYZING to 1 when clang static analyzer is in use

Update the patch to work with current master branch.
Comment 9 Stef Walter 2015-06-29 09:33:43 UTC
These look good to me.
Comment 10 Ting-Wei Lan 2015-06-29 14:18:03 UTC
Attachment 300461 [details] pushed as d7a1d89 - gmacros: Only set G_ANALYZER_ANALYZING to 1 when clang static analyzer is in use
Attachment 297121 [details] pushed as 93dadb1 - gmessages: Add G_GNUC_NORETURN to g_error static function declaration
Comment 11 Philip Withnall 2017-11-08 13:13:23 UTC
*** Bug 734128 has been marked as a duplicate of this bug. ***