Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build fails with modern compilers #474

Closed
eli-schwartz opened this issue Mar 26, 2024 · 8 comments
Closed

Build fails with modern compilers #474

eli-schwartz opened this issue Mar 26, 2024 · 8 comments

Comments

@eli-schwartz
Copy link

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 (117c694), 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
Copy link
Member

treellama commented Mar 26, 2024

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.

@treellama treellama closed this as not planned Won't fix, can't repro, duplicate, stale Mar 26, 2024
@eli-schwartz
Copy link
Author

eli-schwartz commented Mar 26, 2024

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
Copy link
Member

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

@eli-schwartz
Copy link
Author

eli-schwartz commented Mar 26, 2024

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

  • this entire class of errors is going to error without any promotion in this month's upcoming GCC release
  • this entire class of errors already does error without any promotion in currently released versions of clang

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
Copy link
Author

eli-schwartz commented Mar 26, 2024

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
Copy link

AstralPhnx commented Apr 29, 2024

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
Copy link

cheese1 commented Apr 30, 2024

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants