cxong / cdogs-sdl

Classic overhead run-and-gun game
https://cxong.github.io/cdogs-sdl/
GNU General Public License v2.0
868 stars 115 forks source link

Build error with tinydir inlining #838

Open reinerh opened 4 months ago

reinerh commented 4 months ago

On Ubuntu build servers the build fails with this error:

[ 32%] Building C object src/cdogs/CMakeFiles/cdogs.dir/campaigns.c.o
cd /<<PKGBUILDDIR>>/obj-x86_64-linux-gnu/src/cdogs && /usr/bin/cc -DSTATIC -I/<<PKGBUILDDIR>>/src -I/<<PKGBUILDDIR>>/src/cdogs -I/<<PKGBUILDDIR>>/src/. -I/<<PKGBUILDDIR>>/src/proto/nanopb -I/<<PKGBUILDDIR>>/src/cdogs/.. -isystem /usr/include/SDL2 -g -O2 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -ffile-prefix-map=/<<PKGBUILDDIR>>=. -flto=auto -ffat-lto-objects -fstack-protector-strong -fstack-clash-protection -Wformat -Werror=format-security -fcf-protection -fdebug-prefix-map=/<<PKGBUILDDIR>>=/usr/src/cdogs-sdl-1.5.0+dfsg-1 -Wdate-time -D_FORTIFY_SOURCE=3 -I/usr/include/nanopb -g   -fsigned-char -Wall -W -Wstrict-prototypes -Wpointer-arith -Wcast-qual -g -freg-struct-return -std=gnu99 -Wshadow -Wimplicit-fallthrough=0 -Wno-error=format-overflow -Winline -Werror -MD -MT src/cdogs/CMakeFiles/cdogs.dir/campaigns.c.o -MF CMakeFiles/cdogs.dir/campaigns.c.o.d -o CMakeFiles/cdogs.dir/campaigns.c.o -c /<<PKGBUILDDIR>>/src/cdogs/campaigns.c
In file included from /<<PKGBUILDDIR>>/src/cdogs/campaigns.c:33:
In function ‘tinydir_open_sorted’,
    inlined from ‘LoadCampaignsFromFolder’ at /<<PKGBUILDDIR>>/src/cdogs/campaigns.c:248:6:
/<<PKGBUILDDIR>>/src/tinydir/tinydir.h:265:5: error: inlining failed in call to ‘tinydir_open’: --param max-inline-insns-single limit reached [-Werror=inline]
  265 | int tinydir_open(tinydir_dir *dir, const _tinydir_char_t *path)
      |     ^~~~~~~~~~~~
/<<PKGBUILDDIR>>/src/tinydir/tinydir.h:360:13: note: called from here
  360 |         if (tinydir_open(dir, path) == -1)
      |             ^~~~~~~~~~~~~~~~~~~~~~~
/<<PKGBUILDDIR>>/src/tinydir/tinydir.h:265:5: error: inlining failed in call to ‘tinydir_open’: --param max-inline-insns-single limit reached [-Werror=inline]
  265 | int tinydir_open(tinydir_dir *dir, const _tinydir_char_t *path)
      |     ^~~~~~~~~~~~
/<<PKGBUILDDIR>>/src/tinydir/tinydir.h:374:29: note: called from here
  374 |         if (n_files == 0 || tinydir_open(dir, path) == -1)
      |                             ^~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[3]: *** [src/cdogs/CMakeFiles/cdogs.dir/build.make:331: src/cdogs/CMakeFiles/cdogs.dir/campaigns.c.o] Error 1

In CMakeLists.txt -Werror and -Winline are set: https://github.com/cxong/cdogs-sdl/blob/92490cc1e30faf0178d6dea1857a2b653ffb0caf/CMakeLists.txt#L131

tinydir_open is declared as:

_TINYDIR_FUNC
int tinydir_open(tinydir_dir *dir, const _tinydir_char_t *path);

where _TINYDIR_FUNC is:

# define _TINYDIR_FUNC static inline

I'm not sure why it fails on Ubuntu, but succeeds on Debian builds (and also on CI builds here on Github). (maybe related to --param max-inline-insns-single limit reached that could be different?)

Is there a reason why the build explicitely sets -Winline and upgrades these warnings to errors?

cxong commented 4 months ago

we enable most warnings and set as error in order to be more strict. Is this something that can be fixed upstream in tinydir? Are its functions too big to inline?

reinerh commented 4 months ago

I just compared the build flags between Debian and Ubuntu and found that the difference that causes the problem is -D_FORTIFY_SOURCE=. In Debian that is set to 2 and in Ubuntu to 3. When I change it in the Debian build to 3, I get the same error. I guess that with a better fortification the code is also getting a bit larger and it reaches the inlining instructions limit. It probably should be addressed in tinydir upstream. For the cdogs-sdl package, I'll check if I can raise the limit.

reinerh commented 4 months ago

When I raise the inlining limit (-finline-limit=1000), the build continues but fails later with:

src/cdogs/utils.c: In function ‘RelPathFromCWD’:
src/cdogs/utils.c:152:17: error: ‘__builtin___strncat_chk’ output may be truncated copying 4095 bytes from a string of length 4095 [-Werror=stringop-truncation]
  152 |                 strncat(srcBuf, src, CDOGS_PATH_MAX - 1);
      |                 ^
src/cdogs/utils.c: In function ‘GetDataFilePath’:
src/cdogs/utils.c:152:17: error: ‘__builtin___strncat_chk’ output may be truncated copying 4095 bytes from a string of length 4095 [-Werror=stringop-truncation]

(this check also seems to come from -D_FORTIFY_SOURCE=3)

When I ignore that with -Wno-stringop-truncation the build succeeds.

reinerh commented 4 months ago

Nix is disabling fortifying with level 3:

https://github.com/NixOS/nixpkgs/blob/5863c27340ba4de8f83e7e3c023b9599c3cb3c80/pkgs/games/cdogs-sdl/default.nix#L53-L54

Gentoo, FreeBSD and OpenMandriva are patching out -Werror: https://gitweb.gentoo.org/repo/gentoo.git/tree/games-arcade/cdogs-sdl/files/cdogs-sdl-1.1.1-cmake.patch https://cgit.freebsd.org/ports/tree/games/cdogs-sdl/files/patch-CMakeLists.txt https://github.com/OpenMandrivaAssociation/cdogs-sdl/blob/master/cdogs-sdl-disable-werror.patch

cxong commented 4 months ago

which version of ubuntu/gcc runs into this error?

reinerh commented 4 months ago

Yes, 13.2.0, same on Debian unstable.

reinerh commented 4 months ago

After increasing the inline limit and ignoring stringop-truncation errors in the 2.0.0 Debian package, it also successfully built in Ubuntu and it made it just in time before the freeze for 24.04 LTS. :smile: