[v1] Update capstone module

[Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

Posted by Richard Henderson 6 years, 8 months ago

Since v4.0, capstone.h has moved to <capstone/capstone.h>.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/disas/capstone.h | 4 ++++
 configure                | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/include/disas/capstone.h b/include/disas/capstone.h
index e29068dd97..90631d84a9 100644
--- a/include/disas/capstone.h
+++ b/include/disas/capstone.h
@@ -3,7 +3,11 @@
 
 #ifdef CONFIG_CAPSTONE
 
+#ifdef CONFIG_CAPSTONE_CAPSTONE_H
+#include <capstone/capstone.h>
+#else
 #include <capstone.h>
+#endif
 
 #else
 
diff --git a/configure b/configure
index d2fc346302..eec7f061c3 100755
--- a/configure
+++ b/configure
@@ -5021,6 +5021,9 @@ case "$capstone" in
   system)
     QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
     LIBS="$($pkg_config --libs capstone) $LIBS"
+    if check_include capstone/capstone.h; then
+      capstone_capstone_h=yes
+    fi
     ;;
 
   no)
@@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
 fi
 if test "$capstone" != "no" ; then
   echo "CONFIG_CAPSTONE=y" >> $config_host_mak
+  if test "$capstone_capstone_h" != "no" ; then
+    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
+  fi
 fi
 if test "$debug_mutex" = "yes" ; then
   echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak
-- 
2.17.1

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

Posted by Alex Bennée 6 years, 8 months ago

Richard Henderson <richard.henderson@linaro.org> writes:

> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

> ---
>  include/disas/capstone.h | 4 ++++
>  configure                | 6 ++++++
>  2 files changed, 10 insertions(+)
>
> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> index e29068dd97..90631d84a9 100644
> --- a/include/disas/capstone.h
> +++ b/include/disas/capstone.h
> @@ -3,7 +3,11 @@
>
>  #ifdef CONFIG_CAPSTONE
>
> +#ifdef CONFIG_CAPSTONE_CAPSTONE_H
> +#include <capstone/capstone.h>
> +#else
>  #include <capstone.h>
> +#endif
>
>  #else
>
> diff --git a/configure b/configure
> index d2fc346302..eec7f061c3 100755
> --- a/configure
> +++ b/configure
> @@ -5021,6 +5021,9 @@ case "$capstone" in
>    system)
>      QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
>      LIBS="$($pkg_config --libs capstone) $LIBS"
> +    if check_include capstone/capstone.h; then
> +      capstone_capstone_h=yes
> +    fi
>      ;;
>
>    no)
> @@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
>  fi
>  if test "$capstone" != "no" ; then
>    echo "CONFIG_CAPSTONE=y" >> $config_host_mak
> +  if test "$capstone_capstone_h" != "no" ; then
> +    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
> +  fi
>  fi
>  if test "$debug_mutex" = "yes" ; then
>    echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak


--
Alex Bennée

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

Posted by Philippe Mathieu-Daudé 6 years, 8 months ago

On 5/23/19 4:42 AM, Richard Henderson wrote:
> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/disas/capstone.h | 4 ++++
>  configure                | 6 ++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> index e29068dd97..90631d84a9 100644
> --- a/include/disas/capstone.h
> +++ b/include/disas/capstone.h
> @@ -3,7 +3,11 @@
>  
>  #ifdef CONFIG_CAPSTONE
>  
> +#ifdef CONFIG_CAPSTONE_CAPSTONE_H
> +#include <capstone/capstone.h>
> +#else
>  #include <capstone.h>
> +#endif
>  
>  #else
>  
> diff --git a/configure b/configure
> index d2fc346302..eec7f061c3 100755
> --- a/configure
> +++ b/configure
> @@ -5021,6 +5021,9 @@ case "$capstone" in
>    system)
>      QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
>      LIBS="$($pkg_config --libs capstone) $LIBS"
> +    if check_include capstone/capstone.h; then
> +      capstone_capstone_h=yes
> +    fi
>      ;;
>  
>    no)
> @@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
>  fi
>  if test "$capstone" != "no" ; then
>    echo "CONFIG_CAPSTONE=y" >> $config_host_mak
> +  if test "$capstone_capstone_h" != "no" ; then
> +    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
> +  fi
>  fi
>  if test "$debug_mutex" = "yes" ; then
>    echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

