On Mon, Sep 21, 2020 at 7:06 PM Alex Bennée <
alex.bennee@linaro.org> wrote:
>
>
> Alex Bennée <
alex.bennee@linaro.org> writes:
>
> > Richard Henderson <
richard.henderson@linaro.org> 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 <
richard.henderson@linaro.org>
> >> ---
> >> Cc: Paolo Bonzini <
pbonzini@redhat.com>
> >> 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