| Message ID | 20240522-xkb-v3-3-c429de860fa1@daynix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix sanitizer errors with clang 18.1.1 | expand |
On Wed, May 22, 2024 at 12:49 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote: > The non-standard .fa library prefix breaks the link source > de-duplication done by Meson so drop it. Can you show the difference in the command lines? One possibility to force de-duplication of objects is to change "link_whole: foo" to "objects: foo.extract_all_objects(recursive: false)" in all the declare_dependency() invocations that involve a 'fa' archive. This completely gets rid of the archives, which now become just a dummy target. I have gotten reports of "ld" exhausting the limit of open files when using thin archives (thin archives contain "symbolic links" to the files with the actual object code, thus reducing disk usage), and this would also be fixed. The disadvantage is requiring a bump to Meson 1.1.x as the minimum required version (the recommended version is 1.2.x because earlier versions are incompatible with recent macOS). It could be done before this patch (because then this patch is a total no-op), or after too to fix the immediate issue with sanitizer builds. Paolo > The lack of link source de-duplication causes AddressSanitizer to > complain ODR violations, and makes GNU ld abort when combined with > clang's LTO. > > Previously, the non-standard prefix was necessary for fork-fuzzing. > Meson wraps all standard-prefixed libraries with --start-group and > --end-group. This made a fork-fuzz.ld linker script wrapped as well and > broke builds. Commit d2e6f9272d33 ("fuzz: remove fork-fuzzing > scaffolding") dropped fork-fuzzing so we can now restore the standard > prefix. > > The occurences of the prefix were detected and removed by performing > a tree-wide search with 'fa' and .fa (note the quotes and dot). > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > --- > docs/devel/build-system.rst | 5 ----- > meson.build | 17 ++--------------- > stubs/blk-exp-close-all.c | 2 +- > .gitlab-ci.d/buildtest-template.yml | 2 -- > .gitlab-ci.d/buildtest.yml | 2 -- > gdbstub/meson.build | 2 -- > tcg/meson.build | 2 -- > tests/Makefile.include | 2 +- > tests/qtest/libqos/meson.build | 1 - > 9 files changed, 4 insertions(+), 31 deletions(-) > > diff --git a/docs/devel/build-system.rst b/docs/devel/build-system.rst > index 09caf2f8e199..5baf027b7614 100644 > --- a/docs/devel/build-system.rst > +++ b/docs/devel/build-system.rst > @@ -236,15 +236,10 @@ Subsystem sourcesets: > are then turned into static libraries as follows:: > > libchardev = static_library('chardev', chardev_ss.sources(), > - name_suffix: 'fa', > build_by_default: false) > > chardev = declare_dependency(link_whole: libchardev) > > - As of Meson 0.55.1, the special ``.fa`` suffix should be used for everything > - that is used with ``link_whole``, to ensure that the link flags are placed > - correctly in the command line. > - > Target-independent emulator sourcesets: > Various general purpose helper code is compiled only once and > the .o files are linked into all output binaries that need it. > diff --git a/meson.build b/meson.build > index 3c3ad0d5f5eb..9eaf218609eb 100644 > --- a/meson.build > +++ b/meson.build > @@ -3462,14 +3462,12 @@ endif > qom_ss = qom_ss.apply({}) > libqom = static_library('qom', qom_ss.sources() + genh, > dependencies: [qom_ss.dependencies()], > - name_suffix: 'fa', > build_by_default: false) > qom = declare_dependency(link_whole: libqom) > > event_loop_base = files('event-loop-base.c') > event_loop_base = static_library('event-loop-base', > sources: event_loop_base + genh, > - name_suffix: 'fa', > build_by_default: false) > event_loop_base = declare_dependency(link_whole: event_loop_base, > dependencies: [qom]) > @@ -3703,7 +3701,6 @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms', > authz_ss = authz_ss.apply({}) > libauthz = static_library('authz', authz_ss.sources() + genh, > dependencies: [authz_ss.dependencies()], > - name_suffix: 'fa', > build_by_default: false) > > authz = declare_dependency(link_whole: libauthz, > @@ -3712,7 +3709,6 @@ authz = declare_dependency(link_whole: libauthz, > crypto_ss = crypto_ss.apply({}) > libcrypto = static_library('crypto', crypto_ss.sources() + genh, > dependencies: [crypto_ss.dependencies()], > - name_suffix: 'fa', > build_by_default: false) > > crypto = declare_dependency(link_whole: libcrypto, > @@ -3722,13 +3718,11 @@ io_ss = io_ss.apply({}) > libio = static_library('io', io_ss.sources() + genh, > dependencies: [io_ss.dependencies()], > link_with: libqemuutil, > - name_suffix: 'fa', > build_by_default: false) > > io = declare_dependency(link_whole: libio, dependencies: [crypto, qom]) > > libmigration = static_library('migration', sources: migration_files + genh, > - name_suffix: 'fa', > build_by_default: false) > migration = declare_dependency(link_with: libmigration, > dependencies: [zlib, qom, io]) > @@ -3738,7 +3732,6 @@ block_ss = block_ss.apply({}) > libblock = static_library('block', block_ss.sources() + genh, > dependencies: block_ss.dependencies(), > link_depends: block_syms, > - name_suffix: 'fa', > build_by_default: false) > > block = declare_dependency(link_whole: [libblock], > @@ -3748,7 +3741,6 @@ block = declare_dependency(link_whole: [libblock], > blockdev_ss = blockdev_ss.apply({}) > libblockdev = static_library('blockdev', blockdev_ss.sources() + genh, > dependencies: blockdev_ss.dependencies(), > - name_suffix: 'fa', > build_by_default: false) > > blockdev = declare_dependency(link_whole: [libblockdev], > @@ -3757,13 +3749,11 @@ blockdev = declare_dependency(link_whole: [libblockdev], > qmp_ss = qmp_ss.apply({}) > libqmp = static_library('qmp', qmp_ss.sources() + genh, > dependencies: qmp_ss.dependencies(), > - name_suffix: 'fa', > build_by_default: false) > > qmp = declare_dependency(link_whole: [libqmp]) > > libchardev = static_library('chardev', chardev_ss.sources() + genh, > - name_suffix: 'fa', > dependencies: chardev_ss.dependencies(), > build_by_default: false) > > @@ -3771,7 +3761,6 @@ chardev = declare_dependency(link_whole: libchardev) > > hwcore_ss = hwcore_ss.apply({}) > libhwcore = static_library('hwcore', sources: hwcore_ss.sources() + genh, > - name_suffix: 'fa', > build_by_default: false) > hwcore = declare_dependency(link_whole: libhwcore) > common_ss.add(hwcore) > @@ -3807,8 +3796,7 @@ common_all = static_library('common', > sources: common_ss.all_sources() + genh, > include_directories: common_user_inc, > implicit_include_directories: false, > - dependencies: common_ss.all_dependencies(), > - name_suffix: 'fa') > + dependencies: common_ss.all_dependencies()) > > feature_to_c = find_program('scripts/feature_to_c.py') > > @@ -3909,8 +3897,7 @@ foreach target : target_dirs > objects: objects, > include_directories: target_inc, > c_args: c_args, > - build_by_default: false, > - name_suffix: 'fa') > + build_by_default: false) > > if target.endswith('-softmmu') > execs = [{ > diff --git a/stubs/blk-exp-close-all.c b/stubs/blk-exp-close-all.c > index 1c7131676392..2f68e06d7d05 100644 > --- a/stubs/blk-exp-close-all.c > +++ b/stubs/blk-exp-close-all.c > @@ -1,7 +1,7 @@ > #include "qemu/osdep.h" > #include "block/export.h" > > -/* Only used in programs that support block exports (libblockdev.fa) */ > +/* Only used in programs that support block exports (libblockdev.a) */ > void blk_exp_close_all(void) > { > } > diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml > index 22045add8064..69e468a576ba 100644 > --- a/.gitlab-ci.d/buildtest-template.yml > +++ b/.gitlab-ci.d/buildtest-template.yml > @@ -45,10 +45,8 @@ > exclude: > - build/**/*.p > - build/**/*.a.p > - - build/**/*.fa.p > - build/**/*.c.o > - build/**/*.c.o.d > - - build/**/*.fa > > .common_test_job_template: > extends: .base_job_template > diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml > index cfdff175c389..c156e6f1d90e 100644 > --- a/.gitlab-ci.d/buildtest.yml > +++ b/.gitlab-ci.d/buildtest.yml > @@ -178,10 +178,8 @@ build-previous-qemu: > exclude: > - build-previous/**/*.p > - build-previous/**/*.a.p > - - build-previous/**/*.fa.p > - build-previous/**/*.c.o > - build-previous/**/*.c.o.d > - - build-previous/**/*.fa > needs: > job: amd64-opensuse-leap-container > variables: > diff --git a/gdbstub/meson.build b/gdbstub/meson.build > index da5721d8452b..c91e398ae726 100644 > --- a/gdbstub/meson.build > +++ b/gdbstub/meson.build > @@ -19,13 +19,11 @@ gdb_system_ss = gdb_system_ss.apply({}) > > libgdb_user = static_library('gdb_user', > gdb_user_ss.sources() + genh, > - name_suffix: 'fa', > c_args: '-DCONFIG_USER_ONLY', > build_by_default: false) > > libgdb_system = static_library('gdb_system', > gdb_system_ss.sources() + genh, > - name_suffix: 'fa', > build_by_default: false) > > gdb_user = declare_dependency(link_whole: libgdb_user) > diff --git a/tcg/meson.build b/tcg/meson.build > index 8251589fd4e9..f941413d5801 100644 > --- a/tcg/meson.build > +++ b/tcg/meson.build > @@ -31,7 +31,6 @@ tcg_ss = tcg_ss.apply({}) > > libtcg_user = static_library('tcg_user', > tcg_ss.sources() + genh, > - name_suffix: 'fa', > c_args: '-DCONFIG_USER_ONLY', > build_by_default: false) > > @@ -41,7 +40,6 @@ user_ss.add(tcg_user) > > libtcg_system = static_library('tcg_system', > tcg_ss.sources() + genh, > - name_suffix: 'fa', > c_args: '-DCONFIG_SOFTMMU', > build_by_default: false) > > diff --git a/tests/Makefile.include b/tests/Makefile.include > index c9d1674bd070..d39d5dd6a43e 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -87,7 +87,7 @@ distclean-tcg: $(DISTCLEAN_TCG_TARGET_RULES) > .PHONY: check-venv check-avocado check-acceptance check-acceptance-deprecated-warning > > # Build up our target list from the filtered list of ninja targets > -TARGETS=$(patsubst libqemu-%.fa, %, $(filter libqemu-%.fa, $(ninja-targets))) > +TARGETS=$(patsubst libqemu-%.a, %, $(filter libqemu-%.a, $(ninja-targets))) > > TESTS_VENV_TOKEN=$(BUILD_DIR)/pyvenv/tests.group > TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results > diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build > index 3aed6efcb8d1..45b81c83ade3 100644 > --- a/tests/qtest/libqos/meson.build > +++ b/tests/qtest/libqos/meson.build > @@ -68,7 +68,6 @@ if have_virtfs > endif > > libqos = static_library('qos', libqos_srcs + genh, > - name_suffix: 'fa', > build_by_default: false) > > qos = declare_dependency(link_whole: libqos) > > -- > 2.45.1 >
On Wed, May 22, 2024 at 3:45 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On Wed, May 22, 2024 at 12:49 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote: > > The non-standard .fa library prefix breaks the link source > > de-duplication done by Meson so drop it. > > Can you show the difference in the command lines? > > One possibility to force de-duplication of objects is to change > "link_whole: foo" to "objects: foo.extract_all_objects(recursive: > false)" in all the declare_dependency() invocations that involve a > 'fa' archive. > > This completely gets rid of the archives, which now become just a > dummy target. I have gotten reports of "ld" exhausting the limit of > open files when using thin archives (thin archives contain "symbolic > links" to the files with the actual object code, thus reducing disk > usage), and this would also be fixed. Ah, I forgot that this would also fix an issue with link_whole dependencies (https://github.com/mesonbuild/meson/pull/8151). It would be possible to revert 3eacf70bb5a83e4775ad8003cbca63a40f70c8c2 and instead just add gnutls to the crypto variable. Paolo > The disadvantage is requiring a bump to Meson 1.1.x as the minimum > required version (the recommended version is 1.2.x because earlier > versions are incompatible with recent macOS). It could be done before > this patch (because then this patch is a total no-op), or after too to > fix the immediate issue with sanitizer builds. > > Paolo > > > The lack of link source de-duplication causes AddressSanitizer to > > complain ODR violations, and makes GNU ld abort when combined with > > clang's LTO. > > > > Previously, the non-standard prefix was necessary for fork-fuzzing. > > Meson wraps all standard-prefixed libraries with --start-group and > > --end-group. This made a fork-fuzz.ld linker script wrapped as well and > > broke builds. Commit d2e6f9272d33 ("fuzz: remove fork-fuzzing > > scaffolding") dropped fork-fuzzing so we can now restore the standard > > prefix. > > > > The occurences of the prefix were detected and removed by performing > > a tree-wide search with 'fa' and .fa (note the quotes and dot). > > > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > > --- > > docs/devel/build-system.rst | 5 ----- > > meson.build | 17 ++--------------- > > stubs/blk-exp-close-all.c | 2 +- > > .gitlab-ci.d/buildtest-template.yml | 2 -- > > .gitlab-ci.d/buildtest.yml | 2 -- > > gdbstub/meson.build | 2 -- > > tcg/meson.build | 2 -- > > tests/Makefile.include | 2 +- > > tests/qtest/libqos/meson.build | 1 - > > 9 files changed, 4 insertions(+), 31 deletions(-) > > > > diff --git a/docs/devel/build-system.rst b/docs/devel/build-system.rst > > index 09caf2f8e199..5baf027b7614 100644 > > --- a/docs/devel/build-system.rst > > +++ b/docs/devel/build-system.rst > > @@ -236,15 +236,10 @@ Subsystem sourcesets: > > are then turned into static libraries as follows:: > > > > libchardev = static_library('chardev', chardev_ss.sources(), > > - name_suffix: 'fa', > > build_by_default: false) > > > > chardev = declare_dependency(link_whole: libchardev) > > > > - As of Meson 0.55.1, the special ``.fa`` suffix should be used for everything > > - that is used with ``link_whole``, to ensure that the link flags are placed > > - correctly in the command line. > > - > > Target-independent emulator sourcesets: > > Various general purpose helper code is compiled only once and > > the .o files are linked into all output binaries that need it. > > diff --git a/meson.build b/meson.build > > index 3c3ad0d5f5eb..9eaf218609eb 100644 > > --- a/meson.build > > +++ b/meson.build > > @@ -3462,14 +3462,12 @@ endif > > qom_ss = qom_ss.apply({}) > > libqom = static_library('qom', qom_ss.sources() + genh, > > dependencies: [qom_ss.dependencies()], > > - name_suffix: 'fa', > > build_by_default: false) > > qom = declare_dependency(link_whole: libqom) > > > > event_loop_base = files('event-loop-base.c') > > event_loop_base = static_library('event-loop-base', > > sources: event_loop_base + genh, > > - name_suffix: 'fa', > > build_by_default: false) > > event_loop_base = declare_dependency(link_whole: event_loop_base, > > dependencies: [qom]) > > @@ -3703,7 +3701,6 @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms', > > authz_ss = authz_ss.apply({}) > > libauthz = static_library('authz', authz_ss.sources() + genh, > > dependencies: [authz_ss.dependencies()], > > - name_suffix: 'fa', > > build_by_default: false) > > > > authz = declare_dependency(link_whole: libauthz, > > @@ -3712,7 +3709,6 @@ authz = declare_dependency(link_whole: libauthz, > > crypto_ss = crypto_ss.apply({}) > > libcrypto = static_library('crypto', crypto_ss.sources() + genh, > > dependencies: [crypto_ss.dependencies()], > > - name_suffix: 'fa', > > build_by_default: false) > > > > crypto = declare_dependency(link_whole: libcrypto, > > @@ -3722,13 +3718,11 @@ io_ss = io_ss.apply({}) > > libio = static_library('io', io_ss.sources() + genh, > > dependencies: [io_ss.dependencies()], > > link_with: libqemuutil, > > - name_suffix: 'fa', > > build_by_default: false) > > > > io = declare_dependency(link_whole: libio, dependencies: [crypto, qom]) > > > > libmigration = static_library('migration', sources: migration_files + genh, > > - name_suffix: 'fa', > > build_by_default: false) > > migration = declare_dependency(link_with: libmigration, > > dependencies: [zlib, qom, io]) > > @@ -3738,7 +3732,6 @@ block_ss = block_ss.apply({}) > > libblock = static_library('block', block_ss.sources() + genh, > > dependencies: block_ss.dependencies(), > > link_depends: block_syms, > > - name_suffix: 'fa', > > build_by_default: false) > > > > block = declare_dependency(link_whole: [libblock], > > @@ -3748,7 +3741,6 @@ block = declare_dependency(link_whole: [libblock], > > blockdev_ss = blockdev_ss.apply({}) > > libblockdev = static_library('blockdev', blockdev_ss.sources() + genh, > > dependencies: blockdev_ss.dependencies(), > > - name_suffix: 'fa', > > build_by_default: false) > > > > blockdev = declare_dependency(link_whole: [libblockdev], > > @@ -3757,13 +3749,11 @@ blockdev = declare_dependency(link_whole: [libblockdev], > > qmp_ss = qmp_ss.apply({}) > > libqmp = static_library('qmp', qmp_ss.sources() + genh, > > dependencies: qmp_ss.dependencies(), > > - name_suffix: 'fa', > > build_by_default: false) > > > > qmp = declare_dependency(link_whole: [libqmp]) > > > > libchardev = static_library('chardev', chardev_ss.sources() + genh, > > - name_suffix: 'fa', > > dependencies: chardev_ss.dependencies(), > > build_by_default: false) > > > > @@ -3771,7 +3761,6 @@ chardev = declare_dependency(link_whole: libchardev) > > > > hwcore_ss = hwcore_ss.apply({}) > > libhwcore = static_library('hwcore', sources: hwcore_ss.sources() + genh, > > - name_suffix: 'fa', > > build_by_default: false) > > hwcore = declare_dependency(link_whole: libhwcore) > > common_ss.add(hwcore) > > @@ -3807,8 +3796,7 @@ common_all = static_library('common', > > sources: common_ss.all_sources() + genh, > > include_directories: common_user_inc, > > implicit_include_directories: false, > > - dependencies: common_ss.all_dependencies(), > > - name_suffix: 'fa') > > + dependencies: common_ss.all_dependencies()) > > > > feature_to_c = find_program('scripts/feature_to_c.py') > > > > @@ -3909,8 +3897,7 @@ foreach target : target_dirs > > objects: objects, > > include_directories: target_inc, > > c_args: c_args, > > - build_by_default: false, > > - name_suffix: 'fa') > > + build_by_default: false) > > > > if target.endswith('-softmmu') > > execs = [{ > > diff --git a/stubs/blk-exp-close-all.c b/stubs/blk-exp-close-all.c > > index 1c7131676392..2f68e06d7d05 100644 > > --- a/stubs/blk-exp-close-all.c > > +++ b/stubs/blk-exp-close-all.c > > @@ -1,7 +1,7 @@ > > #include "qemu/osdep.h" > > #include "block/export.h" > > > > -/* Only used in programs that support block exports (libblockdev.fa) */ > > +/* Only used in programs that support block exports (libblockdev.a) */ > > void blk_exp_close_all(void) > > { > > } > > diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml > > index 22045add8064..69e468a576ba 100644 > > --- a/.gitlab-ci.d/buildtest-template.yml > > +++ b/.gitlab-ci.d/buildtest-template.yml > > @@ -45,10 +45,8 @@ > > exclude: > > - build/**/*.p > > - build/**/*.a.p > > - - build/**/*.fa.p > > - build/**/*.c.o > > - build/**/*.c.o.d > > - - build/**/*.fa > > > > .common_test_job_template: > > extends: .base_job_template > > diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml > > index cfdff175c389..c156e6f1d90e 100644 > > --- a/.gitlab-ci.d/buildtest.yml > > +++ b/.gitlab-ci.d/buildtest.yml > > @@ -178,10 +178,8 @@ build-previous-qemu: > > exclude: > > - build-previous/**/*.p > > - build-previous/**/*.a.p > > - - build-previous/**/*.fa.p > > - build-previous/**/*.c.o > > - build-previous/**/*.c.o.d > > - - build-previous/**/*.fa > > needs: > > job: amd64-opensuse-leap-container > > variables: > > diff --git a/gdbstub/meson.build b/gdbstub/meson.build > > index da5721d8452b..c91e398ae726 100644 > > --- a/gdbstub/meson.build > > +++ b/gdbstub/meson.build > > @@ -19,13 +19,11 @@ gdb_system_ss = gdb_system_ss.apply({}) > > > > libgdb_user = static_library('gdb_user', > > gdb_user_ss.sources() + genh, > > - name_suffix: 'fa', > > c_args: '-DCONFIG_USER_ONLY', > > build_by_default: false) > > > > libgdb_system = static_library('gdb_system', > > gdb_system_ss.sources() + genh, > > - name_suffix: 'fa', > > build_by_default: false) > > > > gdb_user = declare_dependency(link_whole: libgdb_user) > > diff --git a/tcg/meson.build b/tcg/meson.build > > index 8251589fd4e9..f941413d5801 100644 > > --- a/tcg/meson.build > > +++ b/tcg/meson.build > > @@ -31,7 +31,6 @@ tcg_ss = tcg_ss.apply({}) > > > > libtcg_user = static_library('tcg_user', > > tcg_ss.sources() + genh, > > - name_suffix: 'fa', > > c_args: '-DCONFIG_USER_ONLY', > > build_by_default: false) > > > > @@ -41,7 +40,6 @@ user_ss.add(tcg_user) > > > > libtcg_system = static_library('tcg_system', > > tcg_ss.sources() + genh, > > - name_suffix: 'fa', > > c_args: '-DCONFIG_SOFTMMU', > > build_by_default: false) > > > > diff --git a/tests/Makefile.include b/tests/Makefile.include > > index c9d1674bd070..d39d5dd6a43e 100644 > > --- a/tests/Makefile.include > > +++ b/tests/Makefile.include > > @@ -87,7 +87,7 @@ distclean-tcg: $(DISTCLEAN_TCG_TARGET_RULES) > > .PHONY: check-venv check-avocado check-acceptance check-acceptance-deprecated-warning > > > > # Build up our target list from the filtered list of ninja targets > > -TARGETS=$(patsubst libqemu-%.fa, %, $(filter libqemu-%.fa, $(ninja-targets))) > > +TARGETS=$(patsubst libqemu-%.a, %, $(filter libqemu-%.a, $(ninja-targets))) > > > > TESTS_VENV_TOKEN=$(BUILD_DIR)/pyvenv/tests.group > > TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results > > diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build > > index 3aed6efcb8d1..45b81c83ade3 100644 > > --- a/tests/qtest/libqos/meson.build > > +++ b/tests/qtest/libqos/meson.build > > @@ -68,7 +68,6 @@ if have_virtfs > > endif > > > > libqos = static_library('qos', libqos_srcs + genh, > > - name_suffix: 'fa', > > build_by_default: false) > > > > qos = declare_dependency(link_whole: libqos) > > > > -- > > 2.45.1 > >
On 2024/05/22 22:45, Paolo Bonzini wrote: > On Wed, May 22, 2024 at 12:49 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote: >> The non-standard .fa library prefix breaks the link source >> de-duplication done by Meson so drop it. > > Can you show the difference in the command lines? Without this patch: clang -o qemu-io qemu-io.p/qemu-io.c.o -Werror -flto -Wl,--as-needed -Wl,--no-undefined -pie -Wl,--whole-archive libblock.fa libcrypto.fa libauthz.fa libqom.fa libio.fa libevent-loop-base.fa -Wl,--no-whole-archive -fsanitize=cfi-icall -fsanitize-cfi-icall-generalize-pointers -fsanitize=undefined -fsanitize=address -fstack-protector-strong -Wl,-z,relro -Wl,-z,now -fuse-ld=lld -Wl,--start-group libqemuutil.a subprojects/libvhost-user/libvhost-user-glib.a subprojects/libvhost-user/libvhost-user.a libblock.fa libcrypto.fa libauthz.fa libqom.fa libio.fa libevent-loop-base.fa @block.syms /usr/lib64/libgio-2.0.so /usr/lib64/libgobject-2.0.so /usr/lib64/libglib-2.0.so /usr/lib64/libgmodule-2.0.so -pthread /usr/lib64/libgnutls.so -lm /usr/lib64/libpixman-1.so /usr/lib64/libzstd.so /usr/lib64/libz.so /usr/lib64/libcurl.so /usr/lib64/libssh.so -lbz2 -lpam -Wl,--end-group With this patch: clang -o qemu-io qemu-io.p/qemu-io.c.o -Werror -flto -Wl,--as-needed -Wl,--no-undefined -pie -Wl,--whole-archive -Wl,--start-group libblock.a libcrypto.a libauthz.a libqom.a libio.a libevent-loop-base.a -Wl,--no-whole-archive -fsanitize=cfi-icall -fsanitize-cfi-icall-generalize-pointers -fsanitize=undefined -fsanitize=address -fstack-protector-strong -Wl,-z,relro -Wl,-z,now -fuse-ld=lld libqemuutil.a subprojects/libvhost-user/libvhost-user-glib.a subprojects/libvhost-user/libvhost-user.a @block.syms /usr/lib64/libgio-2.0.so /usr/lib64/libgobject-2.0.so /usr/lib64/libglib-2.0.so /usr/lib64/libgmodule-2.0.so -pthread /usr/lib64/libgnutls.so -lm /usr/lib64/libpixman-1.so /usr/lib64/libzstd.so /usr/lib64/libz.so /usr/lib64/libcurl.so /usr/lib64/libssh.so -lbz2 -lpam -Wl,--end-group > > One possibility to force de-duplication of objects is to change > "link_whole: foo" to "objects: foo.extract_all_objects(recursive: > false)" in all the declare_dependency() invocations that involve a > 'fa' archive. > > This completely gets rid of the archives, which now become just a > dummy target. I have gotten reports of "ld" exhausting the limit of > open files when using thin archives (thin archives contain "symbolic > links" to the files with the actual object code, thus reducing disk > usage), and this would also be fixed. > > The disadvantage is requiring a bump to Meson 1.1.x as the minimum > required version (the recommended version is 1.2.x because earlier > versions are incompatible with recent macOS). It could be done before > this patch (because then this patch is a total no-op), or after too to > fix the immediate issue with sanitizer builds. I wrote such a change and applied after this patch, but it caused dependencies to be ignored. Please see "[PATCH RFC 0/2] meson: Pass objects to declare_dependency()", which I sent earlier. Regards, Akihiko Odaki
diff --git a/docs/devel/build-system.rst b/docs/devel/build-system.rst index 09caf2f8e199..5baf027b7614 100644 --- a/docs/devel/build-system.rst +++ b/docs/devel/build-system.rst @@ -236,15 +236,10 @@ Subsystem sourcesets: are then turned into static libraries as follows:: libchardev = static_library('chardev', chardev_ss.sources(), - name_suffix: 'fa', build_by_default: false) chardev = declare_dependency(link_whole: libchardev) - As of Meson 0.55.1, the special ``.fa`` suffix should be used for everything - that is used with ``link_whole``, to ensure that the link flags are placed - correctly in the command line. - Target-independent emulator sourcesets: Various general purpose helper code is compiled only once and the .o files are linked into all output binaries that need it. diff --git a/meson.build b/meson.build index 3c3ad0d5f5eb..9eaf218609eb 100644 --- a/meson.build +++ b/meson.build @@ -3462,14 +3462,12 @@ endif qom_ss = qom_ss.apply({}) libqom = static_library('qom', qom_ss.sources() + genh, dependencies: [qom_ss.dependencies()], - name_suffix: 'fa', build_by_default: false) qom = declare_dependency(link_whole: libqom) event_loop_base = files('event-loop-base.c') event_loop_base = static_library('event-loop-base', sources: event_loop_base + genh, - name_suffix: 'fa', build_by_default: false) event_loop_base = declare_dependency(link_whole: event_loop_base, dependencies: [qom]) @@ -3703,7 +3701,6 @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms', authz_ss = authz_ss.apply({}) libauthz = static_library('authz', authz_ss.sources() + genh, dependencies: [authz_ss.dependencies()], - name_suffix: 'fa', build_by_default: false) authz = declare_dependency(link_whole: libauthz, @@ -3712,7 +3709,6 @@ authz = declare_dependency(link_whole: libauthz, crypto_ss = crypto_ss.apply({}) libcrypto = static_library('crypto', crypto_ss.sources() + genh, dependencies: [crypto_ss.dependencies()], - name_suffix: 'fa', build_by_default: false) crypto = declare_dependency(link_whole: libcrypto, @@ -3722,13 +3718,11 @@ io_ss = io_ss.apply({}) libio = static_library('io', io_ss.sources() + genh, dependencies: [io_ss.dependencies()], link_with: libqemuutil, - name_suffix: 'fa', build_by_default: false) io = declare_dependency(link_whole: libio, dependencies: [crypto, qom]) libmigration = static_library('migration', sources: migration_files + genh, - name_suffix: 'fa', build_by_default: false) migration = declare_dependency(link_with: libmigration, dependencies: [zlib, qom, io]) @@ -3738,7 +3732,6 @@ block_ss = block_ss.apply({}) libblock = static_library('block', block_ss.sources() + genh, dependencies: block_ss.dependencies(), link_depends: block_syms, - name_suffix: 'fa', build_by_default: false) block = declare_dependency(link_whole: [libblock], @@ -3748,7 +3741,6 @@ block = declare_dependency(link_whole: [libblock], blockdev_ss = blockdev_ss.apply({}) libblockdev = static_library('blockdev', blockdev_ss.sources() + genh, dependencies: blockdev_ss.dependencies(), - name_suffix: 'fa', build_by_default: false) blockdev = declare_dependency(link_whole: [libblockdev], @@ -3757,13 +3749,11 @@ blockdev = declare_dependency(link_whole: [libblockdev], qmp_ss = qmp_ss.apply({}) libqmp = static_library('qmp', qmp_ss.sources() + genh, dependencies: qmp_ss.dependencies(), - name_suffix: 'fa', build_by_default: false) qmp = declare_dependency(link_whole: [libqmp]) libchardev = static_library('chardev', chardev_ss.sources() + genh, - name_suffix: 'fa', dependencies: chardev_ss.dependencies(), build_by_default: false) @@ -3771,7 +3761,6 @@ chardev = declare_dependency(link_whole: libchardev) hwcore_ss = hwcore_ss.apply({}) libhwcore = static_library('hwcore', sources: hwcore_ss.sources() + genh, - name_suffix: 'fa', build_by_default: false) hwcore = declare_dependency(link_whole: libhwcore) common_ss.add(hwcore) @@ -3807,8 +3796,7 @@ common_all = static_library('common', sources: common_ss.all_sources() + genh, include_directories: common_user_inc, implicit_include_directories: false, - dependencies: common_ss.all_dependencies(), - name_suffix: 'fa') + dependencies: common_ss.all_dependencies()) feature_to_c = find_program('scripts/feature_to_c.py') @@ -3909,8 +3897,7 @@ foreach target : target_dirs objects: objects, include_directories: target_inc, c_args: c_args, - build_by_default: false, - name_suffix: 'fa') + build_by_default: false) if target.endswith('-softmmu') execs = [{ diff --git a/stubs/blk-exp-close-all.c b/stubs/blk-exp-close-all.c index 1c7131676392..2f68e06d7d05 100644 --- a/stubs/blk-exp-close-all.c +++ b/stubs/blk-exp-close-all.c @@ -1,7 +1,7 @@ #include "qemu/osdep.h" #include "block/export.h" -/* Only used in programs that support block exports (libblockdev.fa) */ +/* Only used in programs that support block exports (libblockdev.a) */ void blk_exp_close_all(void) { } diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml index 22045add8064..69e468a576ba 100644 --- a/.gitlab-ci.d/buildtest-template.yml +++ b/.gitlab-ci.d/buildtest-template.yml @@ -45,10 +45,8 @@ exclude: - build/**/*.p - build/**/*.a.p - - build/**/*.fa.p - build/**/*.c.o - build/**/*.c.o.d - - build/**/*.fa .common_test_job_template: extends: .base_job_template diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml index cfdff175c389..c156e6f1d90e 100644 --- a/.gitlab-ci.d/buildtest.yml +++ b/.gitlab-ci.d/buildtest.yml @@ -178,10 +178,8 @@ build-previous-qemu: exclude: - build-previous/**/*.p - build-previous/**/*.a.p - - build-previous/**/*.fa.p - build-previous/**/*.c.o - build-previous/**/*.c.o.d - - build-previous/**/*.fa needs: job: amd64-opensuse-leap-container variables: diff --git a/gdbstub/meson.build b/gdbstub/meson.build index da5721d8452b..c91e398ae726 100644 --- a/gdbstub/meson.build +++ b/gdbstub/meson.build @@ -19,13 +19,11 @@ gdb_system_ss = gdb_system_ss.apply({}) libgdb_user = static_library('gdb_user', gdb_user_ss.sources() + genh, - name_suffix: 'fa', c_args: '-DCONFIG_USER_ONLY', build_by_default: false) libgdb_system = static_library('gdb_system', gdb_system_ss.sources() + genh, - name_suffix: 'fa', build_by_default: false) gdb_user = declare_dependency(link_whole: libgdb_user) diff --git a/tcg/meson.build b/tcg/meson.build index 8251589fd4e9..f941413d5801 100644 --- a/tcg/meson.build +++ b/tcg/meson.build @@ -31,7 +31,6 @@ tcg_ss = tcg_ss.apply({}) libtcg_user = static_library('tcg_user', tcg_ss.sources() + genh, - name_suffix: 'fa', c_args: '-DCONFIG_USER_ONLY', build_by_default: false) @@ -41,7 +40,6 @@ user_ss.add(tcg_user) libtcg_system = static_library('tcg_system', tcg_ss.sources() + genh, - name_suffix: 'fa', c_args: '-DCONFIG_SOFTMMU', build_by_default: false) diff --git a/tests/Makefile.include b/tests/Makefile.include index c9d1674bd070..d39d5dd6a43e 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -87,7 +87,7 @@ distclean-tcg: $(DISTCLEAN_TCG_TARGET_RULES) .PHONY: check-venv check-avocado check-acceptance check-acceptance-deprecated-warning # Build up our target list from the filtered list of ninja targets -TARGETS=$(patsubst libqemu-%.fa, %, $(filter libqemu-%.fa, $(ninja-targets))) +TARGETS=$(patsubst libqemu-%.a, %, $(filter libqemu-%.a, $(ninja-targets))) TESTS_VENV_TOKEN=$(BUILD_DIR)/pyvenv/tests.group TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build index 3aed6efcb8d1..45b81c83ade3 100644 --- a/tests/qtest/libqos/meson.build +++ b/tests/qtest/libqos/meson.build @@ -68,7 +68,6 @@ if have_virtfs endif libqos = static_library('qos', libqos_srcs + genh, - name_suffix: 'fa', build_by_default: false) qos = declare_dependency(link_whole: libqos)
The non-standard .fa library prefix breaks the link source de-duplication done by Meson so drop it. The lack of link source de-duplication causes AddressSanitizer to complain ODR violations, and makes GNU ld abort when combined with clang's LTO. Previously, the non-standard prefix was necessary for fork-fuzzing. Meson wraps all standard-prefixed libraries with --start-group and --end-group. This made a fork-fuzz.ld linker script wrapped as well and broke builds. Commit d2e6f9272d33 ("fuzz: remove fork-fuzzing scaffolding") dropped fork-fuzzing so we can now restore the standard prefix. The occurences of the prefix were detected and removed by performing a tree-wide search with 'fa' and .fa (note the quotes and dot). Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> --- docs/devel/build-system.rst | 5 ----- meson.build | 17 ++--------------- stubs/blk-exp-close-all.c | 2 +- .gitlab-ci.d/buildtest-template.yml | 2 -- .gitlab-ci.d/buildtest.yml | 2 -- gdbstub/meson.build | 2 -- tcg/meson.build | 2 -- tests/Makefile.include | 2 +- tests/qtest/libqos/meson.build | 1 - 9 files changed, 4 insertions(+), 31 deletions(-)