| Message ID | 20171017153742.10026-10-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Support the Capstone disassembler | expand |
Hi, > +if [ "$capstone_internal" = "yes" ]; then > + echo "config-host.h: subdir-capstone" >> $config_host_mak > +fi I think this isn't going to work correctly. In case both capstone and dtc are used we need a single line with the dependencies, i.e. config-host.h: subdir-dtc subdir-capstone cheers, Gerd
On 10/17/2017 10:48 PM, Gerd Hoffmann wrote: > Hi, > >> +if [ "$capstone_internal" = "yes" ]; then >> + echo "config-host.h: subdir-capstone" >> $config_host_mak >> +fi > > I think this isn't going to work correctly. In case both capstone and > dtc are used we need a single line with the dependencies, i.e. > > config-host.h: subdir-dtc subdir-capstone Why? Without a build rule, I thought separate dependency lines accumulated. r~
On 10/18/2017 12:48 AM, Gerd Hoffmann wrote: > Hi, > >> +if [ "$capstone_internal" = "yes" ]; then >> + echo "config-host.h: subdir-capstone" >> $config_host_mak >> +fi > > I think this isn't going to work correctly. In case both capstone and > dtc are used we need a single line with the dependencies, i.e. > > config-host.h: subdir-dtc subdir-capstone Make allows the composition of dependencies, as in: a: b a: c It works as long as at most one a: line contains the rule for building a, and all the other lines just add dependencies. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Tue, Oct 17, 2017 at 08:37:42AM -0700, Richard Henderson wrote: > Do not require the submodule, but use it if present (in preference > even to a system copy). This will allow us to easily use capstone > in older systems for which a package is not available, and also > easily track bug fixes from upstream. I don't like the idea that we should blindly use the submodule even if the host OS has it installed. This means developers working on git will never be testing against the capstone that will actually be used when they deploy a release build on their distro. It also gives inconsistent behaviour wrt the way the dtc module is handled, where we always try to use the system version and only try the submodule if the system version is missing. Thus I think we really ought to deal with capstone in the same way as dtc. We could make the check a little more advanced so that it checks for the particular capstone feature QEMU wants. That way if someone does have an outdated capstone we'd still use the submodule. 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 :|
On 10/18/2017 06:34 AM, Daniel P. Berrange wrote: > Thus I think we really ought to deal with capstone in the same > way as dtc. We could make the check a little more advanced so that > it checks for the particular capstone feature QEMU wants. That way > if someone does have an outdated capstone we'd still use the submodule. There's nothing I'm aware of besides bug fixes in between e.g. the fedora26 version of capstone and the one from git. r~
diff --git a/Makefile b/Makefile index 90f91e54eb..ca5eb489b0 100644 --- a/Makefile +++ b/Makefile @@ -383,6 +383,19 @@ subdir-dtc: .git-submodule-status dtc/libfdt dtc/tests dtc/%: 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. +CAP_CFLAGS = $(subst -Werror,,$(CFLAGS) $(QEMU_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 + +subdir-capstone: .git-submodule-status + $(call quiet-command,$(MAKE) -C $(SRC_PATH)/capstone CAPSTONE_SHARED=no BUILDDIR="$(BUILD_DIR)/capstone" CC="$(CC)" AR="$(AR)" LD="$(LD)" CFLAGS="$(CAP_CFLAGS)" $(SUBDIR_MAKEFLAGS) $(BUILD_DIR)/capstone/libcapstone.a) + $(SUBDIR_RULES): libqemuutil.a $(common-obj-y) $(chardev-obj-y) \ $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY)) diff --git a/.gitmodules b/.gitmodules index 7c981a42b6..1500579638 100644 --- a/.gitmodules +++ b/.gitmodules @@ -37,3 +37,6 @@ [submodule "ui/keycodemapdb"] path = ui/keycodemapdb url = git://git.qemu.org/keycodemapdb.git +[submodule "capstone"] + path = capstone + url = git://git.qemu.org/capstone.git diff --git a/capstone b/capstone new file mode 160000 index 0000000000..a279481dbf --- /dev/null +++ b/capstone @@ -0,0 +1 @@ +Subproject commit a279481dbfd54bb1e2336d771e89978cc6d43176 diff --git a/configure b/configure index 11e3078936..5a305c0a69 100755 --- a/configure +++ b/configure @@ -4412,7 +4412,15 @@ fi # capstone if test "$capstone" != no; then - if $pkg_config capstone; then + # have GIT checkout, so activate capstone submodule + if test -e "${source_path}/.git" ; then + git_submodules="${git_submodules} capstone" + capstone=yes + capstone_internal=yes + mkdir -p capstone + QEMU_CFLAGS="$QEMU_CFLAGS -I\$(SRC_PATH)/capstone/include" + LIBS="\$(BUILD_DIR)/capstone/libcapstone.a $LIBS" + elif $pkg_config capstone; then capstone=yes QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)" LIBS="$($pkg_config --libs capstone) $LIBS" @@ -6642,6 +6650,9 @@ done # for target in $targets if [ "$dtc_internal" = "yes" ]; then echo "config-host.h: subdir-dtc" >> $config_host_mak fi +if [ "$capstone_internal" = "yes" ]; then + echo "config-host.h: subdir-capstone" >> $config_host_mak +fi if test "$numa" = "yes"; then echo "CONFIG_NUMA=y" >> $config_host_mak
Do not require the submodule, but use it if present (in preference even to a system copy). This will allow us to easily use capstone in older systems for which a package is not available, and also easily track bug fixes from upstream. Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- Makefile | 13 +++++++++++++ .gitmodules | 3 +++ capstone | 1 + configure | 13 ++++++++++++- 4 files changed, 29 insertions(+), 1 deletion(-) create mode 160000 capstone -- 2.13.6