On Mon, Sep 21, 2020 at 7:06 PM Alex Bennée wrote: > > > Alex Bennée writes: > > > Richard Henderson writes: > > > >> There are better ways to do this, e.g. meson cmake subproject, > >> but that requires cmake 3.7 and some of our CI environments > >> only provide cmake 3.5. > >> > >> Nor can we add a meson.build file to capstone/, because the git > >> submodule would then always report "untracked files". Fixing that > >> would require creating our own branch on the qemu git mirror, at > >> which point we could just as easily create a native meson subproject. > >> > >> Instead, build the library via the main meson.build. > >> > >> This improves the current state of affairs in that we will re-link > >> the qemu executables against a changed libcapstone.a, which we wouldn't > >> do before-hand. In addition, the use of the configuration header file > >> instead of command-line -DEFINES means that we will rebuild the > >> capstone objects with changes to meson.build. > > > > Something is breaking when switching to a branch with this on from > > current master: > > > > Linking target qemu-hppa > > /usr/bin/ld: libcommon.fa.p/disas_alpha.c.o: in function `print_insn_alpha': > > /home/alex/lsrc/qemu.git/builds/all/../../disas/alpha.c:1818: undefined reference to `bfd_getl32' > > collect2: error: ld returned 1 exit status > > make: *** [Makefile.ninja:5965: qemu-alpha] Error 1 > > make: *** Waiting for unfinished jobs.... > > /usr/bin/ld: libcommon.fa.p/disas_hppa.c.o: in function `print_insn_hppa': > > /home/alex/lsrc/qemu.git/builds/all/../../disas/hppa.c:1969: undefined reference to `bfd_getb32' > > collect2: error: ld returned 1 exit status > > make: *** [Makefile.ninja:6259: qemu-hppa] Error 1 > > > > Aggressively wiping out the submodule and doing a fresh configure in a > > empty build directory and I still see a failure: > > > > ../../disas/capstone.c:25:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘cap_skipdata_s390x_cb’ > > cap_skipdata_s390x_cb(const uint8_t *code, size_t code_size, > > ^~~~~~~~~~~~~~~~~~~~~ > > ../../disas/capstone.c:49:17: error: ‘cap_skipdata_s390x_cb’ undeclared here (not in a function); did you mean ‘cap_skipdata_s390x’? > > .callback = cap_skipdata_s390x_cb > > ^~~~~~~~~~~~~~~~~~~~~ > > cap_skipdata_s390x > > Makefile.ninja:1424: recipe for target 'libcommon.fa.p/disas_capstone.c.o' failed > > make: *** [libcommon.fa.p/disas_capstone.c.o] Error 1 > > make: *** Waiting for unfinished jobs.... > > > >> > >> Signed-off-by: Richard Henderson > >> --- > >> Cc: Paolo Bonzini > >> v2: Further reduce probing in configure (paolo), > >> Drop state 'internal' and use 'git' even when it isn't git. > >> Move CONFIG_CAPSTONE to config_host_data. > >> v3: Add Submodules separator; fix default in meson_options.txt. > >> --- > >> configure | 61 +++---------------------- > >> Makefile | 16 ------- > >> meson.build | 111 +++++++++++++++++++++++++++++++++++++++++++--- > >> meson_options.txt | 4 ++ > >> 4 files changed, 115 insertions(+), 77 deletions(-) > >> > >> diff --git a/configure b/configure > >> index 7564479008..76636c430d 100755 > >> --- a/configure > >> +++ b/configure > >> @@ -478,7 +478,7 @@ opengl="" > >> opengl_dmabuf="no" > >> cpuid_h="no" > >> avx2_opt="" > >> -capstone="" > >> +capstone="auto" > >> lzo="" > >> snappy="" > >> bzip2="" > >> @@ -1580,7 +1580,7 @@ for opt do > >> ;; > >> --enable-capstone) capstone="yes" > >> ;; > >> - --enable-capstone=git) capstone="git" > >> + --enable-capstone=git) capstone="internal" > >> ;; > >> --enable-capstone=system) capstone="system" > >> ;; > >> @@ -5128,51 +5128,11 @@ fi > >> # capstone > >> > >> case "$capstone" in > >> - "" | yes) > >> - if $pkg_config capstone; then > >> - capstone=system > >> - elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then > >> - capstone=git > >> - elif test -e "${source_path}/capstone/Makefile" ; then > >> - capstone=internal > >> - elif test -z "$capstone" ; then > >> - capstone=no > >> - else > >> - feature_not_found "capstone" "Install capstone devel or git submodule" > >> - fi > >> - ;; > >> - > >> - system) > >> - if ! $pkg_config capstone; then > >> - feature_not_found "capstone" "Install capstone devel" > >> - fi > >> - ;; > >> -esac > >> - > >> -case "$capstone" in > >> - git | internal) > >> - if test "$capstone" = git; then > >> + auto | yes | internal) > >> + # Simpler to always update submodule, even if not needed. > >> + if test -e "${source_path}/.git" && test $git_update = 'yes' ; then > >> git_submodules="${git_submodules} capstone" > >> fi > >> - mkdir -p capstone > >> - if test "$mingw32" = "yes"; then > >> - LIBCAPSTONE=capstone.lib > >> - else > >> - LIBCAPSTONE=libcapstone.a > >> - fi > >> - capstone_libs="-Lcapstone -lcapstone" > >> - capstone_cflags="-I${source_path}/capstone/include" > >> - ;; > >> - > >> - system) > >> - capstone_libs="$($pkg_config --libs capstone)" > >> - capstone_cflags="$($pkg_config --cflags capstone)" > >> - ;; > >> - > >> - no) > >> - ;; > >> - *) > >> - error_exit "Unknown state for capstone: $capstone" > >> ;; > >> esac > >> > >> @@ -7292,11 +7252,6 @@ fi > >> if test "$ivshmem" = "yes" ; then > >> echo "CONFIG_IVSHMEM=y" >> $config_host_mak > >> fi > >> -if test "$capstone" != "no" ; then > >> - echo "CONFIG_CAPSTONE=y" >> $config_host_mak > >> - echo "CAPSTONE_CFLAGS=$capstone_cflags" >> $config_host_mak > >> - echo "CAPSTONE_LIBS=$capstone_libs" >> $config_host_mak > >> -fi > >> if test "$debug_mutex" = "yes" ; then > >> echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak > >> fi > >> @@ -7819,9 +7774,6 @@ done # for target in $targets > >> if [ "$fdt" = "git" ]; then > >> subdirs="$subdirs dtc" > >> fi > >> -if [ "$capstone" = "git" -o "$capstone" = "internal" ]; then > >> - subdirs="$subdirs capstone" > >> -fi > >> echo "SUBDIRS=$subdirs" >> $config_host_mak > >> if test -n "$LIBCAPSTONE"; then > >> echo "LIBCAPSTONE=$LIBCAPSTONE" >> $config_host_mak > >> @@ -8008,7 +7960,8 @@ NINJA=${ninja:-$PWD/ninjatool} $meson setup \ > >> -Db_coverage=$(if test "$gcov" = yes; then echo true; else echo false; fi) \ > >> -Dsdl=$sdl -Dsdl_image=$sdl_image \ > >> -Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png \ > >> - -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f\ > >> + -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f \ > >> + -Dcapstone=$capstone \ > >> $cross_arg \ > >> "$PWD" "$source_path" > >> > >> diff --git a/Makefile b/Makefile > >> index 7c60b9dcb8..f3da1760ad 100644 > >> --- a/Makefile > >> +++ b/Makefile > >> @@ -156,22 +156,6 @@ dtc/all: .git-submodule-status dtc/libfdt > >> dtc/%: .git-submodule-status > >> @mkdir -p $@ > >> > >> -# Overriding CFLAGS causes us to lose defines added in the sub-makefile. > >> -# Not overriding CFLAGS leads to mis-matches between compilation modes. > >> -# Therefore we replicate some of the logic in the sub-makefile. > >> -# Remove all the extra -Warning flags that QEMU uses that Capstone doesn't; > >> -# no need to annoy QEMU developers with such things. > >> -CAP_CFLAGS = $(patsubst -W%,,$(CFLAGS) $(QEMU_CFLAGS)) $(CAPSTONE_CFLAGS) > >> -CAP_CFLAGS += -DCAPSTONE_USE_SYS_DYN_MEM > >> -CAP_CFLAGS += -DCAPSTONE_HAS_ARM > >> -CAP_CFLAGS += -DCAPSTONE_HAS_ARM64 > >> -CAP_CFLAGS += -DCAPSTONE_HAS_POWERPC > >> -CAP_CFLAGS += -DCAPSTONE_HAS_X86 > >> - > >> -.PHONY: capstone/all > >> -capstone/all: .git-submodule-status > >> - $(call quiet-command,$(MAKE) -C $(SRC_PATH)/capstone CAPSTONE_SHARED=no BUILDDIR="$(BUILD_DIR)/capstone" CC="$(CC)" AR="$(AR)" LD="$(LD)" RANLIB="$(RANLIB)" CFLAGS="$(CAP_CFLAGS)" $(SUBDIR_MAKEFLAGS) $(BUILD_DIR)/capstone/$(LIBCAPSTONE)) > >> - > >> .PHONY: slirp/all > >> slirp/all: .git-submodule-status > >> $(call quiet-command,$(MAKE) -C $(SRC_PATH)/slirp \ > >> diff --git a/meson.build b/meson.build > >> index f4d1ab1096..f23273693d 100644 > >> --- a/meson.build > >> +++ b/meson.build > >> @@ -10,6 +10,7 @@ else > >> keyval = import('unstable-keyval') > >> endif > >> ss = import('sourceset') > >> +fs = import('fs') > >> > >> sh = find_program('sh') > >> cc = meson.get_compiler('c') > >> @@ -409,11 +410,6 @@ if 'CONFIG_USB_LIBUSB' in config_host > >> libusb = declare_dependency(compile_args: config_host['LIBUSB_CFLAGS'].split(), > >> link_args: config_host['LIBUSB_LIBS'].split()) > >> endif > >> -capstone = not_found > >> -if 'CONFIG_CAPSTONE' in config_host > >> - capstone = declare_dependency(compile_args: config_host['CAPSTONE_CFLAGS'].split(), > >> - link_args: config_host['CAPSTONE_LIBS'].split()) > >> -endif > >> libpmem = not_found > >> if 'CONFIG_LIBPMEM' in config_host > >> libpmem = declare_dependency(compile_args: config_host['LIBPMEM_CFLAGS'].split(), > >> @@ -470,7 +466,6 @@ foreach k, v: config_host > >> config_host_data.set(k, v == 'y' ? 1 : v) > >> endif > >> endforeach > >> -genh += configure_file(output: 'config-host.h', configuration: config_host_data) > >> > >> minikconf = find_program('scripts/minikconf.py') > >> config_all_devices = {} > >> @@ -610,6 +605,108 @@ config_all += { > >> 'CONFIG_ALL': true, > >> } > >> > >> +# Submodules > >> + > >> +capstone = not_found > >> +capstone_opt = get_option('capstone') > >> +if capstone_opt == 'no' > >> + capstone_opt = false > >> +elif capstone_opt in ['yes', 'auto', 'system'] > >> + have_internal = fs.exists('capstone/Makefile') > >> + capstone = dependency('capstone', static: enable_static, > >> + required: capstone_opt == 'system' or > >> + capstone_opt == 'yes' and not have_internal) > >> + if capstone.found() > >> + capstone_opt = 'system' > >> + elif have_internal > >> + capstone_opt = 'internal' > >> + else > >> + capstone_opt = false > >> + endif > >> +endif > >> +if capstone_opt == 'internal' > >> + capstone_data = configuration_data() > >> + capstone_data.set('CAPSTONE_USE_SYS_DYN_MEM', '1') > >> + > >> + capstone_files = files( > >> + 'capstone/cs.c', > >> + 'capstone/MCInst.c', > >> + 'capstone/MCInstrDesc.c', > >> + 'capstone/MCRegisterInfo.c', > >> + 'capstone/SStream.c', > >> + 'capstone/utils.c' > >> + ) > >> + > >> + if 'CONFIG_ARM_DIS' in config_all_disas > >> + capstone_data.set('CAPSTONE_HAS_ARM', '1') > >> + capstone_files += files( > >> + 'capstone/arch/ARM/ARMDisassembler.c', > >> + 'capstone/arch/ARM/ARMInstPrinter.c', > >> + 'capstone/arch/ARM/ARMMapping.c', > >> + 'capstone/arch/ARM/ARMModule.c' > >> + ) > >> + endif > >> + > >> + # FIXME: This config entry currently depends on a c++ compiler. > >> + # Which is needed for building libvixl, but not for capstone. > >> + if 'CONFIG_ARM_A64_DIS' in config_all_disas > >> + capstone_data.set('CAPSTONE_HAS_ARM64', '1') > >> + capstone_files += files( > >> + 'capstone/arch/AArch64/AArch64BaseInfo.c', > >> + 'capstone/arch/AArch64/AArch64Disassembler.c', > >> + 'capstone/arch/AArch64/AArch64InstPrinter.c', > >> + 'capstone/arch/AArch64/AArch64Mapping.c', > >> + 'capstone/arch/AArch64/AArch64Module.c' > >> + ) > >> + endif > >> + > >> + if 'CONFIG_PPC_DIS' in config_all_disas > >> + capstone_data.set('CAPSTONE_HAS_POWERPC', '1') > >> + capstone_files += files( > >> + 'capstone/arch/PowerPC/PPCDisassembler.c', > >> + 'capstone/arch/PowerPC/PPCInstPrinter.c', > >> + 'capstone/arch/PowerPC/PPCMapping.c', > >> + 'capstone/arch/PowerPC/PPCModule.c' > >> + ) > >> + endif > >> + > >> + if 'CONFIG_I386_DIS' in config_all_disas > >> + capstone_data.set('CAPSTONE_HAS_X86', 1) > >> + capstone_files += files( > >> + 'capstone/arch/X86/X86Disassembler.c', > >> + 'capstone/arch/X86/X86DisassemblerDecoder.c', > >> + 'capstone/arch/X86/X86ATTInstPrinter.c', > >> + 'capstone/arch/X86/X86IntelInstPrinter.c', > >> + 'capstone/arch/X86/X86Mapping.c', > >> + 'capstone/arch/X86/X86Module.c' > >> + ) > >> + endif > >> + > >> + configure_file(output: 'capstone-defs.h', configuration: capstone_data) > >> + > >> + capstone_cargs = [ > >> + # FIXME: There does not seem to be a way to completely replace the c_args > >> + # that come from add_project_arguments() -- we can only add to them. > >> + # So: disable all warnings with a big hammer. > >> + '-Wno-error', '-w', > >> + > >> + # Include all configuration defines via a header file, which will wind up > >> + # as a dependency on the object file, and thus changes here will result > >> + # in a rebuild. > >> + '-include', 'capstone-defs.h' > >> + ] > >> + > >> + libcapstone = static_library('capstone', > >> + sources: capstone_files, > >> + c_args: capstone_cargs, > >> + include_directories: 'capstone/include') > >> + capstone = declare_dependency(link_with: libcapstone, > >> + include_directories: 'capstone/include') > >> +endif > >> +config_host_data.set('CONFIG_CAPSTONE', capstone.found()) > >> + > >> +genh += configure_file(output: 'config-host.h', configuration: config_host_data) > >> + > >> # Generators > >> > >> hxtool = find_program('scripts/hxtool') > >> @@ -1512,7 +1609,7 @@ summary_info += {'vvfat support': config_host.has_key('CONFIG_VVFAT')} > >> summary_info += {'qed support': config_host.has_key('CONFIG_QED')} > >> summary_info += {'parallels support': config_host.has_key('CONFIG_PARALLELS')} > >> summary_info += {'sheepdog support': config_host.has_key('CONFIG_SHEEPDOG')} > >> -summary_info += {'capstone': config_host.has_key('CONFIG_CAPSTONE')} > >> +summary_info += {'capstone': capstone_opt} > >> summary_info += {'libpmem support': config_host.has_key('CONFIG_LIBPMEM')} > >> summary_info += {'libdaxctl support': config_host.has_key('CONFIG_LIBDAXCTL')} > >> summary_info += {'libudev': config_host.has_key('CONFIG_LIBUDEV')} > >> diff --git a/meson_options.txt b/meson_options.txt > >> index 543cf70043..e650a937e7 100644 > >> --- a/meson_options.txt > >> +++ b/meson_options.txt > >> @@ -22,3 +22,7 @@ option('vnc_sasl', type : 'feature', value : 'auto', > >> description: 'SASL authentication for VNC server') > >> option('xkbcommon', type : 'feature', value : 'auto', > >> description: 'xkbcommon support') > >> + > >> +option('capstone', type: 'combo', value: 'auto', > >> + choices: ['no', 'yes', 'auto', 'system', 'internal'], > >> + description: 'Whether and how to find the capstone library') > > > Hmm even more confusing is configure does: > > GIT submodules: ui/keycodemapdb tests/fp/berkeley-testfloat-3 tests/fp/berkeley-softfloat-3 meson dtc capstone slirp > > but also: > > capstone: system > > which I can't quite help but feel is going to be confusing. maybe remove the capstone system support is a good idea after-all. > > -- > Alex Bennée > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo