Aleph-One-Marathon / alephone

Aleph One is the open source continuation of Bungie’s Marathon 2 game engine.
https://alephone.lhowon.org/
GNU General Public License v3.0
621 stars 100 forks source link

Build fails with modern compilers #474

Closed eli-schwartz closed 3 months ago

eli-schwartz commented 3 months ago

https://wiki.gentoo.org/wiki/Modern_C_porting#How_do_I_reproduce_these_bugs.3F

GCC 14 (not yet released but forthcoming) and clang 16 have started to error on invalid C code that was removed from the standard in c99 or even earlier. Historically, compilers accepted this code even though it was invalid in an effort to avoid breaking really old codebases. Due to the passing of time and an increasing understanding of just how dangerous this invalid code is, that leniency is going away.

Note that this is not cosmetic. It is undefined behavior and compilers will (increasingly) optimize it in weird and wonderful ways that break the original intention of the code. It will also obviously mean that the code cannot be built on newer systems with updated compilers.

An easy way to reproduce the problem even on older compiler versions is to build with the following *FLAGS:

-Werror=implicit-function-declaration -Werror=implicit-int -Werror=int-conversion -Werror=incompatible-pointer-types

Gentoo has been testing these flags in CI across the entire distro; alephone failed to build. Using the latest alephone commit (117c6949a072a6ff0e9be03a2027a12fe5797e95), I reproduced this build error:

x86_64-pc-linux-gnu-gcc -DHAVE_CONFIG_H -I. -I../..  -I../../Source_Files/CSeries -I../../Source_Files/Files -I../../Source_Files/GameWorld -I../../Source_Files/Input -I../../Source_Files/Misc -I../../Source_Files/ModelView -I../../Source_Files/Network -I../../Source_Files/Sound -I../../Source_Files/RenderMain -I../../Source_Files/RenderOther -I../../Source_Files/XML -I../../Source_Files -D__STDC_CONSTANT_MACROS -I/usr/include/libpng16   -I/usr/include/SDL2 -D_REENTRANT -I/usr/include/libpng16 -I/usr/include/AL -I/usr/include/opus  -I/usr/include/SDL2 -D_REENTRANT -I/usr/include/SDL2 -D_REENTRANT -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/freetype2 -I/usr/include/harfbuzz  -I/usr/include/SDL2 -D_REENTRANT -DSDL -std=c99 -march=native -fstack-protector-all -O2 -pipe -fdiagnostics-color=always -frecord-gcc-switches -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-clash-protection -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing  -Wformat -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=int-conversion -Werror=incompatible-pointer-types -c -o SDL_ffmpeg.o SDL_ffmpeg.c
SDL_ffmpeg.c: In function ‘SDL_ffmpegOpen’:
SDL_ffmpeg.c:380:34: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  380 |                 AVCodec *codec = avcodec_find_decoder( stream->_ffmpeg->codecpar->codec_id );
      |                                  ^~~~~~~~~~~~~~~~~~~~
SDL_ffmpeg.c:430:34: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  430 |                 AVCodec *codec = avcodec_find_decoder( file->_ffmpeg->streams[i]->codecpar->codec_id );
      |                                  ^~~~~~~~~~~~~~~~~~~~
SDL_ffmpeg.c:447:21: warning: ‘channel_layout’ is deprecated [-Wdeprecated-declarations]
  447 |                     int channel_layout = stream->_ffmpeg->codecpar->channel_layout ? stream->_ffmpeg->codecpar->channel_layout :
      |                     ^~~
In file included from /usr/include/libavcodec/avcodec.h:42,
                 from SDL_ffmpeg.c:49:
/usr/include/libavcodec/codec_par.h:167:14: note: declared here
  167 |     uint64_t channel_layout;
      |              ^~~~~~~~~~~~~~
SDL_ffmpeg.c:447:21: warning: ‘channel_layout’ is deprecated [-Wdeprecated-declarations]
  447 |                     int channel_layout = stream->_ffmpeg->codecpar->channel_layout ? stream->_ffmpeg->codecpar->channel_layout :
      |                     ^~~
/usr/include/libavcodec/codec_par.h:167:14: note: declared here
  167 |     uint64_t channel_layout;
      |              ^~~~~~~~~~~~~~
SDL_ffmpeg.c:448:25: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
  448 |                         (stream->_ffmpeg->codecpar->channels == 2 ? AV_CH_LAYOUT_STEREO : AV_CH_LAYOUT_MONO);
      |                         ^
/usr/include/libavcodec/codec_par.h:173:14: note: declared here
  173 |     int      channels;
      |              ^~~~~~~~
