| Message ID | 20180215173539.11033-1-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] capstone: fix building using system package | expand |
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?
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.
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.
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. */ > >
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. */
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(-)