Message ID | 20220119134846.19557-1-anton@khirnov.net |
---|---|
State | Accepted |
Commit | 2f0a214a6202516b4dda2bb22b6b3ac20e465d6d |
Headers | show |
Series | [FFmpeg-devel] configure: link to libatomic when it's present | expand |
Context | Check | Description |
---|---|---|
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
andriy/make_ppc | success | Make finished |
andriy/make_fate_ppc | success | Make fate finished |
andriy/make_armv7_RPi4 | success | Make finished |
andriy/make_fate_armv7_RPi4 | success | Make fate finished |
andriy/make_aarch64_jetson | success | Make finished |
andriy/make_fate_aarch64_jetson | success | Make fate finished |
On 1/19/2022 10:48 AM, Anton Khirnov wrote: > C11 atomics in some configurations (e.g. 64bit operations on ppc64 with > GCC) require linking to libatomic. > > Fixes #9275 > --- > configure | 25 ++++++++++++++++--------- > 1 file changed, 16 insertions(+), 9 deletions(-) > > diff --git a/configure b/configure > index 1413122d87..3059c154df 100755 > --- a/configure > +++ b/configure > @@ -3794,20 +3794,20 @@ cws2fws_extralibs="zlib_extralibs" > > # libraries, in any order > avcodec_deps="avutil" > -avcodec_suggest="libm" > +avcodec_suggest="libm stdatomic" > avdevice_deps="avformat avcodec avutil" > -avdevice_suggest="libm" > +avdevice_suggest="libm stdatomic" > avfilter_deps="avutil" > -avfilter_suggest="libm" > +avfilter_suggest="libm stdatomic" > avformat_deps="avcodec avutil" > -avformat_suggest="libm network zlib" > -avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt" > +avformat_suggest="libm network zlib stdatomic" > +avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt stdatomic" > postproc_deps="avutil gpl" > -postproc_suggest="libm" > +postproc_suggest="libm stdatomic" > swresample_deps="avutil" > -swresample_suggest="libm libsoxr" > +swresample_suggest="libm libsoxr stdatomic" > swscale_deps="avutil" > -swscale_suggest="libm" > +swscale_suggest="libm stdatomic" > > avcodec_extralibs="pthreads_extralibs iconv_extralibs dxva2_extralibs" > avfilter_extralibs="pthreads_extralibs" > @@ -6324,7 +6324,14 @@ check_headers asm/types.h > # it seems there are versions of clang in some distros that try to use the > # gcc headers, which explodes for stdatomic > # so we also check that atomics actually work here > -check_builtin stdatomic stdatomic.h "atomic_int foo, bar = ATOMIC_VAR_INIT(-1); atomic_store(&foo, 0); foo += bar" > +# > +# some configurations also require linking to libatomic, so try > +# both with -latomic and without > +for LATOMIC in "-latomic" ""; do > + check_builtin stdatomic stdatomic.h \ > + "atomic_int foo, bar = ATOMIC_VAR_INIT(-1); atomic_store(&foo, 0); foo += bar" \ > + $LATOMIC && eval stdatomic_extralibs="\$LATOMIC" && break > +done LGTM now, thanks. > > check_lib advapi32 "windows.h" RegCloseKey -ladvapi32 > check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom -lbcrypt &&
On 1/19/2022 10:23 AM, James Almer wrote: > On 1/19/2022 10:48 AM, Anton Khirnov wrote: >> C11 atomics in some configurations (e.g. 64bit operations on ppc64 with >> GCC) require linking to libatomic. >> >> Fixes #9275 >> --- >> configure | 25 ++++++++++++++++--------- >> 1 file changed, 16 insertions(+), 9 deletions(-) >> >> diff --git a/configure b/configure >> index 1413122d87..3059c154df 100755 >> --- a/configure >> +++ b/configure >> @@ -3794,20 +3794,20 @@ cws2fws_extralibs="zlib_extralibs" >> # libraries, in any order >> avcodec_deps="avutil" >> -avcodec_suggest="libm" >> +avcodec_suggest="libm stdatomic" >> avdevice_deps="avformat avcodec avutil" >> -avdevice_suggest="libm" >> +avdevice_suggest="libm stdatomic" >> avfilter_deps="avutil" >> -avfilter_suggest="libm" >> +avfilter_suggest="libm stdatomic" >> avformat_deps="avcodec avutil" >> -avformat_suggest="libm network zlib" >> -avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl >> user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia >> bcrypt" >> +avformat_suggest="libm network zlib stdatomic" >> +avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl >> user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia >> bcrypt stdatomic" >> postproc_deps="avutil gpl" >> -postproc_suggest="libm" >> +postproc_suggest="libm stdatomic" >> swresample_deps="avutil" >> -swresample_suggest="libm libsoxr" >> +swresample_suggest="libm libsoxr stdatomic" >> swscale_deps="avutil" >> -swscale_suggest="libm" >> +swscale_suggest="libm stdatomic" >> avcodec_extralibs="pthreads_extralibs iconv_extralibs >> dxva2_extralibs" >> avfilter_extralibs="pthreads_extralibs" >> @@ -6324,7 +6324,14 @@ check_headers asm/types.h >> # it seems there are versions of clang in some distros that try to >> use the >> # gcc headers, which explodes for stdatomic >> # so we also check that atomics actually work here >> -check_builtin stdatomic stdatomic.h "atomic_int foo, bar = >> ATOMIC_VAR_INIT(-1); atomic_store(&foo, 0); foo += bar" >> +# >> +# some configurations also require linking to libatomic, so try >> +# both with -latomic and without >> +for LATOMIC in "-latomic" ""; do >> + check_builtin stdatomic >> stdatomic.h \ >> + "atomic_int foo, bar = ATOMIC_VAR_INIT(-1); >> atomic_store(&foo, 0); foo += bar" \ >> + $LATOMIC && eval stdatomic_extralibs="\$LATOMIC" && break >> +done > > LGTM now, thanks. > >> check_lib advapi32 "windows.h" RegCloseKey >> -ladvapi32 >> check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom -lbcrypt && Wait, this should be checking without first then with, if the first test without fails.
On Sat, Jan 22, 2022 at 7:43 AM Brad Smith <brad-at-comstyle.com@ffmpeg.org> wrote: > > On 1/19/2022 10:23 AM, James Almer wrote: > > > On 1/19/2022 10:48 AM, Anton Khirnov wrote: > >> C11 atomics in some configurations (e.g. 64bit operations on ppc64 with > >> GCC) require linking to libatomic. > >> > >> Fixes #9275 > >> --- > >> configure | 25 ++++++++++++++++--------- > >> 1 file changed, 16 insertions(+), 9 deletions(-) > >> > >> diff --git a/configure b/configure > >> index 1413122d87..3059c154df 100755 > >> --- a/configure > >> +++ b/configure > >> @@ -3794,20 +3794,20 @@ cws2fws_extralibs="zlib_extralibs" > >> # libraries, in any order > >> avcodec_deps="avutil" > >> -avcodec_suggest="libm" > >> +avcodec_suggest="libm stdatomic" > >> avdevice_deps="avformat avcodec avutil" > >> -avdevice_suggest="libm" > >> +avdevice_suggest="libm stdatomic" > >> avfilter_deps="avutil" > >> -avfilter_suggest="libm" > >> +avfilter_suggest="libm stdatomic" > >> avformat_deps="avcodec avutil" > >> -avformat_suggest="libm network zlib" > >> -avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl > >> user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia > >> bcrypt" > >> +avformat_suggest="libm network zlib stdatomic" > >> +avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl > >> user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia > >> bcrypt stdatomic" > >> postproc_deps="avutil gpl" > >> -postproc_suggest="libm" > >> +postproc_suggest="libm stdatomic" > >> swresample_deps="avutil" > >> -swresample_suggest="libm libsoxr" > >> +swresample_suggest="libm libsoxr stdatomic" > >> swscale_deps="avutil" > >> -swscale_suggest="libm" > >> +swscale_suggest="libm stdatomic" > >> avcodec_extralibs="pthreads_extralibs iconv_extralibs > >> dxva2_extralibs" > >> avfilter_extralibs="pthreads_extralibs" > >> @@ -6324,7 +6324,14 @@ check_headers asm/types.h > >> # it seems there are versions of clang in some distros that try to > >> use the > >> # gcc headers, which explodes for stdatomic > >> # so we also check that atomics actually work here > >> -check_builtin stdatomic stdatomic.h "atomic_int foo, bar = > >> ATOMIC_VAR_INIT(-1); atomic_store(&foo, 0); foo += bar" > >> +# > >> +# some configurations also require linking to libatomic, so try > >> +# both with -latomic and without > >> +for LATOMIC in "-latomic" ""; do > >> + check_builtin stdatomic > >> stdatomic.h \ > >> + "atomic_int foo, bar = ATOMIC_VAR_INIT(-1); > >> atomic_store(&foo, 0); foo += bar" \ > >> + $LATOMIC && eval stdatomic_extralibs="\$LATOMIC" && break > >> +done > > > > LGTM now, thanks. > > > >> check_lib advapi32 "windows.h" RegCloseKey > >> -ladvapi32 > >> check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom -lbcrypt && > > Wait, this should be checking without first then with, if the first test > without fails. > This was covered earlier in the thread for the reason it is not - its deliberate, because exhaustive functionality checks would be very complicated. - Hendrik
On 1/22/2022 4:00 AM, Hendrik Leppkes wrote: > On Sat, Jan 22, 2022 at 7:43 AM Brad Smith > <brad-at-comstyle.com@ffmpeg.org> wrote: >> On 1/19/2022 10:23 AM, James Almer wrote: >> >>> On 1/19/2022 10:48 AM, Anton Khirnov wrote: >>>> C11 atomics in some configurations (e.g. 64bit operations on ppc64 with >>>> GCC) require linking to libatomic. >>>> >>>> Fixes #9275 >>>> --- >>>> configure | 25 ++++++++++++++++--------- >>>> 1 file changed, 16 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/configure b/configure >>>> index 1413122d87..3059c154df 100755 >>>> --- a/configure >>>> +++ b/configure >>>> @@ -3794,20 +3794,20 @@ cws2fws_extralibs="zlib_extralibs" >>>> # libraries, in any order >>>> avcodec_deps="avutil" >>>> -avcodec_suggest="libm" >>>> +avcodec_suggest="libm stdatomic" >>>> avdevice_deps="avformat avcodec avutil" >>>> -avdevice_suggest="libm" >>>> +avdevice_suggest="libm stdatomic" >>>> avfilter_deps="avutil" >>>> -avfilter_suggest="libm" >>>> +avfilter_suggest="libm stdatomic" >>>> avformat_deps="avcodec avutil" >>>> -avformat_suggest="libm network zlib" >>>> -avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl >>>> user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia >>>> bcrypt" >>>> +avformat_suggest="libm network zlib stdatomic" >>>> +avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl >>>> user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia >>>> bcrypt stdatomic" >>>> postproc_deps="avutil gpl" >>>> -postproc_suggest="libm" >>>> +postproc_suggest="libm stdatomic" >>>> swresample_deps="avutil" >>>> -swresample_suggest="libm libsoxr" >>>> +swresample_suggest="libm libsoxr stdatomic" >>>> swscale_deps="avutil" >>>> -swscale_suggest="libm" >>>> +swscale_suggest="libm stdatomic" >>>> avcodec_extralibs="pthreads_extralibs iconv_extralibs >>>> dxva2_extralibs" >>>> avfilter_extralibs="pthreads_extralibs" >>>> @@ -6324,7 +6324,14 @@ check_headers asm/types.h >>>> # it seems there are versions of clang in some distros that try to >>>> use the >>>> # gcc headers, which explodes for stdatomic >>>> # so we also check that atomics actually work here >>>> -check_builtin stdatomic stdatomic.h "atomic_int foo, bar = >>>> ATOMIC_VAR_INIT(-1); atomic_store(&foo, 0); foo += bar" >>>> +# >>>> +# some configurations also require linking to libatomic, so try >>>> +# both with -latomic and without >>>> +for LATOMIC in "-latomic" ""; do >>>> + check_builtin stdatomic >>>> stdatomic.h \ >>>> + "atomic_int foo, bar = ATOMIC_VAR_INIT(-1); >>>> atomic_store(&foo, 0); foo += bar" \ >>>> + $LATOMIC && eval stdatomic_extralibs="\$LATOMIC" && break >>>> +done >>> LGTM now, thanks. >>> >>>> check_lib advapi32 "windows.h" RegCloseKey >>>> -ladvapi32 >>>> check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom -lbcrypt && >> Wait, this should be checking without first then with, if the first test >> without fails. >> > This was covered earlier in the thread for the reason it is not - its > deliberate, because exhaustive functionality checks would be very > complicated. Testing this commit out it does as I had suspected and even with --as-needed causes a false positive on OpenBSD / FreeBSD. Now erroneously tries to link against libatomic and unlike the other project (haproxy) I ran across an overly simplistic test (doesn't even involve linking, just checking compiler predefined macros) I don't see any options to disable the broken test either.
Quoting Brad Smith (2022-01-23 20:40:30) > > Testing this commit out it does as I had suspected and even with --as-needed > causes a false positive on OpenBSD / FreeBSD. Why?
On 1/26/2022 6:49 AM, Anton Khirnov wrote: > Quoting Brad Smith (2022-01-23 20:40:30) >> Testing this commit out it does as I had suspected and even with --as-needed >> causes a false positive on OpenBSD / FreeBSD. > Why? Looking at this again and thinking about what it does, the test as is is flawed. With the order being used it doesn't check to see if the linked binary even is linked against libatomic. Using --as-needed it drops the libatomic dependency but since the link succeeds then the code as it is and based on the ordering, as I commented about, says go ahead and link in libatomic. Changing the order so it is "" "-latomic" instead of "-latomic" "" does the right thing. humpty$ cc -D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -DPIC -O2 -pipe -I/usr/local/include -I/usr/X11R6/include -std=c11 -fPIC -c -o test.o test.c humpty$ cc -Wl,--as-needed -Wl,-z,noexecstack -o test test.o -latomic -L/usr/local/lib -L/usr/X11R6/lib humpty$ objdump -p test test: file format elf64-x86-64 Program Header: PHDR off 0x0000000000000040 vaddr 0x0000000000000040 paddr 0x0000000000000040 align 2**3 filesz 0x00000000000002a0 memsz 0x00000000000002a0 flags r-- INTERP off 0x00000000000002e0 vaddr 0x00000000000002e0 paddr 0x00000000000002e0 align 2**0 filesz 0x0000000000000013 memsz 0x0000000000000013 flags r-- LOAD off 0x0000000000000000 vaddr 0x0000000000000000 paddr 0x0000000000000000 align 2**12 filesz 0x00000000000005dc memsz 0x00000000000005dc flags r-- LOAD off 0x00000000000005e0 vaddr 0x00000000000015e0 paddr 0x00000000000015e0 align 2**12 filesz 0x0000000000000450 memsz 0x0000000000000450 flags r-x LOAD off 0x0000000000000a30 vaddr 0x0000000000002a30 paddr 0x0000000000002a30 align 2**12 filesz 0x00000000000001c0 memsz 0x00000000000001c0 flags rw- LOAD off 0x0000000000000bf0 vaddr 0x0000000000003bf0 paddr 0x0000000000003bf0 align 2**12 filesz 0x0000000000000000 memsz 0x0000000000000055 flags rw- DYNAMIC off 0x0000000000000a88 vaddr 0x0000000000002a88 paddr 0x0000000000002a88 align 2**3 filesz 0x0000000000000120 memsz 0x0000000000000120 flags rw- RELRO off 0x0000000000000a30 vaddr 0x0000000000002a30 paddr 0x0000000000002a30 align 2**0 filesz 0x00000000000001c0 memsz 0x00000000000005d0 flags r-- EH_FRAME off 0x00000000000004c8 vaddr 0x00000000000004c8 paddr 0x00000000000004c8 align 2**2 filesz 0x0000000000000034 memsz 0x0000000000000034 flags r-- OPENBSD_RANDOMIZE off 0x0000000000000a30 vaddr 0x0000000000002a30 paddr 0x0000000000002a30 align 2**3 filesz 0x0000000000000030 memsz 0x0000000000000030 flags rw- STACK off 0x0000000000000000 vaddr 0x0000000000000000 paddr 0x0000000000000000 align 2**0 filesz 0x0000000000000000 memsz 0x0000000000000000 flags rw- NOTE off 0x00000000000002f4 vaddr 0x00000000000002f4 paddr 0x00000000000002f4 align 2**2 filesz 0x0000000000000018 memsz 0x0000000000000018 flags r-- Dynamic Section: NEEDED libc.so.96.1 FLAGS_1 0x8000000 DEBUG 0x0 RELA 0x438 RELASZ 0x30 RELAENT 0x18 RELACOUNT 0x1 JMPREL 0x468 PLTRELSZ 0x60 PLTGOT 0x2bb8 PLTREL 0x7 SYMTAB 0x310 SYMENT 0x18 STRTAB 0x3f8 STRSZ 0x3f GNU_HASH 0x3a0 HASH 0x3c0 libatomic is dropped by --as-needed since it is not necessary.
On Sat, Jan 29, 2022 at 5:45 AM Brad Smith <brad-at-comstyle.com@ffmpeg.org> wrote: > > libatomic is dropped by --as-needed since it is not necessary. > Thats what this solution relies on, and thus there is no harm in adding it. - Hendrik
On 1/29/2022 4:54 AM, Hendrik Leppkes wrote: > On Sat, Jan 29, 2022 at 5:45 AM Brad Smith > <brad-at-comstyle.com@ffmpeg.org> wrote: >> libatomic is dropped by --as-needed since it is not necessary. >> > Thats what this solution relies on, and thus there is no harm in adding it. But it doesn't work, and the generated pkg-config files are contaminated too. I'll just have to patch out this broken crap locally.
Quoting Brad Smith (2022-01-29 19:15:46) > On 1/29/2022 4:54 AM, Hendrik Leppkes wrote: > > > On Sat, Jan 29, 2022 at 5:45 AM Brad Smith > > <brad-at-comstyle.com@ffmpeg.org> wrote: > >> libatomic is dropped by --as-needed since it is not necessary. > >> > > Thats what this solution relies on, and thus there is no harm in adding it. > > But it doesn't work, and the generated pkg-config files are contaminated > too. I'll just > have to patch out this broken crap locally. You have now twice disregarded someone explaining why the test first check with -latomic and then without, then you call this code "broken crap" without properly explaining what breaks and why. Consider communicating better if you want people here to take you seriously.
diff --git a/configure b/configure index 1413122d87..3059c154df 100755 --- a/configure +++ b/configure @@ -3794,20 +3794,20 @@ cws2fws_extralibs="zlib_extralibs" # libraries, in any order avcodec_deps="avutil" -avcodec_suggest="libm" +avcodec_suggest="libm stdatomic" avdevice_deps="avformat avcodec avutil" -avdevice_suggest="libm" +avdevice_suggest="libm stdatomic" avfilter_deps="avutil" -avfilter_suggest="libm" +avfilter_suggest="libm stdatomic" avformat_deps="avcodec avutil" -avformat_suggest="libm network zlib" -avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt" +avformat_suggest="libm network zlib stdatomic" +avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt stdatomic" postproc_deps="avutil gpl" -postproc_suggest="libm" +postproc_suggest="libm stdatomic" swresample_deps="avutil" -swresample_suggest="libm libsoxr" +swresample_suggest="libm libsoxr stdatomic" swscale_deps="avutil" -swscale_suggest="libm" +swscale_suggest="libm stdatomic" avcodec_extralibs="pthreads_extralibs iconv_extralibs dxva2_extralibs" avfilter_extralibs="pthreads_extralibs" @@ -6324,7 +6324,14 @@ check_headers asm/types.h # it seems there are versions of clang in some distros that try to use the # gcc headers, which explodes for stdatomic # so we also check that atomics actually work here -check_builtin stdatomic stdatomic.h "atomic_int foo, bar = ATOMIC_VAR_INIT(-1); atomic_store(&foo, 0); foo += bar" +# +# some configurations also require linking to libatomic, so try +# both with -latomic and without +for LATOMIC in "-latomic" ""; do + check_builtin stdatomic stdatomic.h \ + "atomic_int foo, bar = ATOMIC_VAR_INIT(-1); atomic_store(&foo, 0); foo += bar" \ + $LATOMIC && eval stdatomic_extralibs="\$LATOMIC" && break +done check_lib advapi32 "windows.h" RegCloseKey -ladvapi32 check_lib bcrypt "windows.h bcrypt.h" BCryptGenRandom -lbcrypt &&