QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [PATCH v4 01/11] capstone: Convert Makefile bits to meson bits
Date: Mon, 21 Sep 2020 20:17:19 +0100
Message-ID: <87lfh3ndmo.fsf@linaro.org> (raw)
In-Reply-To: <20200921174118.39352-2-richard.henderson@linaro.org>


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.
>
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  configure         |  64 +++-----------------------
>  Makefile          |  16 -------
>  meson.build       | 111 +++++++++++++++++++++++++++++++++++++++++++---
>  meson_options.txt |   4 ++
>  4 files changed, 115 insertions(+), 80 deletions(-)
>
> diff --git a/configure b/configure
> index 7564479008..03915267e2 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,13 +7774,7 @@ 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
> -fi
>  
>  if test "$numa" = "yes"; then
>    echo "CONFIG_NUMA=y" >> $config_host_mak
> @@ -8008,7 +7957,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')


-- 
Alex Bennée


  reply index

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-21 17:41 [PATCH v4 00/11] capstone + disassembler patches Richard Henderson
2020-09-21 17:41 ` [PATCH v4 01/11] capstone: Convert Makefile bits to meson bits Richard Henderson
2020-09-21 19:17   ` Alex Bennée [this message]
2020-09-21 17:41 ` [PATCH v4 02/11] capstone: Update to upstream "next" branch Richard Henderson
2020-09-21 19:18   ` Alex Bennée
2020-09-21 17:41 ` [PATCH v4 03/11] capstone: Require version 4.0 from a system library Richard Henderson
2020-09-21 19:19   ` Alex Bennée
2020-09-22 10:33   ` Philippe Mathieu-Daudé
2020-09-21 17:41 ` [PATCH v4 04/11] disas: Move host asm annotations to tb_gen_code Richard Henderson
2020-09-21 19:29   ` Alex Bennée
2020-09-21 19:53     ` Richard Henderson
2020-09-22  8:26       ` Philippe Mathieu-Daudé
2020-09-22  9:50         ` Alex Bennée
2020-09-21 17:41 ` [PATCH v4 05/11] disas: Clean up CPUDebug initialization Richard Henderson
2020-09-21 19:32   ` Alex Bennée
2020-09-21 17:41 ` [PATCH v4 06/11] disas: Use qemu/bswap.h for bfd endian loads Richard Henderson
2020-09-21 19:33   ` Alex Bennée
2020-09-21 17:41 ` [PATCH v4 07/11] disas: Cleanup plugin_disas Richard Henderson
2020-09-22 10:38   ` Alex Bennée
2020-09-21 17:41 ` [PATCH v4 08/11] disas: Configure capstone for aarch64 host without libvixl Richard Henderson
2020-09-22 10:48   ` Alex Bennée
2020-09-21 17:41 ` [PATCH v4 09/11] disas: Split out capstone code to disas/capstone.c Richard Henderson
2020-09-22 10:50   ` Alex Bennée
2020-09-21 17:41 ` [PATCH v4 10/11] disas: Enable capstone disassembly for s390x Richard Henderson
2020-09-22 10:58   ` Alex Bennée
2020-09-21 17:41 ` [PATCH v4 11/11] disas/capstone: Add skipdata hook " Richard Henderson
2020-09-22 10:59   ` Alex Bennée
2020-09-22  0:50 ` [PATCH v4 00/11] capstone + disassembler patches no-reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87lfh3ndmo.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git
	git clone --mirror https://lore.kernel.org/qemu-devel/2 qemu-devel/git/2.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git