SDL_ffmpeg.c:450:21: warning: ‘swr_alloc_set_opts’ is deprecated [-Wdeprecated-declarations]
  450 |                     stream->swr_context = swr_alloc_set_opts(stream->swr_context, channel_layout, AV_SAMPLE_FMT_FLT,
      |                     ^~~~~~
In file included from SDL_ffmpeg.c:54:
/usr/include/libswresample/swresample.h:260:20: note: declared here
  260 | struct SwrContext *swr_alloc_set_opts(struct SwrContext *s,
      |                    ^~~~~~~~~~~~~~~~~~
SDL_ffmpeg.c: In function ‘SDL_ffmpegCreate’:
SDL_ffmpeg.c:500:24: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  500 |     file->_ffmpeg->url = filename;
      |                        ^
SDL_ffmpeg.c: In function ‘SDL_ffmpegAddAudioFrame’:
SDL_ffmpeg.c:651:5: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
  651 |     int32_t read_samples = frame->size / (av_get_bytes_per_sample(file->audioStream->audioFormat) * acodec->channels);
      |     ^~~~~~~
/usr/include/libavcodec/avcodec.h:1042:9: note: declared here
 1042 |     int channels;
      |         ^~~~~~~~
SDL_ffmpeg.c:668:5: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
  668 |     audio_frame->channels = acodec->channels;
      |     ^~~~~~~~~~~
In file included from /usr/include/libavcodec/avcodec.h:35:
/usr/include/libavutil/frame.h:662:9: note: declared here
  662 |     int channels;
      |         ^~~~~~~~
SDL_ffmpeg.c:668:5: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
  668 |     audio_frame->channels = acodec->channels;
      |     ^~~~~~~~~~~
/usr/include/libavcodec/avcodec.h:1042:9: note: declared here
 1042 |     int channels;
      |         ^~~~~~~~
SDL_ffmpeg.c:670:5: warning: ‘channel_layout’ is deprecated [-Wdeprecated-declarations]
  670 |     audio_frame->channel_layout = acodec->channel_layout;
      |     ^~~~~~~~~~~
/usr/include/libavutil/frame.h:524:14: note: declared here
  524 |     uint64_t channel_layout;
      |              ^~~~~~~~~~~~~~
SDL_ffmpeg.c:670:5: warning: ‘channel_layout’ is deprecated [-Wdeprecated-declarations]
  670 |     audio_frame->channel_layout = acodec->channel_layout;
      |     ^~~~~~~~~~~
/usr/include/libavcodec/avcodec.h:1100:14: note: declared here
 1100 |     uint64_t channel_layout;
      |              ^~~~~~~~~~~~~~
SDL_ffmpeg.c:678:5: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
  678 |     int asize = avcodec_fill_audio_frame(audio_frame, acodec->channels,
      |     ^~~
/usr/include/libavcodec/avcodec.h:1042:9: note: declared here
 1042 |     int channels;
      |         ^~~~~~~~
SDL_ffmpeg.c:681:9: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
  681 |         write_samples * write_bps * acodec->channels, 1);
      |         ^~~~~~~~~~~~~
/usr/include/libavcodec/avcodec.h:1042:9: note: declared here
 1042 |     int channels;
      |         ^~~~~~~~
SDL_ffmpeg.c: In function ‘SDL_ffmpegCreateAudioFrame’:
SDL_ffmpeg.c:765:9: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
  765 |         bytes = file->audioStream->encodeAudioInputSize * av_get_bytes_per_sample(file->audioStream->audioFormat) * file->audioStream->_ctx->channels;
      |         ^~~~~
/usr/include/libavcodec/avcodec.h:1042:9: note: declared here
 1042 |     int channels;
      |         ^~~~~~~~
SDL_ffmpeg.c:768:9: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
  768 |         if (av_samples_alloc_array_and_samples(&frame->conversionBuffer, NULL, file->audioStream->_ctx->channels, file->audioStream->encodeAudioInputSize, file->audioStream->_ctx->sample_fmt, 0) < 0)
      |         ^~
/usr/include/libavcodec/avcodec.h:1042:9: note: declared here
 1042 |     int channels;
      |         ^~~~~~~~
SDL_ffmpeg.c: In function ‘SDL_ffmpegGetAudioSpec’:
SDL_ffmpeg.c:1358:9: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
 1358 |         spec.channels = ( uint8_t )file->audioStream->_ctx->channels;
      |         ^~~~
/usr/include/libavcodec/avcodec.h:1042:9: note: declared here
 1042 |     int channels;
      |         ^~~~~~~~
SDL_ffmpeg.c: In function ‘SDL_ffmpegAddAudioStream’:
SDL_ffmpeg.c:1690:5: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
 1690 |     stream->codecpar->channels = codec.channels;
      |     ^~~~~~
/usr/include/libavcodec/codec_par.h:173:14: note: declared here
  173 |     int      channels;
      |              ^~~~~~~~
SDL_ffmpeg.c:1691:5: warning: ‘channel_layout’ is deprecated [-Wdeprecated-declarations]
 1691 |     stream->codecpar->channel_layout = codec.channels == 2 ? AV_CH_LAYOUT_STEREO : AV_CH_LAYOUT_MONO;
      |     ^~~~~~
/usr/include/libavcodec/codec_par.h:167:14: note: declared here
  167 |     uint64_t channel_layout;
      |              ^~~~~~~~~~~~~~
SDL_ffmpeg.c:1737:9: warning: ‘swr_alloc_set_opts’ is deprecated [-Wdeprecated-declarations]
 1737 |         str->swr_context = swr_alloc_set_opts(str->swr_context, context->channel_layout, context->sample_fmt, context->sample_rate,
      |         ^~~
/usr/include/libswresample/swresample.h:260:20: note: declared here
  260 | struct SwrContext *swr_alloc_set_opts(struct SwrContext *s,
      |                    ^~~~~~~~~~~~~~~~~~
SDL_ffmpeg.c:1737:9: warning: ‘channel_layout’ is deprecated [-Wdeprecated-declarations]
 1737 |         str->swr_context = swr_alloc_set_opts(str->swr_context, context->channel_layout, context->sample_fmt, context->sample_rate,
      |         ^~~
/usr/include/libavcodec/avcodec.h:1100:14: note: declared here
 1100 |     uint64_t channel_layout;
      |              ^~~~~~~~~~~~~~
SDL_ffmpeg.c:1738:13: warning: ‘channel_layout’ is deprecated [-Wdeprecated-declarations]
 1738 |             context->channel_layout, str->audioFormat, context->sample_rate, 0, NULL);
      |             ^~~~~~~
/usr/include/libavcodec/avcodec.h:1100:14: note: declared here
 1100 |     uint64_t channel_layout;
      |              ^~~~~~~~~~~~~~
SDL_ffmpeg.c:1748:9: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
 1748 |         str->sampleBufferSize = av_samples_get_buffer_size(0, stream->codecpar->channels, stream->codecpar->frame_size, AV_SAMPLE_FMT_FLT, 0);
      |         ^~~
/usr/include/libavcodec/codec_par.h:173:14: note: declared here
  173 |     int      channels;
      |              ^~~~~~~~
SDL_ffmpeg.c:1750:9: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
 1750 |         if (av_samples_alloc(&str->sampleBuffer, 0, stream->codecpar->channels, stream->codecpar->frame_size, AV_SAMPLE_FMT_FLT, 0) < 0)
      |         ^~
/usr/include/libavcodec/codec_par.h:173:14: note: declared here
  173 |     int      channels;
      |              ^~~~~~~~
SDL_ffmpeg.c:1750:30: error: passing argument 1 of ‘av_samples_alloc’ from incompatible pointer type [-Werror=incompatible-pointer-types]
 1750 |         if (av_samples_alloc(&str->sampleBuffer, 0, stream->codecpar->channels, stream->codecpar->frame_size, AV_SAMPLE_FMT_FLT, 0) < 0)
      |                              ^~~~~~~~~~~~~~~~~~
      |                              |
      |                              int8_t ** {aka signed char **}
In file included from /usr/include/libavcodec/avcodec.h:30:
/usr/include/libavutil/samplefmt.h:223:32: note: expected ‘uint8_t **’ {aka ‘unsigned char **’} but argument is of type ‘int8_t **’ {aka ‘signed char **’}
  223 | int av_samples_alloc(uint8_t **audio_data, int *linesize, int nb_channels,
      |                      ~~~~~~~~~~^~~~~~~~~~
SDL_ffmpeg.c:1760:13: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
 1760 |             str->encodeAudioInputSize = str->sampleBufferSize / stream->codecpar->channels;
      |             ^~~
/usr/include/libavcodec/codec_par.h:173:14: note: declared here
  173 |     int      channels;
      |              ^~~~~~~~
SDL_ffmpeg.c: In function ‘SDL_ffmpegDecodeAudioFrame’:
SDL_ffmpeg.c:1949:5: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
 1949 |     int channels = file->audioStream->_ctx->channels;
      |     ^~~
/usr/include/libavcodec/avcodec.h:1042:9: note: declared here
 1042 |     int channels;
      |         ^~~~~~~~
SDL_ffmpeg.c:2024:9: warning: ‘channel_layout’ is deprecated [-Wdeprecated-declarations]
 2024 |         dframe->channel_layout |= dframe->channels == 2 ? AV_CH_LAYOUT_STEREO : AV_CH_LAYOUT_MONO;
      |         ^~~~~~
/usr/include/libavutil/frame.h:524:14: note: declared here
  524 |     uint64_t channel_layout;
      |              ^~~~~~~~~~~~~~
SDL_ffmpeg.c:2024:9: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
 2024 |         dframe->channel_layout |= dframe->channels == 2 ? AV_CH_LAYOUT_STEREO : AV_CH_LAYOUT_MONO;
      |         ^~~~~~
/usr/include/libavutil/frame.h:662:9: note: declared here
  662 |     int channels;
      |         ^~~~~~~~
SDL_ffmpeg.c:2026:9: warning: ‘channel_layout’ is deprecated [-Wdeprecated-declarations]
 2026 |         convertedFrame->channel_layout = dframe->channel_layout;
      |         ^~~~~~~~~~~~~~
/usr/include/libavutil/frame.h:524:14: note: declared here
  524 |     uint64_t channel_layout;
      |              ^~~~~~~~~~~~~~
SDL_ffmpeg.c:2026:9: warning: ‘channel_layout’ is deprecated [-Wdeprecated-declarations]
 2026 |         convertedFrame->channel_layout = dframe->channel_layout;
      |         ^~~~~~~~~~~~~~
/usr/include/libavutil/frame.h:524:14: note: declared here
  524 |     uint64_t channel_layout;
      |              ^~~~~~~~~~~~~~
SDL_ffmpeg.c:2039:9: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
 2039 |         int data_size = av_samples_get_buffer_size(&plane_size, convertedFrame->channels, convertedFrame->nb_samples, convertedFrame->format, 1);
      |         ^~~
/usr/include/libavutil/frame.h:662:9: note: declared here
  662 |     int channels;
      |         ^~~~~~~~
SDL_ffmpeg.c:2043:9: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
 2043 |         if (planar && convertedFrame->channels > 1)
      |         ^~
/usr/include/libavutil/frame.h:662:9: note: declared here
  662 |     int channels;
      |         ^~~~~~~~
SDL_ffmpeg.c:2047:13: warning: ‘channels’ is deprecated [-Wdeprecated-declarations]
 2047 |             for (ch = 1; ch < convertedFrame->channels; ch++)
      |             ^~~
/usr/include/libavutil/frame.h:662:9: note: declared here
  662 |     int channels;
      |         ^~~~~~~~
cc1: some warnings being treated as errors
make[3]: *** [Makefile:410: SDL_ffmpeg.o] Error 1

Original downstream report: https://bugs.gentoo.org/921027 Full build logs (against latest commit): build.log

treellama commented 3 months ago

This has nothing to do with modern compilers, undefined behavior, or legacy C code. You're promoting deprecation warnings in the ffmpeg API to errors. If you either stop promoting deprecations to errors, or compile against an older ffmpeg, it should compile fine.

eli-schwartz commented 3 months ago

Please reread the compiler message.

There are a number of lines marked "warning". I'm not talking about those.

I'm talking about the lines surrounding this:

error: passing argument 1 of ‘av_samples_alloc’ from incompatible pointer type [-Werror=incompatible-pointer-types]

It's not a -Wdeprecated-declarations (none of which claim to be an error).

I'm not in the habit of assaulting upstream projects by promoting deprecations to errors and threatening them to fix a deprecation. Deprecated code has plenty of reasons to exist.

They just happened to be in the same file and I neglected to strip down the compilation log of that file on a per-diagnostic basis.

treellama commented 3 months ago

Ah, ok. You should just need to remove that warning promotion then.

eli-schwartz commented 3 months ago

That's not going to cut it. As I originally said:

So simply removing the warning promotion only buys us a couple weeks. These flags are being used to backport the GCC 14 functionality into GCC 13 by testers, to give advance warning so that we can make sure all packages will compile at all once GCC 14 is released.

eli-schwartz commented 3 months ago

P.S. regarding -Wdeprecated-declarations it may make sense to update CMakeLists.txt to add the -Wno-* for that warning to at least SDL_ffmpeg.c since those warnings are uninteresting and make it harder to analyze the terminal progress when compiling alephone. 🤷‍♂️

If you like I can submit a PR.

AstralPhnx commented 2 months ago

Confirming that these issues are also present in Fedora 40 stable. They're outright shipping GCC 14 in Fedora 40's stable repos now by default so a DNF Install for GCC is installing 14 (see image)

image

cheese1 commented 2 months ago

can confirm: this fix works on fedora 40 (at least the compile succeeds, had not yet found time to check the resulting binaries)

eli-schwartz commented 2 months ago

Thanks for the fix. Consider re-filing this ticket as "closed as completed" instead of "closed as not planned". :)