[PATCH 2/3] capstone: Allow version 3.0.5 again

Thomas Huth posted 3 patches 3 years, 8 months ago
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Thomas Huth <thuth@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Beraldo Leal <bleal@redhat.com>, Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>, Kamil Rytarowski <kamil@netbsd.org>, Reinoud Zandijk <reinoud@netbsd.org>, Ryo ONODERA <ryoon@netbsd.org>, Brad Smith <brad@comstyle.com>
[PATCH 2/3] capstone: Allow version 3.0.5 again
Posted by Thomas Huth 3 years, 8 months ago
According to

 https://lore.kernel.org/qemu-devel/20200921174118.39352-1-richard.henderson@linaro.org/

there was an issue with Capstone 3 from Ubuntu 18. Now that we removed
support for Ubuntu 18.04, that issue should hopefully not bite us
anymore. Compiling with version 3.0.5 seems to work fine on other
systems, so let's allow that version again.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 meson.build                | 2 +-
 .gitlab-ci.d/buildtest.yml | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 9b20dcd143..63ea585702 100644
--- a/meson.build
+++ b/meson.build
@@ -2513,7 +2513,7 @@ capstone = not_found
 capstone_opt = get_option('capstone')
 if capstone_opt in ['enabled', 'auto', 'system']
   have_internal = fs.exists(meson.current_source_dir() / 'capstone/Makefile')
