Re: [Qemu-devel] [RFC PATCH] capstone: fix building using system package

2018-02-15 Thread Philippe Mathieu-Daudé 
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.

real0m35.391s
user0m32.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  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 .
> 
> 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 
>^~~~
> 
> On Haiku `pkg-config --cflags capstone` reports "-I/usr/develop/headers".
> 
> Bug: https://bugs.gentoo.org/647570
> Reported-by: Zoltán Mizsei 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> 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 
> -
>  #else
> +#include 
> +#endif /* CONFIG_LIBCAPSTONE_INTERNAL */
> +
> +#else /* CONFIG_CAPSTONE */
>  
>  /* Just enough to allow backends to init without ifdefs.  */
>  
> 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC PATCH] capstone: fix building using system package

2018-02-15 Thread miqlas 

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é  wrote:


  #else
+#include 

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 
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

2018-02-15 Thread Philippe Mathieu-Daudé 
Hi Sergei,

On 02/15/2018 03:21 PM, Sergei Trofimovich wrote:
> On Thu, 15 Feb 2018 14:35:39 -0300
> Philippe Mathieu-Daudé  wrote:
> 
>>  #else
>> +#include 
> 
> 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 
> 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

2018-02-15 Thread Sergei Trofimovich 
On Thu, 15 Feb 2018 14:35:39 -0300
Philippe Mathieu-Daudé  wrote:

>  #else
> +#include 

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 
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

[Qemu-devel] [RFC PATCH] capstone: fix building using system package

2018-02-15 Thread Philippe Mathieu-Daudé 
The use of  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 .

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 
   ^~~~

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

Bug: https://bugs.gentoo.org/647570
Reported-by: Zoltán Mizsei 
Signed-off-by: Philippe Mathieu-Daudé 
---
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 
-
 #else
+#include 
+#endif /* CONFIG_LIBCAPSTONE_INTERNAL */
+
+#else /* CONFIG_CAPSTONE */
 
 /* Just enough to allow backends to init without ifdefs.  */
 
-- 
2.16.1