Fedora 35 build fails on custom glibc overrides

[mattiaverga]

Rawtherapee builds are failing in Fedora, it seems the error is:

[ 32%] Building CXX object rtengine/CMakeFiles/rtengine.dir/myfile.cc.o
cd /builddir/build/BUILD/rawtherapee-5.8/redhat-linux-build/rtengine && /usr/bin/g++ -DAUTO_GDK_FLUSH=0 -DMYFILE_MMAP -DNDEBUG -DRT_FFTW3F_OMP -DSTRICT_MUTEX=1 -DTRACE_MYRWMUTEX=0 -D_DNDEBUG -I/builddir/build/BUILD/rawtherapee-5.8/redhat-linux-build/rtengine -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -I/usr/include/glibmm-2.4 -I/usr/lib64/glibmm-2.4/include -I/usr/include/sigc++-2.0 -I/usr/lib64/sigc++-2.0/include -I/usr/include/gtkmm-3.0 -I/usr/lib64/gtkmm-3.0/include -I/usr/include/giomm-2.4 -I/usr/lib64/giomm-2.4/include -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/gtk-3.0 -I/usr/include/pango-1.0 -I/usr/include/harfbuzz -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/fribidi -I/usr/include/libxml2 -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/gio-unix-2.0 -I/usr/include/cloudproviders -I/usr/include/atk-1.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/dbus-1.0 -I/usr/lib64/dbus-1.0/include -I/usr/include/at-spi-2.0 -I/usr/include/cairomm-1.0 -I/usr/lib64/cairomm-1.0/include -I/usr/include/pangomm-1.4 -I/usr/lib64/pangomm-1.4/include -I/usr/include/atkmm-1.6 -I/usr/lib64/atkmm-1.6/include -I/usr/include/gtk-3.0/unix-print -I/usr/include/gdkmm-3.0 -I/usr/lib64/gdkmm-3.0/include -I/usr/include/libiptcdata -I/usr/include/lensfun -I/usr/include/librsvg-2.0 -I/usr/include/klt -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64 -mcpu=power8 -mtune=power8 -fasynchronous-unwind-tables -fstack-clash-protection -std=c++11  -Werror=unused-label -Werror=delete-incomplete -Wall -Wuninitialized -Wcast-qual -Wno-deprecated-declarations -Wno-unused-result -fopenmp -Werror=unknown-pragmas -DNDEBUG -ftree-vectorize -MD -MT rtengine/CMakeFiles/rtengine.dir/myfile.cc.o -MF CMakeFiles/rtengine.dir/myfile.cc.o.d -o CMakeFiles/rtengine.dir/myfile.cc.o -c /builddir/build/BUILD/rawtherapee-5.8/rtengine/myfile.cc
In file included from /usr/include/features.h:488,
                 from /usr/include/c++/11/ppc64le-redhat-linux/bits/os_defines.h:39,
                 from /usr/include/c++/11/ppc64le-redhat-linux/bits/c++config.h:586,
                 from /usr/include/c++/11/cstdio:41,
                 from /builddir/build/BUILD/rawtherapee-5.8/rtengine/myfile.h:21,
                 from /builddir/build/BUILD/rawtherapee-5.8/rtengine/myfile.cc:19:
/usr/include/wchar.h:582:24: error: 'malloc' attribute argument 1 is ambiguous
  582 |   __attribute_malloc__ __attr_dealloc_fclose;
      |                        ^~~~~~~~~~~~~~~~~~~~~
/usr/include/wchar.h:582:24: note: use a cast to the expected type to disambiguate

Full build log at https://kojipkgs.fedoraproject.org//work/tasks/8055/72488055/build.log

[Floessie]

@mattiaverga Mattia, I'm afraid, but this looks more like a toolchain error at first sight.

[Thanatomanic]

@mattiaverga Why should we support an OS that has not even been released? Until such time, I'll close this.

[mattiaverga]

I've asked for help on Fedora mailing list by someone which has more knowledge than me, this is the reply:

The file rtengine/myfile.cc overrides a number of glibc functions.
One of them is fclose; see line 193.  The compiler sees this code in
wchar.h:

#if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2)
# ifndef __attr_dealloc_fclose
#   if defined __has_builtin
#     if __has_builtin (__builtin_fclose)
/* If the attribute macro hasn't been defined yet (by <stdio.h>) and
   fclose is a built-in, use it.  */
#      define __attr_dealloc_fclose __attr_dealloc (__builtin_fclose, 1)
#     endif
#   endif
# endif
# ifndef __attr_dealloc_fclose
#  define __attr_dealloc_fclose /* empty */
# endif

That leads to the open_wmemstream declaration expanding to this:

extern __FILE *open_wmemstream (wchar_t **__bufloc, size_t *__sizeloc)
noexcept (true)
  __attribute__ ((__malloc__)) __attribute__ ((__malloc__ (fclose, 1)));

Apparently that works great with the glibc declaration of fclose, but
not so great with the myfile.{cc,h} version, which you will note is
lacking all of the attributes that the glibc headers give it.

If upstream wants to override glibc functions, they should make the
function declarations match.

If you're not interested in fixing this I'll have to drop packaging RT in Fedora.

[Thanatomanic]

@mattiaverga As you see, I was woefully ignorant of your role in the RawTherapee project. My apologies. Please keep packaging RT for Fedora.

What remains curious, is why this problem arises now. This particular bit of code has been in our source tree for >6 years and, presumably, has never led to compilation errors or raised flags in static code analysis. I think @Floessie is much better suited to further comment on this than I am.

[mattiaverga]

@Thanatomanic no problem at all. I simply haven't got any C skills, so I need your help for fixing things like this to maintain RT in Fedora repositories.

If it can help, Fedora 35 is already using glibc 2.34, that is scheduled to be released just around today. This is one reason that required a mass rebuild of all packages in Fedora repositories and many had failures related to these glibc upgrades.
Maybe the code is no more compatible with recent glibc.

Accordingly to Fedora automated continuous build tool, the build failures started between glibc-2.33.9000-2 and glibc-2.33.9000-23. The changelog reports:

* Fri Jun 18 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-23
- Make glibc-all-langpacks require glibc-gconv-extra in buildroots (#1973663)

* Thu Jun 17 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-22
- Export libthread_db symbols under GLBIC_PRIVATE (#1965374)

* Thu Jun 17 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-21
- Redo the crafted libc.so.6 symbol table for valgrind (#1965374)

* Thu Jun 17 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-20
- Remove .symtab from libc.so.6 again (#1965374)

* Thu Jun 17 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-19
- Drop glibc-rh697421.patch: The ISO-10646-UCS-2// alias for UTF-8 is incorrect
  (#1972520)

* Wed Jun 16 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-18
- Rebuild against rawhide gcc

* Tue Jun 15 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-17
- Dropped glibc-gconv-modules-revert.patch, applied upstream.
- Auto-sync with upstream branch master,
  commit aa9a7f629632c9180de89632d4f2c1e6039f7781:
- nptl: Export _pthread_cleanup_push, _pthread_cleanup_pop again
- s390x: Align child stack while clone. [BZ #27968]
- y2038: Add test coverage
- libsupport: Add 64-bit time_t support for stat functions
- libsupport: Add 64-bit time_t support for time functions
- io: Add ftw64 with 64-bit time_t support
- io: Add fts64 with 64-bit time_t support
- posix: Add glob64 with 64-bit time_t support
- y2038: Add support for 64-bit time on legacy ABIs
- time: Add 64-bit time support for getdate
- y2038: Add __USE_TIME_BITS64 support for socket-constants.h
- y2038: Use a common definition for shmid_ds
- y2038: Use a common definition for semid_ds
- y2038: Use a common definition for msqid_ds
- y2038: Use a common definition for stat
- y2038: linux: Add __USE_TIME_BITS64 support for struct timex
- y2038: Add __USE_TIME_BITS64 support for struct utimbuf
- y2038: Add __USE_TIME_BITS64 support for struct timespec
- y2038: Add __USE_TIME_BITS64 support for struct timeval
- y2038: Add __USE_TIME_BITS64 support for time_t
- linux: Add recvvmsg fallback for 64-bit time_t SO_TIMESTAMP{NS}
- linux: Add fallback for 64-bit time_t SO_TIMESTAMP{NS}
- linux: Add fallback for 64-bit time_t SO_{RCV,SND}TIMEO
- linux: s390: Add libanl.abilist in s390 and s390x
- linux: mips: Split libanl.abilist in n32 and n64
- linux: mips: Split librt.abilist in n32 and n64
- Reinstate gconv-modules as the default configuration file

* Tue Jun 15 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-16
- Preserve some symbols in libc.so.6's symtab (#1965374)

* Tue Jun 15 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-15
- Install shared objects under their ABI names, avoiding symlinks (#1652867)

* Mon Jun 14 2021 Siddhesh Poyarekar <siddhesh@redhat.com> - 2.33.9000-14
- Add a conditional dependency for glibc-gconv-extra.i686 in x86_64.

* Mon Jun 14 2021 Siddhesh Poyarekar <siddhesh@redhat.com> - 2.33.9000-13
- Auto-sync with upstream branch master,
  commit ebae2f5a6f971a8f0b6c99e00f9c45ef7433924a.
- Revert gconv configuration file name to gconv-modules.

* Thu Jun 03 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-12
- libdl is no longer  a separate shared object.
- CVE-2021-33574: Use-after-free via mq_notify (#1965410)
- Auto-sync with upstream branch master,
  commit 466c1ea15f461edb8e3ffaf5d86d708876343bbf:
- dlfcn: Rework static dlopen hooks
- dlfcn: Eliminate GLIBC_PRIVATE dependency from tststatic2
- dlfcn: Cleanups after -ldl is no longer required
- dlfcn: Move dlopen into libc
- dlfcn: Move dlvsym into libc
- dlfcn: Move dlinfo into libc
- dlfcn: Move dladdr1 into libc
- dlfcn: Move dlmopen into libc
- dlfcn: Move dlsym into libc
- dlfcn: Move dladdr into libc
- dlfcn: Move dlclose into libc
- Improve test coverage of strlen function
- fix typo
- dlfcn: Move dlerror into libc
- Add libc ABI extension kludge for baseline-violating libdl symbols
- scripts/versions.awk: Add local: * to all version nodes
- Add missing symbols to Version files
- Fix use of __pthread_attr_copy in mq_notify (bug 27896)
- Use __pthread_attr_copy in mq_notify (bug 27896)
- Update floating-point feature test macro handling for C2X
- stdio-common: Remove _IO_vfwscanf
- aarch64: align stack in clone [BZ #27939]
- powerpc: Optimized memcmp for power10
- x86-64: Align child stack to 16 bytes [BZ #27902]

* Mon May 31 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-11
- glibc-sigsetxid-sa_onstack.patch was applied upstream
- Auto-sync with upstream branch master,
  commit 271ec55d0ae795f03d92e3aa61bff69a31a19e3a:
- support: Do not build xpthread_attr_setaffinity_np for hurd
- nptl: Add pthread_attr_setaffinity_np failure test
- support: Add xpthread_attr_setaffinity_np wrapper
- nptl: Move createthread to pthread_create
- nptl: Move Linux createthread to nptl
- nptl: Install SIGSETXID handler with SA_ONSTACK [BZ #27914]
- aarch64: Added optimized memset for A64FX
- aarch64: Added optimized memcpy and memmove for A64FX
- benchtests: Fixed bench-memcpy-random: buf1: mprotect failed
- aarch64: Added Vector Length Set test helper script
- aarch64: define BTI_C and BTI_J macros as NOP unless HAVE_AARCH64_BTI
- config: Added HAVE_AARCH64_SVE_ASM for aarch64
- tst-mallinfo2.c: Use correct multiple for total variable

* Wed May 26 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-10
- nptl: Install SIGSETXID handler with SA_ONSTACK [BZ #27914]

* Tue May 25 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-9
- Auto-sync with upstream branch master,
  commit ac0353af81a23535f517586a5d04427120a157ac.
- This removes libpthread as a separate shared object.  New programs
  will depend on the GLIBC_2.34 symbol version.  Upstream development
  brings in the following noteworthy bug fixes:
- ppc64le: scv ABI error handling fails to check IS_ERR_VALUE (#1962971)
- CVE-2021-27645 glibc: Use-after-free in addgetnetgrentX function in
  netgroupcache.c (#1932590)
- Linking the main program with jemalloc causes sysconf to deadlock in
  audit mode (#1909920)

* Fri May 21 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-8
- Switch back to a unified glibc-headers package for downstream (#1940686)

* Fri May 21 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-7
- aarch64: Enable optional memory tagging support

* Tue May 11 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-6
- Use distribution mechanism for debuginfo (#1661510, #1886295, #1905611)

* Thu May 06 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-5
- Build locales in parallel again

* Tue May 04 2021 Florian Weimer <fweimer@redhat.com> - 2.33.9000-4
- Various changes to get glibc building again, using selected upstream
  backports.
- Re-enable -Werror by default.
- This release introduces __libc_start_main@@GLIBC_2.34, so binaries
  are not compatible with glibc 2.33.  Building binaries against this
  glibc version is not recommended because of the partial libpthread
  transition.
- glibc-upstream-amx-detection.patch: Fix build failure after GCC 11.1 update.
- glibc-upstream-malloc-test-hang.patch: Avoid test hang due to
  annobin/binutils bug (#1951492)

* Wed Mar 03 2021 Arjun Shankar <arjun@redhat.com> - 2.33.9000-3
- Drop glibc-rh819430.patch; fixed upstream.
- Auto-sync with upstream branch master,
  commit f01a61e13872109b3b233158ab664364bd1879bc:
- i386: Regenerate ulps
- x86: Add CPU-specific diagnostics to ld.so --list-diagnostics
- x86: Automate generation of PREFERRED_FEATURE_INDEX_1 bitfield
- ld.so: Implement the --list-diagnostics option
- powerpc: Update libm-test-ulps
- tst: Add test for utimes
- tst: Add test for utime
- tst: Add test for futimens
- nptl: __libc_cleanup_push/__libc_cleanup_pop require -fexceptions
- elf: Build __dl_iterate_phdr with unwinding support [BZ #27498]
- nptl: Use <unwind-link.h> for accessing the libgcc_s unwinder
- Implement _Unwind_Resume in libc on top of <unwind-link.h>
- Move sysdeps/gnu/unwind-resume.c to sysdeps/generic/unwind-resume.c
- __frame_state_for: Use <unwind-link.h> for unwinder access
- sparc: Implement backtrace on top <unwind-link.h>
- m68k: Implement backtrace on top of <unwind-link.h>
- i386: Implement backtrace on top of <unwind-link.h>
- arm: Implement backtrace on top of <unwind-link.h>
- backtrace: Implement on top of <unwind-link.h>
- Implement <unwind-link.h> for dynamically loading the libgcc_s unwinder
- Correct buffer end pointer in IO_wdefault_doallocate (BZ #26874)
- aarch64: update ulps.
- Add inputs that generate larger error bounds
- Reduce the statically linked startup code [BZ #23323]
- posix: Falling back to non wide mode in case of encoding error [BZ #14185]
- nptl: Move elision implementations into libc
- NEWS: Add missing bug closures
- added rt to malloc/Depend [BZ #27132]
- x86: Use x86/nptl/pthreaddef.h
- nptl: Move futex-internal into libc
- nptl: Move lowlevellock into libc [BZ #15648]
- nptl: Move futex-internal.c into main nptl directory
- nptl: Reformat Versions
- nptl: Split libpthread-routines into one routine per line
- x86: Remove unused variables for raw cache sizes from cacheinfo.h
- Use Linux 5.11 in build-many-glibcs.py.
- <bits/platform/x86.h>: Correct x86_cpu_TBM
- x86_64/clone.S: Upate comments
- i386/clone.S: Remove redundant EBX load
- aarch64: Remove the unused __read_tp symbol
- build-many-glibcs.py: Use make -O for more consistent log output

You may start seeing this build error in other distributions also when glibc 2.34 lands there too (if that's the right cause).

[Floessie]

Hmm, after sleeping on it, I still think that we're not overriding but overloading those C library functions. After all, func(X*) is something different than func(Y*) even in the global namespace. Furthermore, I have no clue what the "correct" signature should be when supporting multiple libcs or even glibc versions. The only signature we can rely on is the POSIX one int fclose(FILE *stream). But again, this is not overridden but overloaded.

I do understand that those attributes support fortification of the code. What I don't understand is why the simple presence of any fclose symbol now breaks a glibc build...

@mattiaverga Mattia, would you be so kind to invite the commentator from the Fedora mailing list to this conversation, if that's possible? I'm positive we'll find a solution.

Unfortunately I'm not able to test any coming fixes, as even Debian Experimental is still stuck on glibc 2.31.

Best,
Flössie

[fweimer-rh]

The issue is that the new (in GCC 11) two-argument version of the malloc attribute does not support function overloading. It simply bails out:

struct T1;
struct T2;

void close(T1*);
void close(T2*);

T1* open(const char *) __attribute__ ((__malloc__ (close, 1)));

Results in:

t.cc:7:62: error: ‘malloc’ attribute argument 1 is ambiguous
    7 | T1* open(const char *) __attribute__ ((__malloc__ (close, 1)));
      |                                                              ^
t.cc:7:62: note: use a cast to the expected type to disambiguate

I filed a GCC PR: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101747 I don't know if this enhancement will be implemented.

I'm not sure if it is worthwhile to add the cast to the glibc sources. Every use of this version of the malloc attribute would need it in principle.

The best course of action seems to be to avoid overloading ISO C functions in RawTherapee.

[Floessie]

@fweimer-rh Florian, thanks for the insights! 👍

The best course of action seems to be to avoid overloading ISO C functions in RawTherapee.

Yep, don't know, why this was ever done the way it is. Maybe for quick dcraw adaption. I guess the best solution is moving the libc functions into our rtengine namespace...

[Floessie]

In the meantime, I've installed Fedora Rawhide in a VM and am able to reproduce the problem.

This preliminary patch fixes it:

fix-6324.patch.txt

HTH,
Flössie

[mattiaverga]

Thanks @Floessie , I'll try to make a build with the provided patch ASAP.

[Thanatomanic]

I can confirm that the patch solves the building issues, @Floessie feel free to commit. RT also still compiles successfully on some of the other OSes I tested.

[mattiaverga]

@Floessie sorry to have kept you waiting, I've been extremely busy last days.
I had to adapt the patch to make it work with RT 5.8 (there were a couple of fopen calls that have been removed in devel branch), but then the build completely successfully and from a quick test everything seems to work in RT.
I'll wait until you officialize the commit, then I'll backport it into Fedora's RT 5.8.

Thank you for the support!

[Floessie]

@Thanatomanic Roel, thanks for cross-checking!

@mattiaverga Mattia, didn't know you had to backport the stuff to 5.8. If I had known, I would have committed to dev earlier. Now that it's done I hope you can reuse your 5.8 patch. 2e0137d is the patch applied verbatim.

@mattiaverga @mattdm All the best and good luck with Fedora 35! I'm personally really sorry that 5.9 didn't yet happen so it won't make it into Debian 11. Let's hope we're ready for Fedora 36. 👍 And I'm sure we're ready for Debian 12 😉.

Best,
Flössie

[Floessie]

Me:

I'm personally really sorry that 5.9 didn't yet happen so it won't make it into Debian 11. Let's hope we're ready for Fedora 36. 👍

LWN:

If all goes well, the final Fedora 36 release will happen at the end of April.

Me:

😑

But hey, Debian 12 is slated for mid-2023. 👍