-  capstone = dependency('capstone', version: '>=4.0',
+  capstone = dependency('capstone', version: '>=3.0.5',
                         kwargs: static_kwargs, method: 'pkg-config',
                         required: capstone_opt == 'system' or
                                   capstone_opt == 'enabled' and not have_internal)
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 0aea7ab84c..a4d43d743b 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -42,6 +42,7 @@ build-system-ubuntu:
   variables:
     IMAGE: ubuntu2004
     CONFIGURE_ARGS: --enable-docs --enable-fdt=system --enable-slirp=system
+        --enable-capstone=system
     TARGETS: aarch64-softmmu alpha-softmmu cris-softmmu hppa-softmmu
       microblazeel-softmmu mips64el-softmmu
     MAKE_CHECK_ARGS: check-build
-- 
2.27.0
Re: [PATCH 2/3] capstone: Allow version 3.0.5 again
Posted by Richard Henderson 3 years, 8 months ago
On 5/16/22 07:58, Thomas Huth wrote:
> According to
> 
>   https://lore.kernel.org/qemu-devel/20200921174118.39352-1-richard.henderson@linaro.org/
> 
> there was an issue with Capstone 3 from Ubuntu 18. Now that we removed
> support for Ubuntu 18.04, that issue should hopefully not bite us
> anymore. Compiling with version 3.0.5 seems to work fine on other
> systems, so let's allow that version again.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Please update this description with the 3.0.4 version number you found for 18.04.

Anyway,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
Re: [PATCH 2/3] capstone: Allow version 3.0.5 again
Posted by Peter Maydell 3 years, 8 months ago
On Mon, 16 May 2022 at 16:43, Thomas Huth <thuth@redhat.com> wrote:
>
> According to
>
>  https://lore.kernel.org/qemu-devel/20200921174118.39352-1-richard.henderson@linaro.org/
>
> there was an issue with Capstone 3 from Ubuntu 18. Now that we removed
> support for Ubuntu 18.04, that issue should hopefully not bite us
> anymore. Compiling with version 3.0.5 seems to work fine on other
> systems, so let's allow that version again.

Commit bcf368626cb33c4d says the reason for requiring capstone
>=4.0 was "We're about to use a portion of the 4.0 API", not
"Ubuntu's specific capstone 3 is broken"...

-- PMM
Re: [PATCH 2/3] capstone: Allow version 3.0.5 again
Posted by Richard Henderson 3 years, 8 months ago
On 5/16/22 08:46, Peter Maydell wrote:
> On Mon, 16 May 2022 at 16:43, Thomas Huth <thuth@redhat.com> wrote:
>>
>> According to
>>
>>   https://lore.kernel.org/qemu-devel/20200921174118.39352-1-richard.henderson@linaro.org/
>>
>> there was an issue with Capstone 3 from Ubuntu 18. Now that we removed
>> support for Ubuntu 18.04, that issue should hopefully not bite us
>> anymore. Compiling with version 3.0.5 seems to work fine on other
>> systems, so let's allow that version again.
> 
> Commit bcf368626cb33c4d says the reason for requiring capstone
>> =4.0 was "We're about to use a portion of the 4.0 API", not
> "Ubuntu's specific capstone 3 is broken"...

Looks like the patch to which this referred was never merged -- CS_ARCH_RISCV.

I still have a branch with riscv support sitting in it, from Sep 2020. Sadly, I never 
posted that patch, nor said why I withheld it in the end. Perhaps the actual riscv support 
in capstone was poor at the time.

The 4.0 requirement patch itself was kept for Ubuntu 18's issue:

https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg07542.html

# Changes for v4:
#  * Require v4.0 from the system library.
#    Fixes an issue AJB found from v3.0.5 from ubuntu 18.


r~
Re: [PATCH 2/3] capstone: Allow version 3.0.5 again
Posted by Peter Maydell 3 years, 8 months ago
On Mon, 16 May 2022 at 17:47, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 5/16/22 08:46, Peter Maydell wrote:
> > On Mon, 16 May 2022 at 16:43, Thomas Huth <thuth@redhat.com> wrote:
> >>
> >> According to
> >>
> >>   https://lore.kernel.org/qemu-devel/20200921174118.39352-1-richard.henderson@linaro.org/
> >>
> >> there was an issue with Capstone 3 from Ubuntu 18. Now that we removed
> >> support for Ubuntu 18.04, that issue should hopefully not bite us
> >> anymore. Compiling with version 3.0.5 seems to work fine on other
> >> systems, so let's allow that version again.
> >
> > Commit bcf368626cb33c4d says the reason for requiring capstone
> >> =4.0 was "We're about to use a portion of the 4.0 API", not
> > "Ubuntu's specific capstone 3 is broken"...
>
> Looks like the patch to which this referred was never merged -- CS_ARCH_RISCV.
>
> I still have a branch with riscv support sitting in it, from Sep 2020. Sadly, I never
> posted that patch, nor said why I withheld it in the end. Perhaps the actual riscv support
> in capstone was poor at the time.
>
> The 4.0 requirement patch itself was kept for Ubuntu 18's issue:
>
> https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg07542.html

Is that this one?
https://lore.kernel.org/qemu-devel/87wo0no0wz.fsf@linaro.org/

Did we find out why Ubuntu's capstone in particular fell over ?

thanks
-- PMM
Re: [PATCH 2/3] capstone: Allow version 3.0.5 again
Posted by Richard Henderson 3 years, 8 months ago
On 5/16/22 09:53, Peter Maydell wrote:
>> https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg07542.html
> 
> Is that this one?
> https://lore.kernel.org/qemu-devel/87wo0no0wz.fsf@linaro.org/

Could well be.

> 
> Did we find out why Ubuntu's capstone in particular fell over ?

I vaguely recall that it was a snapshot of a capstone prior to the 4.0 release.  The error 
message you quote above is because CAPSTONE_API is not defined to something reasonable.  I 
don't have an ubuntu 18 system to quickly look at.


r~
Re: [PATCH 2/3] capstone: Allow version 3.0.5 again
Posted by Thomas Huth 3 years, 8 months ago
On 16/05/2022 21.14, Richard Henderson wrote:
> On 5/16/22 09:53, Peter Maydell wrote:
>>> https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg07542.html
>>
>> Is that this one?
>> https://lore.kernel.org/qemu-devel/87wo0no0wz.fsf@linaro.org/
> 
> Could well be.
> 
>>
>> Did we find out why Ubuntu's capstone in particular fell over ?
> 
> I vaguely recall that it was a snapshot of a capstone prior to the 4.0 
> release.  The error message you quote above is because CAPSTONE_API is not 
> defined to something reasonable.  I don't have an ubuntu 18 system to 
> quickly look at.

I just had a try with our Ubuntu 18.04 docker container (as it has not been
removed yet) and my patches applied:

$ make docker-test-build@ubuntu1804 \
   EXTRA_CONFIGURE_OPTS=--enable-capstone
   ...
   Dependencies
     ...
     capstone                     : YES 3.0.4
     ...
[1023/3301] Compiling C object libcommon.fa.p/disas_capstone.c.o
FAILED: libcommon.fa.p/disas_capstone.c.o
cc -m64 -mcx16 -Ilibcommon.fa.p -I../src/common-user/host/x86_64 -I../src/dtc/libfdt -I../src/slirp -I../src/slirp/src -I/usr/include/capstone -I/usr/include/pixman-1 -I/usr/include/libpng16 -I/usr/include/spice-server -I/usr/include/spice-1 -I/usr/include/p11-kit-1 -I/usr/include/libusb-1.0 -I/usr/include/SDL2 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/gio-unix-2.0/ -I/usr/include/alsa -I/usr/include/virgl -I/usr/include/ncursesw -I/usr/include/gtk-3.0 -I/usr/include/pango-1.0 -I/usr/include/cairo -I/usr/include/freetype2 -I/usr/include/harfbuzz -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/atk-1.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/at-spi-2.0 -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I/usr/include/vte-2.91 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /tmp/qemu-test/src/linux-headers -isystem linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/include -iquote /tmp/qemu-test/src/disas/libvixl -iquote /tmp/qemu-test/src/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -D_GNU_SOURCE -D_DEFAULT_SOURCE -DNCURSES_WIDECHAR=1 -D_REENTRANT -Wno-undef -DSTRUCT_IOVEC_DEFINED -MD -MQ libcommon.fa.p/disas_capstone.c.o -MF libcommon.fa.p/disas_capstone.c.o.d -o libcommon.fa.p/disas_capstone.c.o -c ../src/disas/capstone.c
../src/disas/capstone.c:25:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘cap_skipdata_s390x_cb’
  cap_skipdata_s390x_cb(const uint8_t *code, size_t code_size,
  ^~~~~~~~~~~~~~~~~~~~~
../src/disas/capstone.c:49:17: error: ‘cap_skipdata_s390x_cb’ undeclared here (not in a function); did you mean ‘cap_skipdata_s390x’?
      .callback = cap_skipdata_s390x_cb
                  ^~~~~~~~~~~~~~~~~~~~~
                  cap_skipdata_s390x
ninja: build stopped: subcommand failed.
Makefile:163: recipe for target 'run-ninja' failed

So it seems like really only the capstone 3.0.4 from Ubuntu 18.04 is broken,
while this compiles fine with the capstone 3.0.5 from Ubuntu 20.04.

I think my patches should be ok to apply now that we dropped support
for Ubuntu 18.04.

  Thomas
Re: [PATCH 2/3] capstone: Allow version 3.0.5 again
Posted by Richard Henderson 3 years, 8 months ago
On 5/16/22 12:22, Thomas Huth wrote:
> So it seems like really only the capstone 3.0.4 from Ubuntu 18.04 is broken,
> while this compiles fine with the capstone 3.0.5 from Ubuntu 20.04.
> 
> I think my patches should be ok to apply now that we dropped support
> for Ubuntu 18.04.

Yes, I think so. Especially with the >=3.0.5 test.


r~