On Wed, Feb 9, 2022 at 6:10 PM Akihiko Odaki <akihiko.od...@gmail.com> wrote:
> On Thu, Feb 10, 2022 at 3:20 AM Will Cohen <wwco...@gmail.com> wrote: > > > > On Wed, Feb 9, 2022 at 9:08 AM Christian Schoenebeck < > qemu_...@crudebyte.com> wrote: > >> > >> On Mittwoch, 9. Februar 2022 14:33:25 CET Akihiko Odaki wrote: > >> > > I like the idea of switching it to __attribute__((weak)). I should > note > >> > > that I'm not sure that I can actually fully test this out since I'm > >> > > getting stuck with the linker noting my undefined fake function > during > >> > > the build, but the idea does make logical sense to me for the > future fail > >> > > case and the happy case builds again when implemented with actual > >> > > pthread_fchdir_np: > >> > > > >> > > [1075/2909] Linking target qemu-nbd > >> > > FAILED: qemu-nbd > >> > > cc -m64 -mcx16 -o qemu-nbd qemu-nbd.p/qemu-nbd.c.o > -Wl,-dead_strip_dylibs > >> > > -Wl,-headerpad_max_install_names -Wl,-undefined,error > -Wl,-force_load > >> > > libblockdev.fa -Wl,-force_load libblock.fa -Wl,-force_load > libcrypto.fa > >> > > -Wl,-force_load libauthz.fa -Wl,-force_load libqom.fa > -Wl,-force_load > >> > > libio.fa -fstack-protector-strong > >> > > -Wl,-rpath,/usr/local/Cellar/gnutls/3.7.3/lib > >> > > -Wl,-rpath,/usr/local/Cellar/pixman/0.40.0/lib libqemuutil.a > >> > > libblockdev.fa libblock.fa libcrypto.fa libauthz.fa libqom.fa > libio.fa > >> > > @block.syms /usr/local/Cellar/gnutls/3.7.3/lib/libgnutls.dylib > -lutil > >> > > -L/usr/local/Cellar/glib/2.70.3/lib -L/usr/local/opt/gettext/lib > >> > > -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lintl > >> > > -L/usr/local/Cellar/glib/2.70.3/lib -L/usr/local/opt/gettext/lib > >> > > -lgio-2.0 -lgobject-2.0 -lglib-2.0 -lintl -lm > >> > > -L/usr/local/Cellar/glib/2.70.3/lib -L/usr/local/opt/gettext/lib > >> > > -lgmodule-2.0 -lglib-2.0 -lintl > >> > > /usr/local/Cellar/pixman/0.40.0/lib/libpixman-1.dylib -lz -lxml2 > >> > > -framework CoreFoundation -framework IOKit -lcurl > >> > > -L/usr/local/Cellar/glib/2.70.3/lib -L/usr/local/opt/gettext/lib > >> > > -lgmodule-2.0 -lglib-2.0 -lintl -lbz2 > >> > > /usr/local/Cellar/libssh/0.9.6/lib/libssh.dylib -lpam> > >> > > Undefined symbols for architecture x86_64: > >> > > "_pthread_fchdir_npfoo", referenced from: > >> > > _qemu_mknodat in libblockdev.fa(os-posix.c.o) > >> > > > >> > > ld: symbol(s) not found for architecture x86_64 > >> > > clang: error: linker command failed with exit code 1 (use -v to see > >> > > invocation) ninja: build stopped: subcommand failed. > >> > > make[1]: *** [run-ninja] Error 1 > >> > > make: *** [all] Error 2 > >> > > > >> > > With that caveat re testing in mind, unless there's another > recommended > >> > > path forward, I think it makes sense to stick with > __attribute__((weak)) > >> > > and prepare v6 which incorporates this and all the other feedback > from > >> > > this round. > >> > __attribute__((weak_import)), which explicitly marks the function as > >> > external, is more appropriate here. It is feature-equivalent with the > >> > availability attribute when the minimum deployment target is lower > >> > than the version which introduced the function. > >> > >> Thanks for chiming in on this macOS issue Akihiko! > >> > >> Are you sure that "weak_import" is still the preferred way? From > behaviour PoV > >> I do not see any difference at all. I can't even tell what the intended > >> difference was, and QEMU currently only seems to use "weak" in the > entire code > >> base. > >> > >> Googling around, "weak_import" seems to be required on ancient OS X > versions > >> only and that it's now deprecated in favour of the more common "weak", > no? > >> > >> Best regards, > >> Christian Schoenebeck > > > > > > Either way seems reasonable to me. For reference, what I'm seeing on > Google and what Christian may be referring to is a circa-2016 conversation > on GCC bugzilla, where a tentative conclusion seems to be that the > distinction between the two is small and weak is probably preferred now: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69179 > > > > GCC doesn't maintain features specific to Apple well so we should look > at clang. Compiling QEMU for macOS with GCC would result in errors > anyway because QEMU uses clang extensions like availability checks and > blocks for Apple's ABIs/APIs. clang still distinguishes > __attribute__((weak)) and __attribute__((weak_import)). > > The present uses of __attribute__((weak)) in QEMU are correct and > shouldn't be replaced with __attribute__((weak_import)) even when > targeting macOS since they have default implementations and are > statically resolved. > > Regards, > Akihiko Odaki > Noted. v6 adjusts to use weak_import. Many thanks!
