1. Patchew
  2. QEMU
  3. View series

[RFC PATCH] capstone: fix building using system package

Philippe Mathieu-Daudé posted 1 patch 7 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180215173539.11033-1-f4bug@amsat.org
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test ppcbe passed
Test ppcle passed
Test s390x failed
configure                | 1 +
include/disas/capstone.h | 6 +++++-
2 files changed, 6 insertions(+), 1 deletion(-)
[RFC PATCH] capstone: fix building using system package
Posted by Philippe Mathieu-Daudé 7 years, 8 months ago 
The use of <capstone/capstone.h> is recommended by the upstream project:
  http://www.capstone-engine.org/lang_c.html
However when building the in-tree cloned submodule, the header is accessible
via <capstone.h>.

This fixes building on Gentoo (and Haiku OS - not supported since 898be3e0415):
    CC      disas.o
  /sources/qemu-2.11.0/include/disas/capstone.h:6:22: fatal error: capstone.h: No such file or directory
  #include <capstone.h>
           ^~~~~~~~~~~~

On Haiku `pkg-config --cflags capstone` reports "-I/usr/develop/headers".

Bug: https://bugs.gentoo.org/647570
Reported-by: Zoltán Mizsei <miqlas@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
RFC because this might be a Gentoo portage issue.

 configure                | 1 +
 include/disas/capstone.h | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 913e14839d..3657a61a35 100755
--- a/configure
+++ b/configure
@@ -7017,6 +7017,7 @@ if [ "$dtc_internal" = "yes" ]; then
   echo "config-host.h: subdir-dtc" >> $config_host_mak
 fi
 if [ "$capstone" = "git" -o "$capstone" = "internal" ]; then
+  echo "CONFIG_LIBCAPSTONE_INTERNAL=y" >> $config_host_mak
   echo "config-host.h: subdir-capstone" >> $config_host_mak
 fi
 if test -n "$LIBCAPSTONE"; then
diff --git a/include/disas/capstone.h b/include/disas/capstone.h
index 84e214956d..aea9601f41 100644
--- a/include/disas/capstone.h
+++ b/include/disas/capstone.h
@@ -3,9 +3,13 @@
 
 #ifdef CONFIG_CAPSTONE
 
+#ifdef CONFIG_LIBCAPSTONE_INTERNAL
 #include <capstone.h>
-
 #else
+#include <capstone/capstone.h>
+#endif /* CONFIG_LIBCAPSTONE_INTERNAL */
+
+#else /* CONFIG_CAPSTONE */
 
 /* Just enough to allow backends to init without ifdefs.  */
 
-- 
2.16.1
Re: [Qemu-devel] [RFC PATCH] capstone: fix building using system package
Posted by Sergei Trofimovich 7 years, 8 months ago 
On Thu, 15 Feb 2018 14:35:39 -0300
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

>  #else
> +#include <capstone/capstone.h>

I think it's incorrect. 'pkg-config' already reports 'capstone/' path:
    $ pkg-config --cflags capstone
    -I/usr/include/capstone

    $ ls /usr/include/capstone/capstone.h
    /usr/include/capstone/capstone.h

Thus I would guess
    #include <capstone.h>
is still correct for system include path as well (contradicts the example).

qemu just needs to use 'pkg-config' to discover the include path and
libs. Maybe new capstone release has different pkgconfig setup?

-- 

  Sergei
Re: [Qemu-devel] [RFC PATCH] capstone: fix building using system package
Posted by Philippe Mathieu-Daudé 7 years, 8 months ago 
Hi Sergei,

On 02/15/2018 03:21 PM, Sergei Trofimovich wrote:
> On Thu, 15 Feb 2018 14:35:39 -0300
> Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> 
>>  #else
>> +#include <capstone/capstone.h>
> 
> I think it's incorrect. 'pkg-config' already reports 'capstone/' path:
>     $ pkg-config --cflags capstone
>     -I/usr/include/capstone

Glad to hear feedback from a Gentoo developer!

Ok so the problem Haiku only, which we don't support anymore.

> 
>     $ ls /usr/include/capstone/capstone.h
>     /usr/include/capstone/capstone.h
> 
> Thus I would guess
>     #include <capstone.h>
> is still correct for system include path as well (contradicts the example).

My guess is the example is probabilisticly safer for people compiling
without using 'pkg-config'.

> qemu just needs to use 'pkg-config' to discover the include path and
> libs. Maybe new capstone release has different pkgconfig setup?

I think it is safer to drop this patch.

Thanks for your review!

Phil.
Re: [Qemu-devel] [RFC PATCH] capstone: fix building using system package
Posted by miqlas 7 years, 8 months ago 
Hi,

afaik (but not tested) pkgconfig --cflags reports /includes on linux, 
and it does the same on Haiku too.
I'm not against to change our capstone recipe, but please, if you can 
check it on Linux and report it back, as i don't want to break other 
software.
Thanks for the nice talk, guys!

--miqlas