Posted by Daniel P. Berrangé 6 years, 8 months ago

On Wed, May 22, 2019 at 10:42:27PM -0400, Richard Henderson wrote:
> Since v4.0, capstone.h has moved to <capstone/capstone.h>.

NB this was a regression bug in capstone pkg-config file which has been
fixed upstream

   https://github.com/aquynh/capstone/pull/1276

In Fedora we pulled in the fix to our v4.0 builds and I'd suggest
other distros should do the same

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/disas/capstone.h | 4 ++++
>  configure                | 6 ++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> index e29068dd97..90631d84a9 100644
> --- a/include/disas/capstone.h
> +++ b/include/disas/capstone.h
> @@ -3,7 +3,11 @@
>  
>  #ifdef CONFIG_CAPSTONE
>  
> +#ifdef CONFIG_CAPSTONE_CAPSTONE_H
> +#include <capstone/capstone.h>
> +#else
>  #include <capstone.h>
> +#endif
>  
>  #else
>  
> diff --git a/configure b/configure
> index d2fc346302..eec7f061c3 100755
> --- a/configure
> +++ b/configure
> @@ -5021,6 +5021,9 @@ case "$capstone" in
>    system)
>      QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
>      LIBS="$($pkg_config --libs capstone) $LIBS"
> +    if check_include capstone/capstone.h; then
> +      capstone_capstone_h=yes
> +    fi
>      ;;
>  
>    no)
> @@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
>  fi
>  if test "$capstone" != "no" ; then
>    echo "CONFIG_CAPSTONE=y" >> $config_host_mak
> +  if test "$capstone_capstone_h" != "no" ; then
> +    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
> +  fi
>  fi
>  if test "$debug_mutex" = "yes" ; then
>    echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak

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

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

Posted by Philippe Mathieu-Daudé 6 years, 8 months ago

On 5/23/19 1:07 PM, Daniel P. Berrangé wrote:
> On Wed, May 22, 2019 at 10:42:27PM -0400, Richard Henderson wrote:
>> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
> 
> NB this was a regression bug in capstone pkg-config file which has been
> fixed upstream
> 
>    https://github.com/aquynh/capstone/pull/1276
> 
> In Fedora we pulled in the fix to our v4.0 builds and I'd suggest
> other distros should do the same

Are you suggesting to not include this patch? It is less invasive to
distributions that package QEMU but don't package libcapstone.

> 
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  include/disas/capstone.h | 4 ++++
>>  configure                | 6 ++++++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
>> index e29068dd97..90631d84a9 100644
>> --- a/include/disas/capstone.h
>> +++ b/include/disas/capstone.h
>> @@ -3,7 +3,11 @@
>>  
>>  #ifdef CONFIG_CAPSTONE
>>  
>> +#ifdef CONFIG_CAPSTONE_CAPSTONE_H
>> +#include <capstone/capstone.h>
>> +#else
>>  #include <capstone.h>
>> +#endif
>>  
>>  #else
>>  
>> diff --git a/configure b/configure
>> index d2fc346302..eec7f061c3 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5021,6 +5021,9 @@ case "$capstone" in
>>    system)
>>      QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
>>      LIBS="$($pkg_config --libs capstone) $LIBS"
>> +    if check_include capstone/capstone.h; then
>> +      capstone_capstone_h=yes
>> +    fi
>>      ;;
>>  
>>    no)
>> @@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
>>  fi
>>  if test "$capstone" != "no" ; then
>>    echo "CONFIG_CAPSTONE=y" >> $config_host_mak
>> +  if test "$capstone_capstone_h" != "no" ; then
>> +    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
>> +  fi
>>  fi
>>  if test "$debug_mutex" = "yes" ; then
>>    echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak
> 
> Regards,
> Daniel
>

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

