Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#1259 closed defect (fixed)

nginx build fails when using gcc7 compiler

Reported by: Dariusz Bogdanski Owned by:
Priority: minor Milestone:
Component: other Version: 1.9.x
Keywords: Cc:
uname -a: Linux dbrpi01 4.9.24-v7+ #991 SMP Sat Apr 22 20:26:57 BST 2017 armv7l GNU/Linux
nginx -V: N/A

Description

Nginx build fails when using gcc7 compiler. I have experienced it on Raspberry Pi running rasbian but given nature of the problem results will be the same on other platforms when gcc7 is used.

Example make error:

src/core/ngx_murmurhash.c:37:11: error: this statement may fall through [-Werror=implicit-fallthrough

h = data[2] << 16;
~

src/core/ngx_murmurhash.c:38:5: note: here

case 2:
~

src/core/ngx_murmurhash.c:39:11: error: this statement may fall through [-Werror=implicit-fallthrough=]

h = data[1] << 8;

src/core/ngx_murmurhash.c:40:5: note: here

case 1:
~ ==

Reason for compilation errors like these is that GCC 7 added (https://gcc.gnu.org/gcc-7/changes.html) a new -Wimplicit-fallthrough which generates a message for case statements that implicitly fallthrough to the next case.

Change History (7)

comment:1 by Maxim Dounin, 3 years ago

Try the following patch:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1493225474 -10800
#      Wed Apr 26 19:51:14 2017 +0300
# Node ID 9e1f56d88fb4e55504fb270db3b1dc642c8fecda
# Parent  7cfefb06849feaaffdb21b6cc6be68b4d1d9a9a5
Added missing "fall through" comments (ticket #1259).

Found by gcc7 (-Wimplicit-fallthrough).

diff --git a/src/core/ngx_murmurhash.c b/src/core/ngx_murmurhash.c
--- a/src/core/ngx_murmurhash.c
+++ b/src/core/ngx_murmurhash.c
@@ -35,8 +35,10 @@ ngx_murmur_hash2(u_char *data, size_t le
     switch (len) {
     case 3:
         h ^= data[2] << 16;
+        /* fall through */
     case 2:
         h ^= data[1] << 8;
+        /* fall through */
     case 1:
         h ^= data[0];
         h *= 0x5bd1e995;
diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c
--- a/src/http/ngx_http_parse.c
+++ b/src/http/ngx_http_parse.c
@@ -1390,6 +1390,7 @@ ngx_http_parse_complex_uri(ngx_http_requ
                 goto done;
             case '+':
                 r->plus_in_uri = 1;
+                /* fall through */
             default:
                 state = sw_usual;
                 *u++ = ch;
@@ -1431,6 +1432,7 @@ ngx_http_parse_complex_uri(ngx_http_requ
                 goto done;
             case '+':
                 r->plus_in_uri = 1;
+                /* fall through */
             default:
                 state = sw_usual;
                 *u++ = ch;
@@ -1478,6 +1480,7 @@ ngx_http_parse_complex_uri(ngx_http_requ
                 goto done;
             case '+':
                 r->plus_in_uri = 1;
+                /* fall through */
             default:
                 state = sw_usual;
                 *u++ = ch;
diff --git a/src/os/unix/ngx_process.c b/src/os/unix/ngx_process.c
--- a/src/os/unix/ngx_process.c
+++ b/src/os/unix/ngx_process.c
@@ -413,6 +413,7 @@ ngx_signal_handler(int signo, siginfo_t 
                 break;
             }
             ngx_debug_quit = 1;
+            /* fall through */
         case ngx_signal_value(NGX_SHUTDOWN_SIGNAL):
             ngx_quit = 1;
             action = ", shutting down";
Last edited 3 years ago by Maxim Dounin (previous) (diff)

comment:2 by Dariusz Bogdanski, 3 years ago

Thanks. With your patch it compiles without any issues with gcc7

Just curious why nginx has -Werror enabled.. It breaks builds for new compilers with new warnings introduced. IMHO it makes perfect sense during development but not for code to be shipped. Without -Werror it would be more future proof...

Last edited 3 years ago by Dariusz Bogdanski (previous) (diff)

comment:3 by Maxim Dounin, 3 years ago

Thanks for testing.

Without -Werror it is very easy to miss important warnings, including ones that only appear on some exotic platforms developers do not have access to, so we prefer to always use -Werror. Not to mention that there is no real difference between "development" and "code to be shipped". If needed, -Werror can be easily disabled with ./configure --with-cc-opt="-Wno-error".

comment:4 by Dariusz Bogdanski, 3 years ago

Thank you for the explanation.

comment:5 by Maxim Dounin <mdounin@…>, 3 years ago

In 6994:f38647c651a8/nginx:

Added missing "fall through" comments (ticket #1259).

Found by gcc7 (-Wimplicit-fallthrough).

comment:6 by Maxim Dounin, 3 years ago

Resolution: fixed
Status: newclosed

Fix committed, thanks.

comment:7 by Maxim Dounin <mdounin@…>, 3 years ago

In 7136:d409ab1e8e4d/nginx:

Added missing "fall through" comments (ticket #1259).

Found by gcc7 (-Wimplicit-fallthrough).

Note: See TracTickets for help on using tickets.