Re: [Qemu-devel] [PATCH v6 00/25] Add qemu_getrandom and ARMv8.5-RNG etc
On 5/14/19 2:43 PM, Eric Blake wrote: >> It didn't occur to me that there was nothing in the object files for the >> reference. I'll have to drop the crypto-obj-y patch and come up with a >> different solution. > > Isn't there a gcc annotation for marking a simple as mandatorily > included during link? No. There's stuff you can mark a single function within an object file that you can use to avoid the function being elided... > __attribute__((externally_visible)) sounds promising (it nullifies the > effects of -fwhole-program, so that a function remains visible even if > the linker would have otherwise suppressed it) > > __attribute__((used)) also sounds useful (the function must be emitted > even if it does not appear to be referenced, which may be enough for the > linker to infer that it is used) ... and you found those. But those do not affect the linker's behaviour with .a files at all. You can force a symbol reference from the ld command-line: -u sym, which can cause the .o containing sym to be included from the .a file. But that doesn't work if there's no global symbol in the .o to reference. You can force all .o from a .a file to be included, with --whole-archive. That is useful when you're using .a files a shorthand for lots and lots of .o files. But in our case that would break the use of stubs. Anyway, see v7 now. r~
Re: [Qemu-devel] [PATCH v6 00/25] Add qemu_getrandom and ARMv8.5-RNG etc
On 5/14/19 12:46 PM, Richard Henderson wrote: > On 5/14/19 9:50 AM, Daniel P. Berrangé wrote: >> On Tue, May 14, 2019 at 09:14:57AM -0700, Richard Henderson wrote: >>> Yes, that would do it. We would need something in the test that forces the >>> objects into the link. Without having yet looked at the test cases, any >>> ideas? >> >> I don't think this is only the test suite. I think it will affect all the >> binaries we build > > You're right, it does. > > $ nm aarch64-softmmu/qemu-system-aarch64 \ > | grep qcrypto_tls_creds_x509_register_types > > comes up empty. > > It didn't occur to me that there was nothing in the object files for the > reference. I'll have to drop the crypto-obj-y patch and come up with a > different solution. Isn't there a gcc annotation for marking a simple as mandatorily included during link? /me goes looking... __attribute__((externally_visible)) sounds promising (it nullifies the effects of -fwhole-program, so that a function remains visible even if the linker would have otherwise suppressed it) __attribute__((used)) also sounds useful (the function must be emitted even if it does not appear to be referenced, which may be enough for the linker to infer that it is used) There may be other tricks, although I didn't go searching very hard. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v6 00/25] Add qemu_getrandom and ARMv8.5-RNG etc
On 5/14/19 9:50 AM, Daniel P. Berrangé wrote: > On Tue, May 14, 2019 at 09:14:57AM -0700, Richard Henderson wrote: >> Yes, that would do it. We would need something in the test that forces the >> objects into the link. Without having yet looked at the test cases, any >> ideas? > > I don't think this is only the test suite. I think it will affect all the > binaries we build You're right, it does. $ nm aarch64-softmmu/qemu-system-aarch64 \ | grep qcrypto_tls_creds_x509_register_types comes up empty. It didn't occur to me that there was nothing in the object files for the reference. I'll have to drop the crypto-obj-y patch and come up with a different solution. r~
Re: [Qemu-devel] [PATCH v6 00/25] Add qemu_getrandom and ARMv8.5-RNG etc
On Tue, May 14, 2019 at 09:14:57AM -0700, Richard Henderson wrote: > On 5/14/19 8:23 AM, Daniel P. Berrangé wrote: > > On Tue, May 14, 2019 at 05:15:31PM +0200, Markus Armbruster wrote: > >> "make check-unit" fails for me: > >> > >> TESTcheck-unit: tests/test-crypto-tlscredsx509 > >> Unexpected error in object_new_with_propv() at > >> /work/armbru/qemu/qom/object.c:674: > >> invalid object type: tls-creds-x509 > >> > >> and > >> > >> TESTcheck-unit: tests/test-io-channel-tls > >> Unexpected error in object_new_with_propv() at > >> /work/armbru/qemu/qom/object.c:674: > >> invalid object type: tls-creds-x509 > >> > >> I haven't looked further. > > > > I have a nasty feeling it is caused by > > > > Subject: [PATCH v6 02/25] crypto: Merge crypto-obj-y into libqemuutil.a > > > > The QOM objects are not directly used by most of the code. We rely on > > the constructor registering the QOM object and then we request an > > instance of it via the type name. So there's no direct function calls > > from any code into the crypto object impls. > > > > When we put the crypto objects into libqemuutil.a the linker is not > > intelligent enough to see the constructor and so thinks all these > > QOM object impls are unused and discards them when linking the final > > binary. > > Yes, that would do it. We would need something in the test that forces the > objects into the link. Without having yet looked at the test cases, any > ideas? I don't think this is only the test suite. I think it will affect all the binaries we build The only way you can force it is to use -Wl,--whole-archive arg to tell ld to include everything from libqemuutil.la, but that will break the way the stubs work, as we want make of the stubs to be discarded. The only other option is to not build $(crypto-obj-y) into libqemuutil.la, but list that variable explicitly everywhere that we list libqemuutil.la Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH v6 00/25] Add qemu_getrandom and ARMv8.5-RNG etc
On 5/14/19 8:23 AM, Daniel P. Berrangé wrote: > On Tue, May 14, 2019 at 05:15:31PM +0200, Markus Armbruster wrote: >> "make check-unit" fails for me: >> >> TESTcheck-unit: tests/test-crypto-tlscredsx509 >> Unexpected error in object_new_with_propv() at >> /work/armbru/qemu/qom/object.c:674: >> invalid object type: tls-creds-x509 >> >> and >> >> TESTcheck-unit: tests/test-io-channel-tls >> Unexpected error in object_new_with_propv() at >> /work/armbru/qemu/qom/object.c:674: >> invalid object type: tls-creds-x509 >> >> I haven't looked further. > > I have a nasty feeling it is caused by > > Subject: [PATCH v6 02/25] crypto: Merge crypto-obj-y into libqemuutil.a > > The QOM objects are not directly used by most of the code. We rely on > the constructor registering the QOM object and then we request an > instance of it via the type name. So there's no direct function calls > from any code into the crypto object impls. > > When we put the crypto objects into libqemuutil.a the linker is not > intelligent enough to see the constructor and so thinks all these > QOM object impls are unused and discards them when linking the final > binary. Yes, that would do it. We would need something in the test that forces the objects into the link. Without having yet looked at the test cases, any ideas? r~
Re: [Qemu-devel] [PATCH v6 00/25] Add qemu_getrandom and ARMv8.5-RNG etc
On Tue, May 14, 2019 at 05:15:31PM +0200, Markus Armbruster wrote: > "make check-unit" fails for me: > > TESTcheck-unit: tests/test-crypto-tlscredsx509 > Unexpected error in object_new_with_propv() at > /work/armbru/qemu/qom/object.c:674: > invalid object type: tls-creds-x509 > > and > > TESTcheck-unit: tests/test-io-channel-tls > Unexpected error in object_new_with_propv() at > /work/armbru/qemu/qom/object.c:674: > invalid object type: tls-creds-x509 > > I haven't looked further. I have a nasty feeling it is caused by Subject: [PATCH v6 02/25] crypto: Merge crypto-obj-y into libqemuutil.a The QOM objects are not directly used by most of the code. We rely on the constructor registering the QOM object and then we request an instance of it via the type name. So there's no direct function calls from any code into the crypto object impls. When we put the crypto objects into libqemuutil.a the linker is not intelligent enough to see the constructor and so thinks all these QOM object impls are unused and discards them when linking the final binary. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH v6 00/25] Add qemu_getrandom and ARMv8.5-RNG etc
"make check-unit" fails for me: TESTcheck-unit: tests/test-crypto-tlscredsx509 Unexpected error in object_new_with_propv() at /work/armbru/qemu/qom/object.c:674: invalid object type: tls-creds-x509 and TESTcheck-unit: tests/test-io-channel-tls Unexpected error in object_new_with_propv() at /work/armbru/qemu/qom/object.c:674: invalid object type: tls-creds-x509 I haven't looked further.