Posted by Daniel P. Berrangé 6 years, 8 months ago

On Thu, May 23, 2019 at 01:17:40PM +0200, Philippe Mathieu-Daudé wrote:
> On 5/23/19 1:07 PM, Daniel P. Berrangé wrote:
> > On Wed, May 22, 2019 at 10:42:27PM -0400, Richard Henderson wrote:
> >> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
> > 
> > NB this was a regression bug in capstone pkg-config file which has been
> > fixed upstream
> > 
> >    https://github.com/aquynh/capstone/pull/1276
> > 
> > In Fedora we pulled in the fix to our v4.0 builds and I'd suggest
> > other distros should do the same
> 
> Are you suggesting to not include this patch? It is less invasive to
> distributions that package QEMU but don't package libcapstone.

If building against a bundled capstone, we ought to be able to get
the right -I flag to not need this.  So its just a question of how
how much we care about distros who've not fixed the regression.

> 
> > 
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >>  include/disas/capstone.h | 4 ++++
> >>  configure                | 6 ++++++
> >>  2 files changed, 10 insertions(+)
> >>
> >> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> >> index e29068dd97..90631d84a9 100644
> >> --- a/include/disas/capstone.h
> >> +++ b/include/disas/capstone.h
> >> @@ -3,7 +3,11 @@
> >>  
> >>  #ifdef CONFIG_CAPSTONE
> >>  
> >> +#ifdef CONFIG_CAPSTONE_CAPSTONE_H
> >> +#include <capstone/capstone.h>
> >> +#else
> >>  #include <capstone.h>
> >> +#endif
> >>  
> >>  #else
> >>  
> >> diff --git a/configure b/configure
> >> index d2fc346302..eec7f061c3 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -5021,6 +5021,9 @@ case "$capstone" in
> >>    system)
> >>      QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
> >>      LIBS="$($pkg_config --libs capstone) $LIBS"
> >> +    if check_include capstone/capstone.h; then
> >> +      capstone_capstone_h=yes
> >> +    fi
> >>      ;;
> >>  
> >>    no)
> >> @@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
> >>  fi
> >>  if test "$capstone" != "no" ; then
> >>    echo "CONFIG_CAPSTONE=y" >> $config_host_mak
> >> +  if test "$capstone_capstone_h" != "no" ; then
> >> +    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
> >> +  fi
> >>  fi
> >>  if test "$debug_mutex" = "yes" ; then
> >>    echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak
> > 
> > Regards,
> > Daniel
> > 
> 

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

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

Posted by Alex Bennée 6 years, 8 months ago

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Wed, May 22, 2019 at 10:42:27PM -0400, Richard Henderson wrote:
>> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
>
> NB this was a regression bug in capstone pkg-config file which has been
> fixed upstream
>
>    https://github.com/aquynh/capstone/pull/1276
>
> In Fedora we pulled in the fix to our v4.0 builds and I'd suggest
> other distros should do the same

But I think it's worth keeping the workaround in the likely event that
it takes a while for that to filter through.

>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  include/disas/capstone.h | 4 ++++
>>  configure                | 6 ++++++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
>> index e29068dd97..90631d84a9 100644
>> --- a/include/disas/capstone.h
>> +++ b/include/disas/capstone.h
>> @@ -3,7 +3,11 @@
>>
>>  #ifdef CONFIG_CAPSTONE
>>
>> +#ifdef CONFIG_CAPSTONE_CAPSTONE_H
>> +#include <capstone/capstone.h>
>> +#else
>>  #include <capstone.h>
>> +#endif
>>
>>  #else
>>
>> diff --git a/configure b/configure
>> index d2fc346302..eec7f061c3 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5021,6 +5021,9 @@ case "$capstone" in
>>    system)
>>      QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
>>      LIBS="$($pkg_config --libs capstone) $LIBS"
>> +    if check_include capstone/capstone.h; then
>> +      capstone_capstone_h=yes
>> +    fi
>>      ;;
>>
>>    no)
>> @@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
>>  fi
>>  if test "$capstone" != "no" ; then
>>    echo "CONFIG_CAPSTONE=y" >> $config_host_mak
>> +  if test "$capstone_capstone_h" != "no" ; then
>> +    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
>> +  fi
>>  fi
>>  if test "$debug_mutex" = "yes" ; then
>>    echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak
>
> Regards,
> Daniel