2018-02-15 19:39 keltezéssel, Philippe Mathieu-Daudé írta:
> Hi Sergei,
>
> On 02/15/2018 03:21 PM, Sergei Trofimovich wrote:
>> On Thu, 15 Feb 2018 14:35:39 -0300
>> Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>>>   #else
>>> +#include <capstone/capstone.h>
>> I think it's incorrect. 'pkg-config' already reports 'capstone/' path:
>>      $ pkg-config --cflags capstone
>>      -I/usr/include/capstone
> Glad to hear feedback from a Gentoo developer!
>
> Ok so the problem Haiku only, which we don't support anymore.
>
>>      $ ls /usr/include/capstone/capstone.h
>>      /usr/include/capstone/capstone.h
>>
>> Thus I would guess
>>      #include <capstone.h>
>> is still correct for system include path as well (contradicts the example).
> My guess is the example is probabilisticly safer for people compiling
> without using 'pkg-config'.
>
>> qemu just needs to use 'pkg-config' to discover the include path and
>> libs. Maybe new capstone release has different pkgconfig setup?
> I think it is safer to drop this patch.
>
> Thanks for your review!
>
> Phil.
Re: [Qemu-devel] [RFC PATCH] capstone: fix building using system package
Posted by Philippe Mathieu-Daudé 7 years, 8 months ago 
What is interesting with this patch, is that, forcing use of system
capstone, Travis builds ran much faster; longest build took 40min:
https://travis-ci.org/philmd/qemu/builds/341979248

This revealed (without profiling yet) that compiling the capstone C++
takes some time...

mingw32@i7-4600U# time make subdir-capstone
  CC      cs.o
  CC      utils.o
  CC      SStream.o
  CC      MCInstrDesc.o
  CC      MCRegisterInfo.o
  CC      arch/ARM/ARMDisassembler.o
  CC      arch/ARM/ARMInstPrinter.o
  CC      arch/ARM/ARMMapping.o
  CC      arch/ARM/ARMModule.o
  CC      arch/AArch64/AArch64BaseInfo.o
  CC      arch/AArch64/AArch64Disassembler.o
  CC      arch/AArch64/AArch64InstPrinter.o
  CC      arch/AArch64/AArch64Mapping.o
  CC      arch/AArch64/AArch64Module.o
  CC      arch/Mips/MipsDisassembler.o
  CC      arch/Mips/MipsInstPrinter.o
  CC      arch/Mips/MipsMapping.o
  CC      arch/Mips/MipsModule.o
  CC      arch/PowerPC/PPCDisassembler.o
  CC      arch/PowerPC/PPCInstPrinter.o
  CC      arch/PowerPC/PPCMapping.o
  CC      arch/PowerPC/PPCModule.o
  CC      arch/Sparc/SparcDisassembler.o
  CC      arch/Sparc/SparcInstPrinter.o
  CC      arch/Sparc/SparcMapping.o
  CC      arch/Sparc/SparcModule.o
  CC      arch/SystemZ/SystemZDisassembler.o
  CC      arch/SystemZ/SystemZInstPrinter.o
  CC      arch/SystemZ/SystemZMapping.o
  CC      arch/SystemZ/SystemZModule.o
  CC      arch/SystemZ/SystemZMCTargetDesc.o
  CC      arch/X86/X86DisassemblerDecoder.o
  CC      arch/X86/X86Disassembler.o
  CC      arch/X86/X86IntelInstPrinter.o
  CC      arch/X86/X86ATTInstPrinter.o
  CC      arch/X86/X86Mapping.o
  CC      arch/X86/X86Module.o
  CC      arch/XCore/XCoreDisassembler.o
  CC      arch/XCore/XCoreInstPrinter.o
  CC      arch/XCore/XCoreMapping.o
  CC      arch/XCore/XCoreModule.o
  CC      MCInst.o
  AR      capstone.lib
i686-w64-mingw32.shared-ar: creating capstone/capstone.lib
make: 'subdir-capstone' is up to date.

real	0m35.391s
user	0m32.857s
sys	0m2.414s

Not that bad after all.

So I still dunno why this patch improved build time...

On 02/15/2018 02:35 PM, Philippe Mathieu-Daudé wrote:
> The use of <capstone/capstone.h> is recommended by the upstream project:
>   http://www.capstone-engine.org/lang_c.html
> However when building the in-tree cloned submodule, the header is accessible
> via <capstone.h>.
> 
> This fixes building on Gentoo (and Haiku OS - not supported since 898be3e0415):
>     CC      disas.o
>   /sources/qemu-2.11.0/include/disas/capstone.h:6:22: fatal error: capstone.h: No such file or directory
>   #include <capstone.h>
>            ^~~~~~~~~~~~
> 
> On Haiku `pkg-config --cflags capstone` reports "-I/usr/develop/headers".
> 
> Bug: https://bugs.gentoo.org/647570
> Reported-by: Zoltán Mizsei <miqlas@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> RFC because this might be a Gentoo portage issue.
> 
>  configure                | 1 +
>  include/disas/capstone.h | 6 +++++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 913e14839d..3657a61a35 100755
> --- a/configure
> +++ b/configure
> @@ -7017,6 +7017,7 @@ if [ "$dtc_internal" = "yes" ]; then
>    echo "config-host.h: subdir-dtc" >> $config_host_mak
>  fi
>  if [ "$capstone" = "git" -o "$capstone" = "internal" ]; then
> +  echo "CONFIG_LIBCAPSTONE_INTERNAL=y" >> $config_host_mak
>    echo "config-host.h: subdir-capstone" >> $config_host_mak
>  fi
>  if test -n "$LIBCAPSTONE"; then
> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> index 84e214956d..aea9601f41 100644
> --- a/include/disas/capstone.h
> +++ b/include/disas/capstone.h
> @@ -3,9 +3,13 @@
>  
>  #ifdef CONFIG_CAPSTONE
>  
> +#ifdef CONFIG_LIBCAPSTONE_INTERNAL
>  #include <capstone.h>
> -
>  #else
> +#include <capstone/capstone.h>
> +#endif /* CONFIG_LIBCAPSTONE_INTERNAL */
> +
> +#else /* CONFIG_CAPSTONE */
>  
>  /* Just enough to allow backends to init without ifdefs.  */
>  
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.