Re: [PATCH v5 09/11] 9p: darwin: Implement compatibility for mknodat

[Will Cohen]

On Wed, Feb 9, 2022 at 9:08 AM Christian Schoenebeck <qemu_oss@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