--
Alex Bennée

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

Posted by Richard Henderson 6 years, 8 months ago

On 5/23/19 7:07 AM, Daniel P. Berrangé wrote:
> On Wed, May 22, 2019 at 10:42:27PM -0400, Richard Henderson wrote:
>> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
> 
> NB this was a regression bug in capstone pkg-config file which has been
> fixed upstream
> 
>    https://github.com/aquynh/capstone/pull/1276
> 
> In Fedora we pulled in the fix to our v4.0 builds and I'd suggest
> other distros should do the same

Heh.  And here's me thinking this was an intentional change.


r~

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

Posted by Richard Henderson 6 years, 8 months ago

On 5/23/19 7:07 AM, Daniel P. Berrangé wrote:
> On Wed, May 22, 2019 at 10:42:27PM -0400, Richard Henderson wrote:
>> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
> NB this was a regression bug in capstone pkg-config file which has been
> fixed upstream
> 
>    https://github.com/aquynh/capstone/pull/1276
> 
> In Fedora we pulled in the fix to our v4.0 builds and I'd suggest
> other distros should do the same
> 

It seems this fix is present in the tagged 4.0 release.

This would have only been present if a distro packaged snapshots.
At least one may have done so, based on

https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00329.html

but there's no further information to go on.

I've checked Fedora 30 and Debian Buster, which are the only two I could
immediately identify that shipped 4.0.1, as opposed to some 3.x version.  Both
have the pkg-config bug fixed.

Since I cannot test any fixup path, I'm going to drop this patch entirely.

r~

Re: [Qemu-devel] [PATCH 1/3] capstone: Adjust include of capstone.h

Posted by David Hildenbrand 6 years, 8 months ago

On 23.05.19 04:42, Richard Henderson wrote:
> Since v4.0, capstone.h has moved to <capstone/capstone.h>.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/disas/capstone.h | 4 ++++
>  configure                | 6 ++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/include/disas/capstone.h b/include/disas/capstone.h
> index e29068dd97..90631d84a9 100644
> --- a/include/disas/capstone.h
> +++ b/include/disas/capstone.h
> @@ -3,7 +3,11 @@
>  
>  #ifdef CONFIG_CAPSTONE
>  
> +#ifdef CONFIG_CAPSTONE_CAPSTONE_H
> +#include <capstone/capstone.h>
> +#else
>  #include <capstone.h>
> +#endif
>  
>  #else
>  
> diff --git a/configure b/configure
> index d2fc346302..eec7f061c3 100755
> --- a/configure
> +++ b/configure
> @@ -5021,6 +5021,9 @@ case "$capstone" in
>    system)
>      QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags capstone)"
>      LIBS="$($pkg_config --libs capstone) $LIBS"
> +    if check_include capstone/capstone.h; then
> +      capstone_capstone_h=yes
> +    fi
>      ;;
>  
>    no)
> @@ -7197,6 +7200,9 @@ if test "$ivshmem" = "yes" ; then
>  fi
>  if test "$capstone" != "no" ; then
>    echo "CONFIG_CAPSTONE=y" >> $config_host_mak
> +  if test "$capstone_capstone_h" != "no" ; then
> +    echo "CONFIG_CAPSTONE_CAPSTONE_H=y" >> $config_host_mak
> +  fi
>  fi
>  if test "$debug_mutex" = "yes" ; then
>    echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb