GCC 7.1 Build Errors - Githubissues

CruzR commented 7 years ago

GCC 7 adds a couple of new warnings, two of which apply to C-Dogs SDL source code:

--Wimplicit-fallthrough: Warn when a switch case falls through.
--Wformat-overflow: Warn about calls to formatted input/output functions such as sprintf and vsprintf that might overflow the destination buffer.

Because C-Dogs SDL is compiled with -Wall -Werror, compilation fails when using GCC 7.1.
Compiling with errors for these two warnings disabled works and yields the following output (irrelevant warnings removed):

[  0%] Building C object src/json/CMakeFiles/json.dir/json.c.o
/home/thomas/projects/cdogs-sdl/src/json/json.c: In function ‘lexer’:
/home/thomas/projects/cdogs-sdl/src/json/json.c:1875:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
      ++*p;
      ^~~~
/home/thomas/projects/cdogs-sdl/src/json/json.c:1876:5: note: here
     case ']':
     ^~~~
/home/thomas/projects/cdogs-sdl/src/json/json.c:1914:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
      ++*p;
      ^~~~
/home/thomas/projects/cdogs-sdl/src/json/json.c:1915:5: note: here
     case ']':
     ^~~~
/home/thomas/projects/cdogs-sdl/src/json/json.c:1996:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
      ++*p;
      ^~~~
/home/thomas/projects/cdogs-sdl/src/json/json.c:1997:5: note: here
     case ']':
     ^~~~
/home/thomas/projects/cdogs-sdl/src/json/json.c:2107:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
      ++*p;
      ^~~~
/home/thomas/projects/cdogs-sdl/src/json/json.c:2108:5: note: here
     case ']':
     ^~~~
[  8%] Building C object src/cdogs/yajl/CMakeFiles/yajl_s.dir/yajl_parser.c.o
/home/thomas/projects/cdogs-sdl/src/cdogs/yajl/yajl_parser.c: In function ‘yajl_do_parse’:
/home/thomas/projects/cdogs-sdl/src/cdogs/yajl/yajl_parser.c:334:24: warning: this statement may fall through [-Wimplicit-fallthrough=]
                     if (yajl_bs_current(hand->stateStack) ==
                        ^
/home/thomas/projects/cdogs-sdl/src/cdogs/yajl/yajl_parser.c:347:17: note: here
                 case yajl_tok_colon:
                 ^~~~
/home/thomas/projects/cdogs-sdl/src/cdogs/yajl/yajl_parser.c:405:24: warning: this statement may fall through [-Wimplicit-fallthrough=]
                     if (yajl_bs_current(hand->stateStack) ==
                        ^
/home/thomas/projects/cdogs-sdl/src/cdogs/yajl/yajl_parser.c:414:17: note: here
                 default:
                 ^~~~~~~
[ 25%] Building C object src/cdogs/CMakeFiles/cdogs.dir/draw/char_sprites.c.o
/home/thomas/projects/cdogs-sdl/src/cdogs/draw/char_sprites.c: In function ‘CharSpriteClassesLoadDir’:
/home/thomas/projects/cdogs-sdl/src/cdogs/draw/char_sprites.c:113:15: warning: ‘/data.json’ directive writing 10 bytes into a region of size between 1 and 4096 [-Wformat-overflow=]
  sprintf(buf, "%s/data.json", path);
               ^~~~~~~~~~~~~~
/home/thomas/projects/cdogs-sdl/src/cdogs/draw/char_sprites.c:113:2: note: ‘sprintf’ output between 11 and 4106 bytes into a destination of size 4096
  sprintf(buf, "%s/data.json", path);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[ 50%] Building C object src/cdogs/CMakeFiles/cdogs.dir/utils.c.o
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c: In function ‘Div8Str’:
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:16: warning: ‘%d’ directive writing between 1 and 10 bytes into a region of size 8 [-Wformat-overflow=]
  sprintf(buf, "%d", i/8);
                ^~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:15: note: directive argument in the range [-268435456, 268435455]
  sprintf(buf, "%d", i/8);
               ^~~~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:2: note: ‘sprintf’ output between 2 and 11 bytes into a destination of size 8
  sprintf(buf, "%d", i/8);
  ^~~~~~~~~~~~~~~~~~~~~~~
[ 54%] Building C object src/cdogsed/CMakeFiles/cdogsedlib.dir/char_editor.c.o
In file included from /home/thomas/projects/cdogs-sdl/src/cdogsed/char_editor.c:45:0:
/home/thomas/projects/cdogs-sdl/src/nuklear/nuklear.h: In function ‘nk_murmur_hash’:
/home/thomas/projects/cdogs-sdl/src/nuklear/nuklear.h:5140:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
     case 3: k1 ^= (nk_uint)(tail[2] << 16);
             ~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/thomas/projects/cdogs-sdl/src/nuklear/nuklear.h:5141:5: note: here
     case 2: k1 ^= (nk_uint)(tail[1] << 8u);
     ^~~~
/home/thomas/projects/cdogs-sdl/src/nuklear/nuklear.h:5141:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
     case 2: k1 ^= (nk_uint)(tail[1] << 8u);
             ~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/thomas/projects/cdogs-sdl/src/nuklear/nuklear.h:5142:5: note: here
     case 1: k1 ^= tail[0];
     ^~~~
[ 61%] Building C object src/CMakeFiles/cdogs-sdl.dir/command_line.c.o
/home/thomas/projects/cdogs-sdl/src/command_line.c: In function ‘ParseArgs’:
/home/thomas/projects/cdogs-sdl/src/command_line.c:214:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
    {
    ^
/home/thomas/projects/cdogs-sdl/src/command_line.c:237:3: note: here
   default:
   ^~~~~~~
[ 63%] Building C object src/CMakeFiles/cdogs-sdl.dir/hiscores.c.o
/home/thomas/projects/cdogs-sdl/src/hiscores.c: In function ‘HighScoreDraw’:
/home/thomas/projects/cdogs-sdl/src/hiscores.c:210:14: warning: ‘%d’ directive writing between 1 and 11 bytes into a region of size 10 [-Wformat-overflow=]
  sprintf(s, "%d.", idx + 1);
              ^~
/home/thomas/projects/cdogs-sdl/src/hiscores.c:210:13: note: directive argument in the range [-2147483647, 20]
  sprintf(s, "%d.", idx + 1);
             ^~~~~
/home/thomas/projects/cdogs-sdl/src/hiscores.c:210:2: note: ‘sprintf’ output between 3 and 13 bytes into a destination of size 10
  sprintf(s, "%d.", idx + 1);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~
[ 69%] Building C object src/cdogs/yajl/CMakeFiles/yajl.dir/yajl_parser.c.o
/home/thomas/projects/cdogs-sdl/src/cdogs/yajl/yajl_parser.c: In function ‘yajl_do_parse’:
/home/thomas/projects/cdogs-sdl/src/cdogs/yajl/yajl_parser.c:334:24: warning: this statement may fall through [-Wimplicit-fallthrough=]
                     if (yajl_bs_current(hand->stateStack) ==
                        ^
/home/thomas/projects/cdogs-sdl/src/cdogs/yajl/yajl_parser.c:347:17: note: here
                 case yajl_tok_colon:
                 ^~~~
/home/thomas/projects/cdogs-sdl/src/cdogs/yajl/yajl_parser.c:405:24: warning: this statement may fall through [-Wimplicit-fallthrough=]
                     if (yajl_bs_current(hand->stateStack) ==
                        ^
/home/thomas/projects/cdogs-sdl/src/cdogs/yajl/yajl_parser.c:414:17: note: here
                 default:
                 ^~~~~~~
[ 77%] Building C object src/tests/CMakeFiles/config_test.dir/__/cdogs/utils.c.o
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c: In function ‘Div8Str’:
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:16: warning: ‘%d’ directive writing between 1 and 10 bytes into a region of size 8 [-Wformat-overflow=]
  sprintf(buf, "%d", i/8);
                ^~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:15: note: directive argument in the range [-268435456, 268435455]
  sprintf(buf, "%d", i/8);
               ^~~~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:2: note: ‘sprintf’ output between 2 and 11 bytes into a destination of size 8
  sprintf(buf, "%d", i/8);
  ^~~~~~~~~~~~~~~~~~~~~~~
[ 80%] Building C object src/tests/CMakeFiles/c_array_test.dir/__/cdogs/utils.c.o
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c: In function ‘Div8Str’:
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:16: warning: ‘%d’ directive writing between 1 and 10 bytes into a region of size 8 [-Wformat-overflow=]
  sprintf(buf, "%d", i/8);
                ^~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:15: note: directive argument in the range [-268435456, 268435455]
  sprintf(buf, "%d", i/8);
               ^~~~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:2: note: ‘sprintf’ output between 2 and 11 bytes into a destination of size 8
  sprintf(buf, "%d", i/8);
  ^~~~~~~~~~~~~~~~~~~~~~~
[ 84%] Building C object src/tests/CMakeFiles/autosave_test.dir/__/cdogs/utils.c.o
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c: In function ‘Div8Str’:
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:16: warning: ‘%d’ directive writing between 1 and 10 bytes into a region of size 8 [-Wformat-overflow=]
  sprintf(buf, "%d", i/8);
                ^~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:15: note: directive argument in the range [-268435456, 268435455]
  sprintf(buf, "%d", i/8);
               ^~~~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:2: note: ‘sprintf’ output between 2 and 11 bytes into a destination of size 8
  sprintf(buf, "%d", i/8);
  ^~~~~~~~~~~~~~~~~~~~~~~
[ 88%] Building C object src/tests/CMakeFiles/pic_test.dir/__/cdogs/utils.c.o
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c: In function ‘Div8Str’:
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:16: warning: ‘%d’ directive writing between 1 and 10 bytes into a region of size 8 [-Wformat-overflow=]
  sprintf(buf, "%d", i/8);
                ^~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:15: note: directive argument in the range [-268435456, 268435455]
  sprintf(buf, "%d", i/8);
               ^~~~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:2: note: ‘sprintf’ output between 2 and 11 bytes into a destination of size 8
  sprintf(buf, "%d", i/8);
  ^~~~~~~~~~~~~~~~~~~~~~~
[ 90%] Building C object src/tests/CMakeFiles/json_test.dir/__/cdogs/utils.c.o
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c: In function ‘Div8Str’:
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:16: warning: ‘%d’ directive writing between 1 and 10 bytes into a region of size 8 [-Wformat-overflow=]
  sprintf(buf, "%d", i/8);
                ^~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:15: note: directive argument in the range [-268435456, 268435455]
  sprintf(buf, "%d", i/8);
               ^~~~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:2: note: ‘sprintf’ output between 2 and 11 bytes into a destination of size 8
  sprintf(buf, "%d", i/8);
  ^~~~~~~~~~~~~~~~~~~~~~~
[ 94%] Building C object src/tests/CMakeFiles/minkowski_hex_test.dir/__/cdogs/utils.c.o
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c: In function ‘Div8Str’:
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:16: warning: ‘%d’ directive writing between 1 and 10 bytes into a region of size 8 [-Wformat-overflow=]
  sprintf(buf, "%d", i/8);
                ^~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:15: note: directive argument in the range [-268435456, 268435455]
  sprintf(buf, "%d", i/8);
               ^~~~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:2: note: ‘sprintf’ output between 2 and 11 bytes into a destination of size 8
  sprintf(buf, "%d", i/8);
  ^~~~~~~~~~~~~~~~~~~~~~~
[ 98%] Building C object src/tests/CMakeFiles/player_test.dir/__/cdogs/utils.c.o
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c: In function ‘Div8Str’:
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:16: warning: ‘%d’ directive writing between 1 and 10 bytes into a region of size 8 [-Wformat-overflow=]
  sprintf(buf, "%d", i/8);
                ^~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:15: note: directive argument in the range [-268435456, 268435455]
  sprintf(buf, "%d", i/8);
               ^~~~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:2: note: ‘sprintf’ output between 2 and 11 bytes into a destination of size 8
  sprintf(buf, "%d", i/8);
  ^~~~~~~~~~~~~~~~~~~~~~~
[100%] Building C object src/tests/CMakeFiles/utils_test.dir/__/cdogs/utils.c.o
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c: In function ‘Div8Str’:
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:16: warning: ‘%d’ directive writing between 1 and 10 bytes into a region of size 8 [-Wformat-overflow=]
  sprintf(buf, "%d", i/8);
                ^~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:15: note: directive argument in the range [-268435456, 268435455]
  sprintf(buf, "%d", i/8);
               ^~~~
/home/thomas/projects/cdogs-sdl/src/cdogs/utils.c:442:2: note: ‘sprintf’ output between 2 and 11 bytes into a destination of size 8
  sprintf(buf, "%d", i/8);
  ^~~~~~~~~~~~~~~~~~~~~~~

CruzR commented 7 years ago

I'd be willing to prepare a pull-request to fix these issues.

The --Wimplicit-fallthrough warnings can be silenced by adding a comment like /* Intentionally falls through */ to the case statements. (Of course we should first ensure that the fall-through actually is intentional in all cases.)

The --Wformat-overflow warnings could probably either be silenced by increasing the buffer sizes to the amount suggested by GCC, or by using one of the safe formatting functions (snprintf, ...) instead.

cxong commented 7 years ago

Thanks for finding and reporting this 👍 I'll go through these in detail later, here are my thoughts so far:

I don't use GCC 7 yet so to prevent these from reappearing, will need to add the compiler to the CI mix. Is it possible to configure Travis CI to use GCC 7? See .travis.yml
Some of the errors are from 3rd party libraries; these problems should be reported upstream, and silence the warnings here in the meantime
Some are genuine buffer overflow problems, I've been lax about these but if they can be automatically detected now, it is best to fix them rather than ignore the warning. I prefer fixing warnings than ignoring them.

cxong commented 7 years ago

The warnings are disabled for now

cxong commented 6 years ago

Some of the buffer overflow warnings are because we are doing sprintf with static sized buffers intended to store paths, then doing operations like appending. The logic is that the buffer should be big enough to store any valid path. To fix this we would need to move to dynamic buffers and use functions like snprintf throughout. Will take a bit of refactoring. Perhaps this is a good chance to look at a C string library?

cxong commented 4 years ago

Going to close this as further fixes will be made for #615

cxong / cdogs-sdl

GCC 7.1 Build Errors #509