* [PATCH v2 01/12] gitlab: enable ccache for many build jobs
2023-08-24 16:38 [PATCH v2 00/12] gdbstub and testing fixes for 8.2 Alex Bennée
@ 2023-08-24 16:38 ` Alex Bennée
2023-08-25 7:38 ` Philippe Mathieu-Daudé
2023-08-25 7:46 ` Michael Tokarev
2023-08-24 16:39 ` [PATCH v2 02/12] tests/docker: cleanup non-verbose output Alex Bennée
` (10 subsequent siblings)
11 siblings, 2 replies; 26+ messages in thread
From: Alex Bennée @ 2023-08-24 16:38 UTC (permalink / raw)
To: qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki,
Alex Bennée, qemu-arm, Juan Quintela, Thomas Huth,
Peter Maydell, Ilya Leoshkevich, Daniel P. Berrangé,
David Hildenbrand, Greg Kurz, Wainer dos Santos Moschetta,
Philippe Mathieu-Daudé, Cédric Le Goater
From: Daniel P. Berrangé <berrange@redhat.com>
The `ccache` tool can be very effective at reducing compilation times
when re-running pipelines with only minor changes each time. For example
a fresh 'build-system-fedora' job will typically take 20 minutes on the
gitlab.com shared runners. With ccache this is reduced to as little as
6 minutes.
Normally meson would auto-detect existance of ccache in $PATH and use
it automatically, but the way we wrap meson from configure breaks this,
as we're passing in an config file with explicitly set compiler paths.
Thus we need to add $CCACHE_WRAPPERSPATH to the front of $PATH. For
unknown reasons if doing this in msys though, gcc becomes unable to
invoke 'cc1' when run from meson. For msys we thus set CC='ccache gcc'
before invoking 'configure' instead.
A second problem with msys is that cache misses are incredibly
expensive, so enabling ccache massively slows down the build when
the cache isn't well populated. This is suspected to be a result of
the cost of spawning processes under the msys architecture. To deal
with this we set CCACHE_DEPEND=1 which enables ccache's 'depend_only'
strategy. This avoids extra spawning of the pre-processor during
cache misses, with the downside that is it less likely ccache will
find a cache hit after semantically benign compiler flag changes.
This is the lesser of two evils, as otherwise we can't use ccache
at all under msys and remain inside the job time limit.
If people are finding ccache to hurt their pipelines, it can be
disabled by setting the 'CCACHE_DISABLE=1' env variable against
their gitlab fork CI settings.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20230804111054.281802-2-berrange@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
docs/devel/ci-jobs.rst.inc | 7 +++++
.gitlab-ci.d/buildtest-template.yml | 11 ++++++++
.gitlab-ci.d/crossbuild-template.yml | 26 +++++++++++++++++++
.gitlab-ci.d/windows.yml | 13 ++++++++--
.../dockerfiles/debian-hexagon-cross.docker | 9 ++++++-
5 files changed, 63 insertions(+), 3 deletions(-)
diff --git a/docs/devel/ci-jobs.rst.inc b/docs/devel/ci-jobs.rst.inc
index 3f6802d51e..4c39cdb2d9 100644
--- a/docs/devel/ci-jobs.rst.inc
+++ b/docs/devel/ci-jobs.rst.inc
@@ -188,3 +188,10 @@ If you've got access to a CentOS Stream 8 x86_64 host that can be
used as a gitlab-CI runner, you can set this variable to enable the
tests that require this kind of host. The runner should be tagged with
both "centos_stream_8" and "x86_64".
+
+CCACHE_DISABLE
+~~~~~~~~~~~~~~
+The jobs are configured to use "ccache" by default since this typically
+reduces compilation time, at the cost of increased storage. If the
+use of "ccache" is suspected to be hurting the overall job execution
+time, setting the "CCACHE_DISABLE=1" env variable to disable it.
diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml
index f3e39b7eb1..4fbfeb6667 100644
--- a/.gitlab-ci.d/buildtest-template.yml
+++ b/.gitlab-ci.d/buildtest-template.yml
@@ -2,11 +2,21 @@
extends: .base_job_template
stage: build
image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:$QEMU_CI_CONTAINER_TAG
+ cache:
+ paths:
+ - ccache
+ key: "$CI_JOB_NAME"
+ when: always
before_script:
- JOBS=$(expr $(nproc) + 1)
script:
+ - export CCACHE_BASEDIR="$(pwd)"
+ - export CCACHE_DIR="$CCACHE_BASEDIR/ccache"
+ - export CCACHE_MAXSIZE="500M"
+ - export PATH="$CCACHE_WRAPPERSDIR:$PATH"
- mkdir build
- cd build
+ - ccache --zero-stats
- ../configure --enable-werror --disable-docs --enable-fdt=system
${TARGETS:+--target-list="$TARGETS"}
$CONFIGURE_ARGS ||
@@ -20,6 +30,7 @@
then
make -j"$JOBS" $MAKE_CHECK_ARGS ;
fi
+ - ccache --show-stats
# We jump some hoops in common_test_job_template to avoid
# rebuilding all the object files we skip in the artifacts
diff --git a/.gitlab-ci.d/crossbuild-template.yml b/.gitlab-ci.d/crossbuild-template.yml
index d97611053b..3e5f4d9cd8 100644
--- a/.gitlab-ci.d/crossbuild-template.yml
+++ b/.gitlab-ci.d/crossbuild-template.yml
@@ -2,10 +2,20 @@
extends: .base_job_template
stage: build
image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:$QEMU_CI_CONTAINER_TAG
+ cache:
+ paths:
+ - ccache
+ key: "$CI_JOB_NAME"
+ when: always
timeout: 80m
script:
+ - export CCACHE_BASEDIR="$(pwd)"
+ - export CCACHE_DIR="$CCACHE_BASEDIR/ccache"
+ - export CCACHE_MAXSIZE="500M"
+ - export PATH="$CCACHE_WRAPPERSDIR:$PATH"
- mkdir build
- cd build
+ - ccache --zero-stats
- ../configure --enable-werror --disable-docs --enable-fdt=system
--disable-user $QEMU_CONFIGURE_OPTS $EXTRA_CONFIGURE_OPTS
--target-list-exclude="arm-softmmu cris-softmmu
@@ -18,6 +28,7 @@
version="$(git describe --match v[0-9]* 2>/dev/null || git rev-parse --short HEAD)";
mv -v qemu-setup*.exe qemu-setup-${version}.exe;
fi
+ - ccache --show-stats
# Job to cross-build specific accelerators.
#
@@ -29,7 +40,15 @@
stage: build
image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:$QEMU_CI_CONTAINER_TAG
timeout: 30m
+ cache:
+ paths:
+ - ccache/
+ key: "$CI_JOB_NAME"
script:
+ - export CCACHE_BASEDIR="$(pwd)"
+ - export CCACHE_DIR="$CCACHE_BASEDIR/ccache"
+ - export CCACHE_MAXSIZE="500M"
+ - export PATH="$CCACHE_WRAPPERSDIR:$PATH"
- mkdir build
- cd build
- ../configure --enable-werror --disable-docs $QEMU_CONFIGURE_OPTS
@@ -40,7 +59,14 @@
extends: .base_job_template
stage: build
image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:$QEMU_CI_CONTAINER_TAG
+ cache:
+ paths:
+ - ccache/
+ key: "$CI_JOB_NAME"
script:
+ - export CCACHE_BASEDIR="$(pwd)"
+ - export CCACHE_DIR="$CCACHE_BASEDIR/ccache"
+ - export CCACHE_MAXSIZE="500M"
- mkdir build
- cd build
- ../configure --enable-werror --disable-docs $QEMU_CONFIGURE_OPTS
diff --git a/.gitlab-ci.d/windows.yml b/.gitlab-ci.d/windows.yml
index cd7622a761..12a987cd71 100644
--- a/.gitlab-ci.d/windows.yml
+++ b/.gitlab-ci.d/windows.yml
@@ -5,13 +5,14 @@
- windows
- windows-1809
cache:
- key: "${CI_JOB_NAME}-cache"
+ key: "$CI_JOB_NAME"
paths:
- msys64/var/cache
+ - ccache
when: always
needs: []
stage: build
- timeout: 80m
+ timeout: 100m
variables:
# This feature doesn't (currently) work with PowerShell, it stops
# the echo'ing of commands being run and doesn't show any timing
@@ -72,6 +73,7 @@
bison diffutils flex
git grep make sed
$MINGW_TARGET-capstone
+ $MINGW_TARGET-ccache
$MINGW_TARGET-curl
$MINGW_TARGET-cyrus-sasl
$MINGW_TARGET-dtc
@@ -101,11 +103,18 @@
- Write-Output "Running build at $(Get-Date -Format u)"
- $env:CHERE_INVOKING = 'yes' # Preserve the current working directory
- $env:MSYS = 'winsymlinks:native' # Enable native Windows symlink
+ - $env:CCACHE_BASEDIR = "$env:CI_PROJECT_DIR"
+ - $env:CCACHE_DIR = "$env:CCACHE_BASEDIR/ccache"
+ - $env:CCACHE_MAXSIZE = "500M"
+ - $env:CCACHE_DEPEND = 1 # cache misses are too expensive with preprocessor mode
+ - $env:CC = "ccache gcc"
- mkdir build
- cd build
+ - ..\msys64\usr\bin\bash -lc "ccache --zero-stats"
- ..\msys64\usr\bin\bash -lc "../configure --enable-fdt=system $CONFIGURE_ARGS"
- ..\msys64\usr\bin\bash -lc "make"
- ..\msys64\usr\bin\bash -lc "make check MTESTARGS='$TEST_ARGS' || { cat meson-logs/testlog.txt; exit 1; } ;"
+ - ..\msys64\usr\bin\bash -lc "ccache --show-stats"
- Write-Output "Finished build at $(Get-Date -Format u)"
msys2-64bit:
diff --git a/tests/docker/dockerfiles/debian-hexagon-cross.docker b/tests/docker/dockerfiles/debian-hexagon-cross.docker
index c2cfb6a5d0..578269766c 100644
--- a/tests/docker/dockerfiles/debian-hexagon-cross.docker
+++ b/tests/docker/dockerfiles/debian-hexagon-cross.docker
@@ -15,6 +15,7 @@ RUN apt-get update && \
# Install common build utilities
apt-get install -y --no-install-recommends \
curl \
+ ccache \
xz-utils \
ca-certificates \
bison \
@@ -24,13 +25,19 @@ RUN apt-get update && \
python3-venv && \
# Install QEMU build deps for use in CI
DEBIAN_FRONTEND=noninteractive eatmydata \
- apt build-dep -yy --arch-only qemu
+ apt build-dep -yy --arch-only qemu && \
+ mkdir -p /usr/libexec/ccache-wrappers && \
+ ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/c++ && \
+ ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/cc && \
+ ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/g++ && \
+ ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/gcc
ENV TOOLCHAIN_INSTALL /opt
ENV TOOLCHAIN_RELEASE 16.0.0
ENV TOOLCHAIN_BASENAME "clang+llvm-${TOOLCHAIN_RELEASE}-cross-hexagon-unknown-linux-musl"
ENV TOOLCHAIN_URL https://codelinaro.jfrog.io/artifactory/codelinaro-toolchain-for-hexagon/v${TOOLCHAIN_RELEASE}/${TOOLCHAIN_BASENAME}.tar.xz
+ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers"
RUN curl -#SL "$TOOLCHAIN_URL" | tar -xJC "$TOOLCHAIN_INSTALL"
ENV PATH $PATH:${TOOLCHAIN_INSTALL}/${TOOLCHAIN_BASENAME}/x86_64-linux-gnu/bin
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 01/12] gitlab: enable ccache for many build jobs
2023-08-24 16:38 ` [PATCH v2 01/12] gitlab: enable ccache for many build jobs Alex Bennée
@ 2023-08-25 7:38 ` Philippe Mathieu-Daudé
2023-08-25 7:46 ` Michael Tokarev
1 sibling, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-08-25 7:38 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki, qemu-arm,
Juan Quintela, Thomas Huth, Peter Maydell, Ilya Leoshkevich,
Daniel P. Berrangé, David Hildenbrand, Greg Kurz,
Wainer dos Santos Moschetta, Cédric Le Goater
On 24/8/23 18:38, Alex Bennée wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
>
> The `ccache` tool can be very effective at reducing compilation times
> when re-running pipelines with only minor changes each time. For example
> a fresh 'build-system-fedora' job will typically take 20 minutes on the
> gitlab.com shared runners. With ccache this is reduced to as little as
> 6 minutes.
>
> Normally meson would auto-detect existance of ccache in $PATH and use
> it automatically, but the way we wrap meson from configure breaks this,
> as we're passing in an config file with explicitly set compiler paths.
> Thus we need to add $CCACHE_WRAPPERSPATH to the front of $PATH. For
> unknown reasons if doing this in msys though, gcc becomes unable to
> invoke 'cc1' when run from meson. For msys we thus set CC='ccache gcc'
> before invoking 'configure' instead.
>
> A second problem with msys is that cache misses are incredibly
> expensive, so enabling ccache massively slows down the build when
> the cache isn't well populated. This is suspected to be a result of
> the cost of spawning processes under the msys architecture. To deal
> with this we set CCACHE_DEPEND=1 which enables ccache's 'depend_only'
> strategy. This avoids extra spawning of the pre-processor during
> cache misses, with the downside that is it less likely ccache will
> find a cache hit after semantically benign compiler flag changes.
> This is the lesser of two evils, as otherwise we can't use ccache
> at all under msys and remain inside the job time limit.
>
> If people are finding ccache to hurt their pipelines, it can be
> disabled by setting the 'CCACHE_DISABLE=1' env variable against
> their gitlab fork CI settings.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> Message-Id: <20230804111054.281802-2-berrange@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> docs/devel/ci-jobs.rst.inc | 7 +++++
> .gitlab-ci.d/buildtest-template.yml | 11 ++++++++
> .gitlab-ci.d/crossbuild-template.yml | 26 +++++++++++++++++++
> .gitlab-ci.d/windows.yml | 13 ++++++++--
> .../dockerfiles/debian-hexagon-cross.docker | 9 ++++++-
> 5 files changed, 63 insertions(+), 3 deletions(-)
Very nice, thanks!
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 01/12] gitlab: enable ccache for many build jobs
2023-08-24 16:38 ` [PATCH v2 01/12] gitlab: enable ccache for many build jobs Alex Bennée
2023-08-25 7:38 ` Philippe Mathieu-Daudé
@ 2023-08-25 7:46 ` Michael Tokarev
2023-08-25 8:11 ` Daniel P. Berrangé
2023-08-25 8:34 ` Philippe Mathieu-Daudé
1 sibling, 2 replies; 26+ messages in thread
From: Michael Tokarev @ 2023-08-25 7:46 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki, qemu-arm,
Juan Quintela, Thomas Huth, Peter Maydell, Ilya Leoshkevich,
Daniel P. Berrangé, David Hildenbrand, Greg Kurz,
Wainer dos Santos Moschetta, Philippe Mathieu-Daudé,
Cédric Le Goater
24.08.2023 19:38, Alex Bennée wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
>
> The `ccache` tool can be very effective at reducing compilation times
> when re-running pipelines with only minor changes each time. For example
> a fresh 'build-system-fedora' job will typically take 20 minutes on the
> gitlab.com shared runners. With ccache this is reduced to as little as
> 6 minutes.
I've been using ccache when building qemu in debian, for quite a while.
The problem here, in the way qemu build system works, is that the cache
is hugely dependent on the path to the source. You change just one char
in there (/build/qemu/v8.1.0/ => /build/qemu/v8.1.1) and whole cache becomes
unusable, it all gets compiled anew. This is because qemu build sys uses
absolute file names when building, and this is detected by ccache, so
the source dir gets mixed into the hash together with gcc version and
other things.
Dunno how well this will work in the qemu ci though.
/mjt
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 01/12] gitlab: enable ccache for many build jobs
2023-08-25 7:46 ` Michael Tokarev
@ 2023-08-25 8:11 ` Daniel P. Berrangé
2023-08-25 8:34 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 26+ messages in thread
From: Daniel P. Berrangé @ 2023-08-25 8:11 UTC (permalink / raw)
To: Michael Tokarev
Cc: Alex Bennée, qemu-devel, Beraldo Leal, Nicholas Piggin,
Richard Henderson, Markus Armbruster, Yonggang Luo, qemu-ppc,
David Gibson, Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki,
qemu-arm, Juan Quintela, Thomas Huth, Peter Maydell,
Ilya Leoshkevich, David Hildenbrand, Greg Kurz,
Wainer dos Santos Moschetta, Philippe Mathieu-Daudé,
Cédric Le Goater
On Fri, Aug 25, 2023 at 10:46:29AM +0300, Michael Tokarev wrote:
> 24.08.2023 19:38, Alex Bennée wrote:
> > From: Daniel P. Berrangé <berrange@redhat.com>
> >
> > The `ccache` tool can be very effective at reducing compilation times
> > when re-running pipelines with only minor changes each time. For example
> > a fresh 'build-system-fedora' job will typically take 20 minutes on the
> > gitlab.com shared runners. With ccache this is reduced to as little as
> > 6 minutes.
>
> I've been using ccache when building qemu in debian, for quite a while.
>
> The problem here, in the way qemu build system works, is that the cache
> is hugely dependent on the path to the source. You change just one char
> in there (/build/qemu/v8.1.0/ => /build/qemu/v8.1.1) and whole cache becomes
> unusable, it all gets compiled anew. This is because qemu build sys uses
> absolute file names when building, and this is detected by ccache, so
> the source dir gets mixed into the hash together with gcc version and
> other things.
>
> Dunno how well this will work in the qemu ci though.
Should be fine, as gitlab always checks out code in a fixed directory
name matching the git repo name.
With 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 :|
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 01/12] gitlab: enable ccache for many build jobs
2023-08-25 7:46 ` Michael Tokarev
2023-08-25 8:11 ` Daniel P. Berrangé
@ 2023-08-25 8:34 ` Philippe Mathieu-Daudé
2023-08-25 8:43 ` Thomas Huth
2023-08-25 9:13 ` Michael Tokarev
1 sibling, 2 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-08-25 8:34 UTC (permalink / raw)
To: Michael Tokarev, Alex Bennée, qemu-devel, Paolo Bonzini
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki, qemu-arm,
Juan Quintela, Thomas Huth, Peter Maydell, Ilya Leoshkevich,
Daniel P. Berrangé, David Hildenbrand, Greg Kurz,
Wainer dos Santos Moschetta, Cédric Le Goater
On 25/8/23 09:46, Michael Tokarev wrote:
> 24.08.2023 19:38, Alex Bennée wrote:
>> From: Daniel P. Berrangé <berrange@redhat.com>
>>
>> The `ccache` tool can be very effective at reducing compilation times
>> when re-running pipelines with only minor changes each time. For example
>> a fresh 'build-system-fedora' job will typically take 20 minutes on the
>> gitlab.com shared runners. With ccache this is reduced to as little as
>> 6 minutes.
>
> I've been using ccache when building qemu in debian, for quite a while.
>
> The problem here, in the way qemu build system works, is that the cache
> is hugely dependent on the path to the source. You change just one char
> in there (/build/qemu/v8.1.0/ => /build/qemu/v8.1.1) and whole cache
> becomes
> unusable, it all gets compiled anew. This is because qemu build sys uses
> absolute file names when building, and this is detected by ccache, so
> the source dir gets mixed into the hash together with gcc version and
> other things.
__FILE__ is used by assert() family, some DEBUG_PRINTF(), but mainly
by "qapi/error.h", so all error_setg*() calls.
This has been bugging me since quite some time, since if you build
the same QEMU in different paths (usually on different machines) then
the output doesn't match.
GCC 8 & Clang 10 provides -ffile-prefix-map, but
1/ Our minimal GCC supported is v7.4,
2/ meson doesn't support it. Still there is a feature request:
https://github.com/mesonbuild/meson/issues/10533
For more info, see also the reproducible build project:
https://reproducible-builds.org/docs/build-path/
Regards,
Phil.
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 01/12] gitlab: enable ccache for many build jobs
2023-08-25 8:34 ` Philippe Mathieu-Daudé
@ 2023-08-25 8:43 ` Thomas Huth
2023-08-25 9:13 ` Michael Tokarev
1 sibling, 0 replies; 26+ messages in thread
From: Thomas Huth @ 2023-08-25 8:43 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael Tokarev, Alex Bennée,
qemu-devel, Paolo Bonzini
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki, qemu-arm,
Juan Quintela, Peter Maydell, Ilya Leoshkevich,
Daniel P. Berrangé, David Hildenbrand, Greg Kurz,
Wainer dos Santos Moschetta, Cédric Le Goater
On 25/08/2023 10.34, Philippe Mathieu-Daudé wrote:
...
> __FILE__ is used by assert() family, some DEBUG_PRINTF(), but mainly
> by "qapi/error.h", so all error_setg*() calls.
>
> This has been bugging me since quite some time, since if you build
> the same QEMU in different paths (usually on different machines) then
> the output doesn't match.
>
> GCC 8 & Clang 10 provides -ffile-prefix-map, but
>
> 1/ Our minimal GCC supported is v7.4,
IIRC we use 7.4 since this was the default GCC on NetBSD 9 ... I guess we
could bump it once NetBSD 10 gets released.
Thomas
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 01/12] gitlab: enable ccache for many build jobs
2023-08-25 8:34 ` Philippe Mathieu-Daudé
2023-08-25 8:43 ` Thomas Huth
@ 2023-08-25 9:13 ` Michael Tokarev
1 sibling, 0 replies; 26+ messages in thread
From: Michael Tokarev @ 2023-08-25 9:13 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Alex Bennée, qemu-devel,
Paolo Bonzini
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki, qemu-arm,
Juan Quintela, Thomas Huth, Peter Maydell, Ilya Leoshkevich,
Daniel P. Berrangé, David Hildenbrand, Greg Kurz,
Wainer dos Santos Moschetta, Cédric Le Goater
25.08.2023 11:34, Philippe Mathieu-Daudé wrote:
...
> __FILE__ is used by assert() family, some DEBUG_PRINTF(), but mainly
> by "qapi/error.h", so all error_setg*() calls.
>
> This has been bugging me since quite some time, since if you build
> the same QEMU in different paths (usually on different machines) then
> the output doesn't match.
>
> GCC 8 & Clang 10 provides -ffile-prefix-map, but
This option is automatically enabled on debian. Still, ccache does
not use the old cache contents when building in a different directory.
/mjt
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 02/12] tests/docker: cleanup non-verbose output
2023-08-24 16:38 [PATCH v2 00/12] gdbstub and testing fixes for 8.2 Alex Bennée
2023-08-24 16:38 ` [PATCH v2 01/12] gitlab: enable ccache for many build jobs Alex Bennée
@ 2023-08-24 16:39 ` Alex Bennée
2023-08-24 16:39 ` [PATCH v2 03/12] tests/tcg: remove quoting for info output Alex Bennée
` (9 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Alex Bennée @ 2023-08-24 16:39 UTC (permalink / raw)
To: qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki,
Alex Bennée, qemu-arm, Juan Quintela, Thomas Huth,
Peter Maydell, Ilya Leoshkevich, Daniel P. Berrangé,
David Hildenbrand, Greg Kurz, Wainer dos Santos Moschetta,
Philippe Mathieu-Daudé, Cédric Le Goater
Even with --quiet docker will spam the sha256 to the console. Avoid
this by redirecting stdout. While we are at it fix the name we echo
which was broken during 0b1a649047 (tests/docker: use direct RUNC call
to build containers).
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
tests/docker/Makefile.include | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 142e8605ee..dfabafab92 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -46,9 +46,9 @@ docker-image-%: $(DOCKER_FILES_DIR)/%.docker
--build-arg BUILDKIT_INLINE_CACHE=1 \
$(if $(NOUSER),, \
--build-arg USER=$(USER) \
- --build-arg UID=$(UID)) \
- -t qemu/$* - < $<, \
- "BUILD", $1)
+ --build-arg UID=$(UID)) \
+ -t qemu/$* - < $< $(if $V,,> /dev/null),\
+ "BUILD", $*)
# Special rule for debootstraped binfmt linux-user images
docker-binfmt-image-debian-%: $(DOCKER_FILES_DIR)/debian-bootstrap.docker
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 03/12] tests/tcg: remove quoting for info output
2023-08-24 16:38 [PATCH v2 00/12] gdbstub and testing fixes for 8.2 Alex Bennée
2023-08-24 16:38 ` [PATCH v2 01/12] gitlab: enable ccache for many build jobs Alex Bennée
2023-08-24 16:39 ` [PATCH v2 02/12] tests/docker: cleanup non-verbose output Alex Bennée
@ 2023-08-24 16:39 ` Alex Bennée
2023-08-24 16:39 ` [PATCH v2 04/12] docs/style: permit inline loop variables Alex Bennée
` (8 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Alex Bennée @ 2023-08-24 16:39 UTC (permalink / raw)
To: qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki,
Alex Bennée, qemu-arm, Juan Quintela, Thomas Huth,
Peter Maydell, Ilya Leoshkevich, Daniel P. Berrangé,
David Hildenbrand, Greg Kurz, Wainer dos Santos Moschetta,
Philippe Mathieu-Daudé, Cédric Le Goater
This avoids ugly multi-line wrapping for the test on non V=1 builds.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
tests/tcg/aarch64/Makefile.target | 2 +-
tests/tcg/multiarch/system/Makefile.softmmu-target | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index 681dfa077c..b77bbd9b3c 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -14,7 +14,7 @@ AARCH64_TESTS=fcvt pcalign-a64 lse2-fault
fcvt: LDFLAGS+=-lm
run-fcvt: fcvt
- $(call run-test,$<,$(QEMU) $<, "$< on $(TARGET_NAME)")
+ $(call run-test,$<,$(QEMU) $<)
$(call diff-out,$<,$(AARCH64_SRC)/fcvt.ref)
config-cc.mak: Makefile
diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target
index 7ba9053375..a051d689d7 100644
--- a/tests/tcg/multiarch/system/Makefile.softmmu-target
+++ b/tests/tcg/multiarch/system/Makefile.softmmu-target
@@ -37,10 +37,10 @@ run-gdbstub-untimely-packet: hello
--qemu $(QEMU) \
--bin $< --qargs \
"-monitor none -display none -chardev file$(COMMA)path=untimely-packet.out$(COMMA)id=output $(QEMU_OPTS)", \
- "softmmu gdbstub untimely packets")
+ softmmu gdbstub untimely packets)
$(call quiet-command, \
(! grep -Fq 'Packet instead of Ack, ignoring it' untimely-packet.gdb.err), \
- "GREP", "file untimely-packet.gdb.err")
+ "GREP", file untimely-packet.gdb.err)
else
run-gdbstub-%:
$(call skip-test, "gdbstub test $*", "no guest arch support")
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 04/12] docs/style: permit inline loop variables
2023-08-24 16:38 [PATCH v2 00/12] gdbstub and testing fixes for 8.2 Alex Bennée
` (2 preceding siblings ...)
2023-08-24 16:39 ` [PATCH v2 03/12] tests/tcg: remove quoting for info output Alex Bennée
@ 2023-08-24 16:39 ` Alex Bennée
2023-08-24 16:39 ` [PATCH v2 05/12] .gitlab-ci.d/cirrus.yml: Update FreeBSD to v13.2 Alex Bennée
` (7 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Alex Bennée @ 2023-08-24 16:39 UTC (permalink / raw)
To: qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki,
Alex Bennée, qemu-arm, Juan Quintela, Thomas Huth,
Peter Maydell, Ilya Leoshkevich, Daniel P. Berrangé,
David Hildenbrand, Greg Kurz, Wainer dos Santos Moschetta,
Philippe Mathieu-Daudé, Cédric Le Goater,
Stefan Hajnoczi
I've already wasted enough of my time debugging aliased variables in
deeply nested loops. While not scattering variable declarations around
is a good aim I think we can make an exception for stuff used inside a
loop.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20230822155004.1158931-1-alex.bennee@linaro.org>
---
docs/devel/style.rst | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/docs/devel/style.rst b/docs/devel/style.rst
index 3cfcdeb9cd..2f68b50079 100644
--- a/docs/devel/style.rst
+++ b/docs/devel/style.rst
@@ -204,7 +204,14 @@ Declarations
Mixed declarations (interleaving statements and declarations within
blocks) are generally not allowed; declarations should be at the beginning
-of blocks.
+of blocks. To avoid accidental re-use it is permissible to declare
+loop variables inside for loops:
+
+.. code-block:: c
+
+ for (int i = 0; i < ARRAY_SIZE(thing); i++) {
+ /* do something loopy */
+ }
Every now and then, an exception is made for declarations inside a
#ifdef or #ifndef block: if the code looks nicer, such declarations can
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 05/12] .gitlab-ci.d/cirrus.yml: Update FreeBSD to v13.2
2023-08-24 16:38 [PATCH v2 00/12] gdbstub and testing fixes for 8.2 Alex Bennée
` (3 preceding siblings ...)
2023-08-24 16:39 ` [PATCH v2 04/12] docs/style: permit inline loop variables Alex Bennée
@ 2023-08-24 16:39 ` Alex Bennée
2023-08-24 16:39 ` [PATCH v2 06/12] tests: remove test-gdbstub.py Alex Bennée
` (6 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Alex Bennée @ 2023-08-24 16:39 UTC (permalink / raw)
To: qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki,
Alex Bennée, qemu-arm, Juan Quintela, Thomas Huth,
Peter Maydell, Ilya Leoshkevich, Daniel P. Berrangé,
David Hildenbrand, Greg Kurz, Wainer dos Santos Moschetta,
Philippe Mathieu-Daudé, Cédric Le Goater
From: Thomas Huth <thuth@redhat.com>
The FreeBSD CI job started to fail due to linking problems ... time
to update to the latest version to get this fixed.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20230823144533.230477-1-thuth@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
.gitlab-ci.d/cirrus.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.gitlab-ci.d/cirrus.yml b/.gitlab-ci.d/cirrus.yml
index 1507c928e5..41d64d6680 100644
--- a/.gitlab-ci.d/cirrus.yml
+++ b/.gitlab-ci.d/cirrus.yml
@@ -50,7 +50,7 @@ x64-freebsd-13-build:
NAME: freebsd-13
CIRRUS_VM_INSTANCE_TYPE: freebsd_instance
CIRRUS_VM_IMAGE_SELECTOR: image_family
- CIRRUS_VM_IMAGE_NAME: freebsd-13-1
+ CIRRUS_VM_IMAGE_NAME: freebsd-13-2
CIRRUS_VM_CPUS: 8
CIRRUS_VM_RAM: 8G
UPDATE_COMMAND: pkg update; pkg upgrade -y
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 06/12] tests: remove test-gdbstub.py
2023-08-24 16:38 [PATCH v2 00/12] gdbstub and testing fixes for 8.2 Alex Bennée
` (4 preceding siblings ...)
2023-08-24 16:39 ` [PATCH v2 05/12] .gitlab-ci.d/cirrus.yml: Update FreeBSD to v13.2 Alex Bennée
@ 2023-08-24 16:39 ` Alex Bennée
2023-08-24 16:39 ` [PATCH v2 07/12] tests/tcg: clean-up gdb confirm/pagination settings Alex Bennée
` (5 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Alex Bennée @ 2023-08-24 16:39 UTC (permalink / raw)
To: qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki,
Alex Bennée, qemu-arm, Juan Quintela, Thomas Huth,
Peter Maydell, Ilya Leoshkevich, Daniel P. Berrangé,
David Hildenbrand, Greg Kurz, Wainer dos Santos Moschetta,
Philippe Mathieu-Daudé, Cédric Le Goater
This isn't directly called by our CI and because it doesn't run via
our run-test.py script does things slightly differently. Lets remove
it as we have plenty of working in-tree tests now for various aspects
of gdbstub.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
tests/guest-debug/test-gdbstub.py | 177 ------------------------------
1 file changed, 177 deletions(-)
delete mode 100644 tests/guest-debug/test-gdbstub.py
diff --git a/tests/guest-debug/test-gdbstub.py b/tests/guest-debug/test-gdbstub.py
deleted file mode 100644
index 98a5df4d42..0000000000
--- a/tests/guest-debug/test-gdbstub.py
+++ /dev/null
@@ -1,177 +0,0 @@
-#
-# This script needs to be run on startup
-# qemu -kernel ${KERNEL} -s -S
-# and then:
-# gdb ${KERNEL}.vmlinux -x ${QEMU_SRC}/tests/guest-debug/test-gdbstub.py
-
-import gdb
-
-failcount = 0
-
-
-def report(cond, msg):
- "Report success/fail of test"
- if cond:
- print ("PASS: %s" % (msg))
- else:
- print ("FAIL: %s" % (msg))
- global failcount
- failcount += 1
-
-
-def check_step():
- "Step an instruction, check it moved."
- start_pc = gdb.parse_and_eval('$pc')
- gdb.execute("si")
- end_pc = gdb.parse_and_eval('$pc')
-
- return not (start_pc == end_pc)
-
-
-def check_break(sym_name):
- "Setup breakpoint, continue and check we stopped."
- sym, ok = gdb.lookup_symbol(sym_name)
- bp = gdb.Breakpoint(sym_name)
-
- gdb.execute("c")
-
- # hopefully we came back
- end_pc = gdb.parse_and_eval('$pc')
- print ("%s == %s %d" % (end_pc, sym.value(), bp.hit_count))
- bp.delete()
-
- # can we test we hit bp?
- return end_pc == sym.value()
-
-
-# We need to do hbreak manually as the python interface doesn't export it
-def check_hbreak(sym_name):
- "Setup hardware breakpoint, continue and check we stopped."
- sym, ok = gdb.lookup_symbol(sym_name)
- gdb.execute("hbreak %s" % (sym_name))
- gdb.execute("c")
-
- # hopefully we came back
- end_pc = gdb.parse_and_eval('$pc')
- print ("%s == %s" % (end_pc, sym.value()))
-
- if end_pc == sym.value():
- gdb.execute("d 1")
- return True
- else:
- return False
-
-
-class WatchPoint(gdb.Breakpoint):
-
- def get_wpstr(self, sym_name):
- "Setup sym and wp_str for given symbol."
- self.sym, ok = gdb.lookup_symbol(sym_name)
- wp_addr = gdb.parse_and_eval(sym_name).address
- self.wp_str = '*(%(type)s)(&%(address)s)' % dict(
- type = wp_addr.type, address = sym_name)
-
- return(self.wp_str)
-
- def __init__(self, sym_name, type):
- wp_str = self.get_wpstr(sym_name)
- super(WatchPoint, self).__init__(wp_str, gdb.BP_WATCHPOINT, type)
-
- def stop(self):
- end_pc = gdb.parse_and_eval('$pc')
- print ("HIT WP @ %s" % (end_pc))
- return True
-
-
-def do_one_watch(sym, wtype, text):
-
- wp = WatchPoint(sym, wtype)
- gdb.execute("c")
- report_str = "%s for %s (%s)" % (text, sym, wp.sym.value())
-
- if wp.hit_count > 0:
- report(True, report_str)
- wp.delete()
- else:
- report(False, report_str)
-
-
-def check_watches(sym_name):
- "Watch a symbol for any access."
-
- # Should hit for any read
- do_one_watch(sym_name, gdb.WP_ACCESS, "awatch")
-
- # Again should hit for reads
- do_one_watch(sym_name, gdb.WP_READ, "rwatch")
-
- # Finally when it is written
- do_one_watch(sym_name, gdb.WP_WRITE, "watch")
-
-
-class CatchBreakpoint(gdb.Breakpoint):
- def __init__(self, sym_name):
- super(CatchBreakpoint, self).__init__(sym_name)
- self.sym, ok = gdb.lookup_symbol(sym_name)
-
- def stop(self):
- end_pc = gdb.parse_and_eval('$pc')
- print ("CB: %s == %s" % (end_pc, self.sym.value()))
- if end_pc == self.sym.value():
- report(False, "Hit final catchpoint")
-
-
-def run_test():
- "Run through the tests one by one"
-
- print ("Checking we can step the first few instructions")
- step_ok = 0
- for i in range(3):
- if check_step():
- step_ok += 1
-
- report(step_ok == 3, "single step in boot code")
-
- print ("Checking HW breakpoint works")
- break_ok = check_hbreak("kernel_init")
- report(break_ok, "hbreak @ kernel_init")
-
- # Can't set this up until we are in the kernel proper
- # if we make it to run_init_process we've over-run and
- # one of the tests failed
- print ("Setup catch-all for run_init_process")
- cbp = CatchBreakpoint("run_init_process")
- cpb2 = CatchBreakpoint("try_to_run_init_process")
-
- print ("Checking Normal breakpoint works")
- break_ok = check_break("wait_for_completion")
- report(break_ok, "break @ wait_for_completion")
-
- print ("Checking watchpoint works")
- check_watches("system_state")
-
-#
-# This runs as the script it sourced (via -x)
-#
-
-try:
- print ("Connecting to remote")
- gdb.execute("target remote localhost:1234")
-
- # These are not very useful in scripts
- gdb.execute("set pagination off")
- gdb.execute("set confirm off")
-
- # Run the actual tests
- run_test()
-
-except:
- print ("GDB Exception: %s" % (sys.exc_info()[0]))
- failcount += 1
- import code
- code.InteractiveConsole(locals=globals()).interact()
- raise
-
-# Finally kill the inferior and exit gdb with a count of failures
-gdb.execute("kill")
-exit(failcount)
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 07/12] tests/tcg: clean-up gdb confirm/pagination settings
2023-08-24 16:38 [PATCH v2 00/12] gdbstub and testing fixes for 8.2 Alex Bennée
` (5 preceding siblings ...)
2023-08-24 16:39 ` [PATCH v2 06/12] tests: remove test-gdbstub.py Alex Bennée
@ 2023-08-24 16:39 ` Alex Bennée
2023-08-24 16:39 ` [PATCH v2 08/12] gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT Alex Bennée
` (4 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Alex Bennée @ 2023-08-24 16:39 UTC (permalink / raw)
To: qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki,
Alex Bennée, qemu-arm, Juan Quintela, Thomas Huth,
Peter Maydell, Ilya Leoshkevich, Daniel P. Berrangé,
David Hildenbrand, Greg Kurz, Wainer dos Santos Moschetta,
Philippe Mathieu-Daudé, Cédric Le Goater
We can do this all in the run-test.py script so remove the extraneous
bits from the individual tests which got copied from the original
non-CI gdb tests.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
tests/guest-debug/run-test.py | 2 ++
tests/tcg/aarch64/gdbstub/test-sve-ioctl.py | 3 ---
tests/tcg/aarch64/gdbstub/test-sve.py | 3 ---
tests/tcg/multiarch/gdbstub/memory.py | 3 ---
tests/tcg/multiarch/gdbstub/sha1.py | 4 ----
tests/tcg/multiarch/gdbstub/test-proc-mappings.py | 4 ----
tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py | 4 ----
| 4 ----
tests/tcg/s390x/gdbstub/test-signals-s390x.py | 4 ----
tests/tcg/s390x/gdbstub/test-svc.py | 4 ----
10 files changed, 2 insertions(+), 33 deletions(-)
diff --git a/tests/guest-debug/run-test.py b/tests/guest-debug/run-test.py
index a032e01f79..b13b27d4b1 100755
--- a/tests/guest-debug/run-test.py
+++ b/tests/guest-debug/run-test.py
@@ -83,6 +83,8 @@ def log(output, msg):
gdb_cmd += " %s" % (args.gdb_args)
# run quietly and ignore .gdbinit
gdb_cmd += " -q -n -batch"
+ # disable pagination
+ gdb_cmd += " -ex 'set pagination off'"
# disable prompts in case of crash
gdb_cmd += " -ex 'set confirm off'"
# connect to remote
diff --git a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
index b9ef169c1a..ee8d467e59 100644
--- a/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
+++ b/tests/tcg/aarch64/gdbstub/test-sve-ioctl.py
@@ -76,9 +76,6 @@ def run_test():
exit(0)
try:
- # These are not very useful in scripts
- gdb.execute("set pagination off")
-
# Run the actual tests
run_test()
except:
diff --git a/tests/tcg/aarch64/gdbstub/test-sve.py b/tests/tcg/aarch64/gdbstub/test-sve.py
index ef57c7412c..afd8ece98d 100644
--- a/tests/tcg/aarch64/gdbstub/test-sve.py
+++ b/tests/tcg/aarch64/gdbstub/test-sve.py
@@ -66,9 +66,6 @@ def run_test():
exit(0)
try:
- # These are not very useful in scripts
- gdb.execute("set pagination off")
-
# Run the actual tests
run_test()
except:
diff --git a/tests/tcg/multiarch/gdbstub/memory.py b/tests/tcg/multiarch/gdbstub/memory.py
index 67864ad902..dd25e72281 100644
--- a/tests/tcg/multiarch/gdbstub/memory.py
+++ b/tests/tcg/multiarch/gdbstub/memory.py
@@ -115,9 +115,6 @@ def run_test():
exit(0)
try:
- # These are not very useful in scripts
- gdb.execute("set pagination off")
-
# Run the actual tests
run_test()
except (gdb.error):
diff --git a/tests/tcg/multiarch/gdbstub/sha1.py b/tests/tcg/multiarch/gdbstub/sha1.py
index 423b720e6d..416728415f 100644
--- a/tests/tcg/multiarch/gdbstub/sha1.py
+++ b/tests/tcg/multiarch/gdbstub/sha1.py
@@ -73,10 +73,6 @@ def run_test():
exit(0)
try:
- # These are not very useful in scripts
- gdb.execute("set pagination off")
- gdb.execute("set confirm off")
-
# Run the actual tests
run_test()
except (gdb.error):
diff --git a/tests/tcg/multiarch/gdbstub/test-proc-mappings.py b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
index 5e3e5a2fb7..04ec61d219 100644
--- a/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
+++ b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
@@ -51,10 +51,6 @@ def main():
exit(0)
try:
- # These are not very useful in scripts
- gdb.execute("set pagination off")
- gdb.execute("set confirm off")
-
# Run the actual tests
run_test()
except gdb.error:
diff --git a/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py b/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py
index d91e8fdf19..926fa962b7 100644
--- a/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py
+++ b/tests/tcg/multiarch/gdbstub/test-qxfer-auxv-read.py
@@ -42,10 +42,6 @@ def run_test():
exit(0)
try:
- # These are not very useful in scripts
- gdb.execute("set pagination off")
- gdb.execute("set confirm off")
-
# Run the actual tests
run_test()
except (gdb.error):
--git a/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py b/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py
index 798d508bc7..e57d2a8db8 100644
--- a/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py
+++ b/tests/tcg/multiarch/gdbstub/test-thread-breakpoint.py
@@ -45,10 +45,6 @@ def run_test():
exit(0)
try:
- # These are not very useful in scripts
- gdb.execute("set pagination off")
- gdb.execute("set confirm off")
-
# Run the actual tests
run_test()
except (gdb.error):
diff --git a/tests/tcg/s390x/gdbstub/test-signals-s390x.py b/tests/tcg/s390x/gdbstub/test-signals-s390x.py
index 80a284b475..ca2bbc0b03 100644
--- a/tests/tcg/s390x/gdbstub/test-signals-s390x.py
+++ b/tests/tcg/s390x/gdbstub/test-signals-s390x.py
@@ -61,10 +61,6 @@ def run_test():
exit(0)
try:
- # These are not very useful in scripts
- gdb.execute("set pagination off")
- gdb.execute("set confirm off")
-
# Run the actual tests
run_test()
except (gdb.error):
diff --git a/tests/tcg/s390x/gdbstub/test-svc.py b/tests/tcg/s390x/gdbstub/test-svc.py
index 18fad3f163..804705fede 100644
--- a/tests/tcg/s390x/gdbstub/test-svc.py
+++ b/tests/tcg/s390x/gdbstub/test-svc.py
@@ -49,10 +49,6 @@ def main():
exit(0)
try:
- # These are not very useful in scripts
- gdb.execute("set pagination off")
- gdb.execute("set confirm off")
-
# Run the actual tests
run_test()
except gdb.error:
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 08/12] gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT
2023-08-24 16:38 [PATCH v2 00/12] gdbstub and testing fixes for 8.2 Alex Bennée
` (6 preceding siblings ...)
2023-08-24 16:39 ` [PATCH v2 07/12] tests/tcg: clean-up gdb confirm/pagination settings Alex Bennée
@ 2023-08-24 16:39 ` Alex Bennée
2023-08-24 16:39 ` [PATCH v2 09/12] gdbstub: remove unused user_ctx field Alex Bennée
` (3 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Alex Bennée @ 2023-08-24 16:39 UTC (permalink / raw)
To: qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki,
Alex Bennée, qemu-arm, Juan Quintela, Thomas Huth,
Peter Maydell, Ilya Leoshkevich, Daniel P. Berrangé,
David Hildenbrand, Greg Kurz, Wainer dos Santos Moschetta,
Philippe Mathieu-Daudé, Cédric Le Goater,
Matheus Branco Borella
From: Matheus Branco Borella <dark.ryu.550@gmail.com>
This fix is implemented by having the vCont handler set the value of
`gdbserver_state.c_cpu` if any threads are to be resumed. The specific
CPU picked is arbitrarily from the ones to be resumed, but it should
be okay, as all GDB cares about is that it is a resumed thread.
Signed-off-by: Matheus Branco Borella <dark.ryu.550@gmail.com>
Message-Id: <20230804182633.47300-2-dark.ryu.550@gmail.com>
[AJB: style and whitespace fixes]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1725
---
v2
- fix up some whitespace
---
gdbstub/gdbstub.c | 29 ++++++
tests/tcg/multiarch/system/interrupt.c | 28 ++++++
tests/tcg/multiarch/gdbstub/interrupt.py | 97 +++++++++++++++++++
.../multiarch/system/Makefile.softmmu-target | 12 ++-
4 files changed, 164 insertions(+), 2 deletions(-)
create mode 100644 tests/tcg/multiarch/system/interrupt.c
create mode 100644 tests/tcg/multiarch/gdbstub/interrupt.py
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 5f28d5cf57..e7d48fa0d4 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -597,6 +597,15 @@ static int gdb_handle_vcont(const char *p)
* or incorrect parameters passed.
*/
res = 0;
+
+ /*
+ * target_count and last_target keep track of how many CPUs we are going to
+ * step or resume, and a pointer to the state structure of one of them,
+ * respectivelly
+ */
+ int target_count = 0;
+ CPUState *last_target = NULL;
+
while (*p) {
if (*p++ != ';') {
return -ENOTSUP;
@@ -637,6 +646,9 @@ static int gdb_handle_vcont(const char *p)
while (cpu) {
if (newstates[cpu->cpu_index] == 1) {
newstates[cpu->cpu_index] = cur_action;
+
+ target_count++;
+ last_target = cpu;
}
cpu = gdb_next_attached_cpu(cpu);
@@ -654,6 +666,9 @@ static int gdb_handle_vcont(const char *p)
while (cpu) {
if (newstates[cpu->cpu_index] == 1) {
newstates[cpu->cpu_index] = cur_action;
+
+ target_count++;
+ last_target = cpu;
}
cpu = gdb_next_cpu_in_process(cpu);
@@ -671,11 +686,25 @@ static int gdb_handle_vcont(const char *p)
/* only use if no previous match occourred */
if (newstates[cpu->cpu_index] == 1) {
newstates[cpu->cpu_index] = cur_action;
+
+ target_count++;
+ last_target = cpu;
}
break;
}
}
+ /*
+ * if we're about to resume a specific set of CPUs/threads, make it so that
+ * in case execution gets interrupted, we can send GDB a stop reply with a
+ * correct value. it doesn't really matter which CPU we tell GDB the signal
+ * happened in (VM pauses stop all of them anyway), so long as it is one of
+ * the ones we resumed/single stepped here.
+ */
+ if (target_count > 0) {
+ gdbserver_state.c_cpu = last_target;
+ }
+
gdbserver_state.signal = signal;
gdb_continue_partial(newstates);
return res;
diff --git a/tests/tcg/multiarch/system/interrupt.c b/tests/tcg/multiarch/system/interrupt.c
new file mode 100644
index 0000000000..98d4f2eff9
--- /dev/null
+++ b/tests/tcg/multiarch/system/interrupt.c
@@ -0,0 +1,28 @@
+/*
+ * External interruption test. This test is structured in such a way that it
+ * passes the cases that require it to exit, but we can make it enter an
+ * infinite loop from GDB.
+ *
+ * We don't have the benefit of libc, just builtin C primitives and
+ * whatever is in minilib.
+ */
+
+#include <minilib.h>
+
+void loop(void)
+{
+ do {
+ /*
+ * Loop forever. Just make sure the condition is always a constant
+ * expression, so that this loop is not UB, as per the C
+ * standard.
+ */
+ } while (1);
+}
+
+int main(void)
+{
+ return 0;
+}
+
+
diff --git a/tests/tcg/multiarch/gdbstub/interrupt.py b/tests/tcg/multiarch/gdbstub/interrupt.py
new file mode 100644
index 0000000000..e222ac94c5
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/interrupt.py
@@ -0,0 +1,97 @@
+from __future__ import print_function
+#
+# Test some of the softmmu debug features with the multiarch memory
+# test. It is a port of the original vmlinux focused test case but
+# using the "memory" test instead.
+#
+# This is launched via tests/guest-debug/run-test.py
+#
+
+import gdb
+import sys
+
+failcount = 0
+
+
+def report(cond, msg):
+ "Report success/fail of test"
+ if cond:
+ print("PASS: %s" % (msg))
+ else:
+ print("FAIL: %s" % (msg))
+ global failcount
+ failcount += 1
+
+
+def check_interrupt(thread):
+ """
+ Check that, if thread is resumed, we go back to the same thread when the
+ program gets interrupted.
+ """
+
+ # Switch to the thread we're going to be running the test in.
+ print("thread ", thread.num)
+ gdb.execute("thr %d" % thread.num)
+
+ # Enter the loop() function on this thread.
+ #
+ # While there are cleaner ways to do this, we want to minimize the number of
+ # side effects on the gdbstub's internal state, since those may mask bugs.
+ # Ideally, there should be no difference between what we're doing here and
+ # the program reaching the loop() function on its own.
+ #
+ # For this to be safe, we only need the prologue of loop() to not have
+ # instructions that may have problems with what we're doing here. We don't
+ # have to worry about anything else, as this function never returns.
+ gdb.execute("set $pc = loop")
+
+ # Continue and then interrupt the task.
+ gdb.post_event(lambda: gdb.execute("interrupt"))
+ gdb.execute("c")
+
+ # Check whether the thread we're in after the interruption is the same we
+ # ran continue from.
+ return (thread.num == gdb.selected_thread().num)
+
+
+def run_test():
+ """
+ Test if interrupting the code always lands us on the same thread when
+ running with scheduler-lock enabled.
+ """
+
+ gdb.execute("set scheduler-locking on")
+ for thread in gdb.selected_inferior().threads():
+ report(check_interrupt(thread),
+ "thread %d resumes correctly on interrupt" % thread.num)
+
+
+#
+# This runs as the script it sourced (via -x, via run-test.py)
+#
+try:
+ inferior = gdb.selected_inferior()
+ arch = inferior.architecture()
+ print("ATTACHED: %s" % arch.name())
+except (gdb.error, AttributeError):
+ print("SKIPPING (not connected)", file=sys.stderr)
+ exit(0)
+
+if gdb.parse_and_eval('$pc') == 0:
+ print("SKIP: PC not set")
+ exit(0)
+if len(gdb.selected_inferior().threads()) == 1:
+ print("SKIP: set to run on a single thread")
+ exit(0)
+
+try:
+ # Run the actual tests
+ run_test()
+except (gdb.error):
+ print("GDB Exception: %s" % (sys.exc_info()[0]))
+ failcount += 1
+ pass
+
+# Finally kill the inferior and exit gdb with a count of failures
+gdb.execute("kill")
+exit(failcount)
diff --git a/tests/tcg/multiarch/system/Makefile.softmmu-target b/tests/tcg/multiarch/system/Makefile.softmmu-target
index a051d689d7..90810a32b2 100644
--- a/tests/tcg/multiarch/system/Makefile.softmmu-target
+++ b/tests/tcg/multiarch/system/Makefile.softmmu-target
@@ -27,7 +27,15 @@ run-gdbstub-memory: memory
"-monitor none -display none -chardev file$(COMMA)path=$<.out$(COMMA)id=output $(QEMU_OPTS)" \
--bin $< --test $(MULTIARCH_SRC)/gdbstub/memory.py, \
softmmu gdbstub support)
-
+run-gdbstub-interrupt: interrupt
+ $(call run-test, $@, $(GDB_SCRIPT) \
+ --gdb $(HAVE_GDB_BIN) \
+ --qemu $(QEMU) \
+ --output $<.gdb.out \
+ --qargs \
+ "-smp 2 -monitor none -display none -chardev file$(COMMA)path=$<.out$(COMMA)id=output $(QEMU_OPTS)" \
+ --bin $< --test $(MULTIARCH_SRC)/gdbstub/interrupt.py, \
+ softmmu gdbstub support)
run-gdbstub-untimely-packet: hello
$(call run-test, $@, $(GDB_SCRIPT) \
--gdb $(HAVE_GDB_BIN) \
@@ -50,4 +58,4 @@ run-gdbstub-%:
$(call skip-test, "gdbstub test $*", "need working gdb")
endif
-MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-untimely-packet
+MULTIARCH_RUNS += run-gdbstub-memory run-gdbstub-interrupt run-gdbstub-untimely-packet
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 09/12] gdbstub: remove unused user_ctx field
2023-08-24 16:38 [PATCH v2 00/12] gdbstub and testing fixes for 8.2 Alex Bennée
` (7 preceding siblings ...)
2023-08-24 16:39 ` [PATCH v2 08/12] gdbstub: fixes cases where wrong threads were reported to GDB on SIGINT Alex Bennée
@ 2023-08-24 16:39 ` Alex Bennée
2023-08-25 7:31 ` Philippe Mathieu-Daudé
2023-08-24 16:39 ` [PATCH v2 10/12] gdbstub: refactor get_feature_xml Alex Bennée
` (2 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Alex Bennée @ 2023-08-24 16:39 UTC (permalink / raw)
To: qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki,
Alex Bennée, qemu-arm, Juan Quintela, Thomas Huth,
Peter Maydell, Ilya Leoshkevich, Daniel P. Berrangé,
David Hildenbrand, Greg Kurz, Wainer dos Santos Moschetta,
Philippe Mathieu-Daudé, Cédric Le Goater
This was always NULL so drop it.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
gdbstub/gdbstub.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index e7d48fa0d4..8e9bc17e07 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -836,7 +836,7 @@ static inline int startswith(const char *string, const char *pattern)
return !strncmp(string, pattern, strlen(pattern));
}
-static int process_string_cmd(void *user_ctx, const char *data,
+static int process_string_cmd(const char *data,
const GdbCmdParseEntry *cmds, int num_cmds)
{
int i;
@@ -863,7 +863,7 @@ static int process_string_cmd(void *user_ctx, const char *data,
}
gdbserver_state.allow_stop_reply = cmd->allow_stop_reply;
- cmd->handler(params, user_ctx);
+ cmd->handler(params, NULL);
return 0;
}
@@ -881,7 +881,7 @@ static void run_cmd_parser(const char *data, const GdbCmdParseEntry *cmd)
/* In case there was an error during the command parsing we must
* send a NULL packet to indicate the command is not supported */
- if (process_string_cmd(NULL, data, cmd, 1)) {
+ if (process_string_cmd(data, cmd, 1)) {
gdb_put_packet("");
}
}
@@ -1394,7 +1394,7 @@ static void handle_v_commands(GArray *params, void *user_ctx)
return;
}
- if (process_string_cmd(NULL, get_param(params, 0)->data,
+ if (process_string_cmd(get_param(params, 0)->data,
gdb_v_commands_table,
ARRAY_SIZE(gdb_v_commands_table))) {
gdb_put_packet("");
@@ -1738,13 +1738,13 @@ static void handle_gen_query(GArray *params, void *user_ctx)
return;
}
- if (!process_string_cmd(NULL, get_param(params, 0)->data,
+ if (!process_string_cmd(get_param(params, 0)->data,
gdb_gen_query_set_common_table,
ARRAY_SIZE(gdb_gen_query_set_common_table))) {
return;
}
- if (process_string_cmd(NULL, get_param(params, 0)->data,
+ if (process_string_cmd(get_param(params, 0)->data,
gdb_gen_query_table,
ARRAY_SIZE(gdb_gen_query_table))) {
gdb_put_packet("");
@@ -1757,13 +1757,13 @@ static void handle_gen_set(GArray *params, void *user_ctx)
return;
}
- if (!process_string_cmd(NULL, get_param(params, 0)->data,
+ if (!process_string_cmd(get_param(params, 0)->data,
gdb_gen_query_set_common_table,
ARRAY_SIZE(gdb_gen_query_set_common_table))) {
return;
}
- if (process_string_cmd(NULL, get_param(params, 0)->data,
+ if (process_string_cmd(get_param(params, 0)->data,
gdb_gen_set_table,
ARRAY_SIZE(gdb_gen_set_table))) {
gdb_put_packet("");
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 09/12] gdbstub: remove unused user_ctx field
2023-08-24 16:39 ` [PATCH v2 09/12] gdbstub: remove unused user_ctx field Alex Bennée
@ 2023-08-25 7:31 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-08-25 7:31 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki, qemu-arm,
Juan Quintela, Thomas Huth, Peter Maydell, Ilya Leoshkevich,
Daniel P. Berrangé, David Hildenbrand, Greg Kurz,
Wainer dos Santos Moschetta, Cédric Le Goater
On 24/8/23 18:39, Alex Bennée wrote:
> This was always NULL so drop it.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> gdbstub/gdbstub.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 10/12] gdbstub: refactor get_feature_xml
2023-08-24 16:38 [PATCH v2 00/12] gdbstub and testing fixes for 8.2 Alex Bennée
` (8 preceding siblings ...)
2023-08-24 16:39 ` [PATCH v2 09/12] gdbstub: remove unused user_ctx field Alex Bennée
@ 2023-08-24 16:39 ` Alex Bennée
2023-08-24 19:25 ` Richard Henderson
2023-08-24 16:39 ` [PATCH v2 11/12] gdbstub: replace global gdb_has_xml with a function Alex Bennée
2023-08-24 16:39 ` [PATCH v2 12/12] gdbstub: move comment for gdb_register_coprocessor Alex Bennée
11 siblings, 1 reply; 26+ messages in thread
From: Alex Bennée @ 2023-08-24 16:39 UTC (permalink / raw)
To: qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki,
Alex Bennée, qemu-arm, Juan Quintela, Thomas Huth,
Peter Maydell, Ilya Leoshkevich, Daniel P. Berrangé,
David Hildenbrand, Greg Kurz, Wainer dos Santos Moschetta,
Philippe Mathieu-Daudé, Cédric Le Goater
Try to bring up the code to more modern standards by:
- use dynamic GString built xml over a fixed buffer
- use autofree to save on explicit g_free() calls
- don't hand hack strstr to find the delimiter
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2
- avoid needless g_strndup for copy of annex
---
gdbstub/internals.h | 2 +-
gdbstub/gdbstub.c | 63 +++++++++++++++++++++------------------------
2 files changed, 31 insertions(+), 34 deletions(-)
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index f2b46cce41..4876ebd74f 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -33,7 +33,7 @@ typedef struct GDBProcess {
uint32_t pid;
bool attached;
- char target_xml[1024];
+ char *target_xml;
} GDBProcess;
enum RSState {
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 8e9bc17e07..31a2451f27 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -354,54 +354,51 @@ static CPUState *gdb_get_cpu(uint32_t pid, uint32_t tid)
static const char *get_feature_xml(const char *p, const char **newp,
GDBProcess *process)
{
- size_t len;
int i;
const char *name;
CPUState *cpu = gdb_get_first_cpu_in_process(process);
CPUClass *cc = CPU_GET_CLASS(cpu);
- len = 0;
- while (p[len] && p[len] != ':')
- len++;
- *newp = p + len;
+ /* ‘qXfer:features:read:annex:offset,length' */
+ char *term = g_strstr_len(p, -1, ":");
+ size_t len = term - p;
+ g_autofree char *annex = g_strndup(p, len);
+ /* leave newp at offset,length for the rest of the params */
+ *newp = term + 1;
- name = NULL;
- if (strncmp(p, "target.xml", len) == 0) {
- char *buf = process->target_xml;
- const size_t buf_sz = sizeof(process->target_xml);
- /* Generate the XML description for this CPU. */
- if (!buf[0]) {
+ name = NULL;
+ if (g_strcmp0(annex, "target.xml") == 0) {
+ if (!process->target_xml) {
GDBRegisterState *r;
+ GString *xml = g_string_new("<?xml version=\"1.0\"?>");
+
+ g_string_append(xml,
+ "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
+ "<target>");
- pstrcat(buf, buf_sz,
- "<?xml version=\"1.0\"?>"
- "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
- "<target>");
if (cc->gdb_arch_name) {
- gchar *arch = cc->gdb_arch_name(cpu);
- pstrcat(buf, buf_sz, "<architecture>");
- pstrcat(buf, buf_sz, arch);
- pstrcat(buf, buf_sz, "</architecture>");
- g_free(arch);
+ g_autofree gchar *arch = cc->gdb_arch_name(cpu);
+ g_string_append_printf(xml,
+ "<architecture>%s</architecture>",
+ arch);
}
- pstrcat(buf, buf_sz, "<xi:include href=\"");
- pstrcat(buf, buf_sz, cc->gdb_core_xml_file);
- pstrcat(buf, buf_sz, "\"/>");
+ g_string_append(xml, "<xi:include href=\"");
+ g_string_append(xml, cc->gdb_core_xml_file);
+ g_string_append(xml, "\"/>");
for (r = cpu->gdb_regs; r; r = r->next) {
- pstrcat(buf, buf_sz, "<xi:include href=\"");
- pstrcat(buf, buf_sz, r->xml);
- pstrcat(buf, buf_sz, "\"/>");
+ g_string_append(xml, "<xi:include href=\"");
+ g_string_append(xml, r->xml);
+ g_string_append(xml, "\"/>");
}
- pstrcat(buf, buf_sz, "</target>");
+ g_string_append(xml, "</target>");
+
+ process->target_xml = g_string_free(xml, false);
+ return process->target_xml;
}
- return buf;
}
if (cc->gdb_get_dynamic_xml) {
- char *xmlname = g_strndup(p, len);
- const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
-
- g_free(xmlname);
+ const char *xml = cc->gdb_get_dynamic_xml(cpu, annex);
if (xml) {
return xml;
}
@@ -2245,6 +2242,6 @@ void gdb_create_default_process(GDBState *s)
process = &s->processes[s->process_num - 1];
process->pid = pid;
process->attached = false;
- process->target_xml[0] = '\0';
+ process->target_xml = NULL;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 10/12] gdbstub: refactor get_feature_xml
2023-08-24 16:39 ` [PATCH v2 10/12] gdbstub: refactor get_feature_xml Alex Bennée
@ 2023-08-24 19:25 ` Richard Henderson
2023-08-29 12:04 ` Alex Bennée
0 siblings, 1 reply; 26+ messages in thread
From: Richard Henderson @ 2023-08-24 19:25 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
On 8/24/23 09:39, Alex Bennée wrote:
> Try to bring up the code to more modern standards by:
>
> - use dynamic GString built xml over a fixed buffer
> - use autofree to save on explicit g_free() calls
> - don't hand hack strstr to find the delimiter
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> v2
> - avoid needless g_strndup for copy of annex
> ---
> gdbstub/internals.h | 2 +-
> gdbstub/gdbstub.c | 63 +++++++++++++++++++++------------------------
> 2 files changed, 31 insertions(+), 34 deletions(-)
>
> diff --git a/gdbstub/internals.h b/gdbstub/internals.h
> index f2b46cce41..4876ebd74f 100644
> --- a/gdbstub/internals.h
> +++ b/gdbstub/internals.h
> @@ -33,7 +33,7 @@ typedef struct GDBProcess {
> uint32_t pid;
> bool attached;
>
> - char target_xml[1024];
> + char *target_xml;
> } GDBProcess;
>
> enum RSState {
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index 8e9bc17e07..31a2451f27 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -354,54 +354,51 @@ static CPUState *gdb_get_cpu(uint32_t pid, uint32_t tid)
> static const char *get_feature_xml(const char *p, const char **newp,
> GDBProcess *process)
> {
> - size_t len;
> int i;
> const char *name;
> CPUState *cpu = gdb_get_first_cpu_in_process(process);
> CPUClass *cc = CPU_GET_CLASS(cpu);
>
> - len = 0;
> - while (p[len] && p[len] != ':')
> - len++;
> - *newp = p + len;
> + /* ‘qXfer:features:read:annex:offset,length' */
This is misleading, because "...:read:" has already been consumed.
> + char *term = g_strstr_len(p, -1, ":");
This is strchr(p, ':').
> + g_autofree char *annex = g_strndup(p, len);
> + /* leave newp at offset,length for the rest of the params */
> + *newp = term + 1;
>
> - name = NULL;
> - if (strncmp(p, "target.xml", len) == 0) {
> - char *buf = process->target_xml;
> - const size_t buf_sz = sizeof(process->target_xml);
>
> - /* Generate the XML description for this CPU. */
> - if (!buf[0]) {
> + name = NULL;
> + if (g_strcmp0(annex, "target.xml") == 0) {
Why the change to g_strcmp0? There's no null pointer to be handled.
If you keep the strncmp, you don't have to allocate memory early.
> if (cc->gdb_get_dynamic_xml) {
> - char *xmlname = g_strndup(p, len);
> - const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
> -
> - g_free(xmlname);
> + const char *xml = cc->gdb_get_dynamic_xml(cpu, annex);
You can leave the g_strndup (and autofree) to here.
r~
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 10/12] gdbstub: refactor get_feature_xml
2023-08-24 19:25 ` Richard Henderson
@ 2023-08-29 12:04 ` Alex Bennée
0 siblings, 0 replies; 26+ messages in thread
From: Alex Bennée @ 2023-08-29 12:04 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
Richard Henderson <richard.henderson@linaro.org> writes:
> On 8/24/23 09:39, Alex Bennée wrote:
>> Try to bring up the code to more modern standards by:
>> - use dynamic GString built xml over a fixed buffer
>> - use autofree to save on explicit g_free() calls
>> - don't hand hack strstr to find the delimiter
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>> v2
>> - avoid needless g_strndup for copy of annex
>> ---
>> gdbstub/internals.h | 2 +-
>> gdbstub/gdbstub.c | 63 +++++++++++++++++++++------------------------
>> 2 files changed, 31 insertions(+), 34 deletions(-)
>> diff --git a/gdbstub/internals.h b/gdbstub/internals.h
>> index f2b46cce41..4876ebd74f 100644
>> --- a/gdbstub/internals.h
>> +++ b/gdbstub/internals.h
>> @@ -33,7 +33,7 @@ typedef struct GDBProcess {
>> uint32_t pid;
>> bool attached;
>> - char target_xml[1024];
>> + char *target_xml;
>> } GDBProcess;
>> enum RSState {
>> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
>> index 8e9bc17e07..31a2451f27 100644
>> --- a/gdbstub/gdbstub.c
>> +++ b/gdbstub/gdbstub.c
>> @@ -354,54 +354,51 @@ static CPUState *gdb_get_cpu(uint32_t pid, uint32_t tid)
>> static const char *get_feature_xml(const char *p, const char **newp,
>> GDBProcess *process)
>> {
>> - size_t len;
>> int i;
>> const char *name;
>> CPUState *cpu = gdb_get_first_cpu_in_process(process);
>> CPUClass *cc = CPU_GET_CLASS(cpu);
>> - len = 0;
>> - while (p[len] && p[len] != ':')
>> - len++;
>> - *newp = p + len;
>> + /* ‘qXfer:features:read:annex:offset,length' */
>
> This is misleading, because "...:read:" has already been consumed.
>
>> + char *term = g_strstr_len(p, -1, ":");
>
> This is strchr(p, ':').
>
>> + g_autofree char *annex = g_strndup(p, len);
>> + /* leave newp at offset,length for the rest of the params */
>> + *newp = term + 1;
>> - name = NULL;
>> - if (strncmp(p, "target.xml", len) == 0) {
>> - char *buf = process->target_xml;
>> - const size_t buf_sz = sizeof(process->target_xml);
>> - /* Generate the XML description for this CPU. */
>> - if (!buf[0]) {
>> + name = NULL;
>> + if (g_strcmp0(annex, "target.xml") == 0) {
>
> Why the change to g_strcmp0? There's no null pointer to be handled.
> If you keep the strncmp, you don't have to allocate memory early.
I figured by extracting annex upfront I would simplify the checks
further down. Anyway reverted and more clean-up applied.
>
>> if (cc->gdb_get_dynamic_xml) {
>> - char *xmlname = g_strndup(p, len);
>> - const char *xml = cc->gdb_get_dynamic_xml(cpu, xmlname);
>> -
>> - g_free(xmlname);
>> + const char *xml = cc->gdb_get_dynamic_xml(cpu, annex);
>
> You can leave the g_strndup (and autofree) to here.
>
>
>
> r~
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 11/12] gdbstub: replace global gdb_has_xml with a function
2023-08-24 16:38 [PATCH v2 00/12] gdbstub and testing fixes for 8.2 Alex Bennée
` (9 preceding siblings ...)
2023-08-24 16:39 ` [PATCH v2 10/12] gdbstub: refactor get_feature_xml Alex Bennée
@ 2023-08-24 16:39 ` Alex Bennée
2023-08-24 19:27 ` Richard Henderson
2023-08-25 7:33 ` Philippe Mathieu-Daudé
2023-08-24 16:39 ` [PATCH v2 12/12] gdbstub: move comment for gdb_register_coprocessor Alex Bennée
11 siblings, 2 replies; 26+ messages in thread
From: Alex Bennée @ 2023-08-24 16:39 UTC (permalink / raw)
To: qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki,
Alex Bennée, qemu-arm, Juan Quintela, Thomas Huth,
Peter Maydell, Ilya Leoshkevich, Daniel P. Berrangé,
David Hildenbrand, Greg Kurz, Wainer dos Santos Moschetta,
Philippe Mathieu-Daudé, Cédric Le Goater
Try and make the self reported global hack a little less hackish by
providing a query function instead. As gdb_has_xml was always set if
we negotiated XML we can now use the presence of ->target_xml as the
test instead.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
gdbstub/internals.h | 1 +
include/exec/gdbstub.h | 10 +++++-----
gdbstub/gdbstub.c | 12 +++++++-----
gdbstub/softmmu.c | 1 -
gdbstub/user.c | 1 -
target/arm/gdbstub.c | 8 ++++----
target/ppc/gdbstub.c | 4 ++--
7 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 4876ebd74f..fee243081f 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -33,6 +33,7 @@ typedef struct GDBProcess {
uint32_t pid;
bool attached;
+ /* If gdb sends qXfer:features:read:target.xml this will be populated */
char *target_xml;
} GDBProcess;
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 7d743fe1e9..0ee39cfdd1 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -31,12 +31,12 @@ int gdbserver_start(const char *port_or_device);
void gdb_set_stop_cpu(CPUState *cpu);
/**
- * gdb_has_xml:
- * This is an ugly hack to cope with both new and old gdb.
- * If gdb sends qXfer:features:read then assume we're talking to a newish
- * gdb that understands target descriptions.
+ * gdb_has_xml() - report of gdb supports modern target descriptions
+ *
+ * This will report true if the gdb negotiated qXfer:features:read
+ * target descriptions.
*/
-extern bool gdb_has_xml;
+bool gdb_has_xml(void);
/* in gdbstub-xml.c, generated by scripts/feature_to_c.sh */
extern const char *const xml_builtin[][2];
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 31a2451f27..10fb755390 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -75,8 +75,6 @@ void gdb_init_gdbserver_state(void)
gdbserver_state.sstep_flags &= gdbserver_state.supported_sstep_flags;
}
-bool gdb_has_xml;
-
/* writes 2*len+1 bytes in buf */
void gdb_memtohex(GString *buf, const uint8_t *mem, int len)
{
@@ -351,6 +349,11 @@ static CPUState *gdb_get_cpu(uint32_t pid, uint32_t tid)
}
}
+bool gdb_has_xml(void)
+{
+ return !!gdb_get_cpu_process(gdbserver_state.g_cpu)->target_xml;
+}
+
static const char *get_feature_xml(const char *p, const char **newp,
GDBProcess *process)
{
@@ -1078,7 +1081,7 @@ static void handle_set_reg(GArray *params, void *user_ctx)
{
int reg_size;
- if (!gdb_has_xml) {
+ if (!gdb_get_cpu_process(gdbserver_state.g_cpu)->target_xml) {
gdb_put_packet("");
return;
}
@@ -1099,7 +1102,7 @@ static void handle_get_reg(GArray *params, void *user_ctx)
{
int reg_size;
- if (!gdb_has_xml) {
+ if (!gdb_get_cpu_process(gdbserver_state.g_cpu)->target_xml) {
gdb_put_packet("");
return;
}
@@ -1566,7 +1569,6 @@ static void handle_query_xfer_features(GArray *params, void *user_ctx)
return;
}
- gdb_has_xml = true;
p = get_param(params, 0)->data;
xml = get_feature_xml(p, &p, process);
if (!xml) {
diff --git a/gdbstub/softmmu.c b/gdbstub/softmmu.c
index f509b7285d..9f0b8b5497 100644
--- a/gdbstub/softmmu.c
+++ b/gdbstub/softmmu.c
@@ -97,7 +97,6 @@ static void gdb_chr_event(void *opaque, QEMUChrEvent event)
vm_stop(RUN_STATE_PAUSED);
replay_gdb_attached();
- gdb_has_xml = false;
break;
default:
break;
diff --git a/gdbstub/user.c b/gdbstub/user.c
index 5b375be1d9..7ab6e5d975 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -198,7 +198,6 @@ static void gdb_accept_init(int fd)
gdbserver_state.c_cpu = gdb_first_attached_cpu();
gdbserver_state.g_cpu = gdbserver_state.c_cpu;
gdbserver_user_state.fd = fd;
- gdb_has_xml = false;
}
static bool gdb_accept_socket(int gdb_fd)
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index f421c5d041..8fc8351df7 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -48,7 +48,7 @@ int arm_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
}
if (n < 24) {
/* FPA registers. */
- if (gdb_has_xml) {
+ if (gdb_has_xml()) {
return 0;
}
return gdb_get_zeroes(mem_buf, 12);
@@ -56,7 +56,7 @@ int arm_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
switch (n) {
case 24:
/* FPA status register. */
- if (gdb_has_xml) {
+ if (gdb_has_xml()) {
return 0;
}
return gdb_get_reg32(mem_buf, 0);
@@ -102,7 +102,7 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
}
if (n < 24) { /* 16-23 */
/* FPA registers (ignored). */
- if (gdb_has_xml) {
+ if (gdb_has_xml()) {
return 0;
}
return 12;
@@ -110,7 +110,7 @@ int arm_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
switch (n) {
case 24:
/* FPA status register (ignored). */
- if (gdb_has_xml) {
+ if (gdb_has_xml()) {
return 0;
}
return 4;
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index ca39efdc35..2ad11510bf 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -56,7 +56,7 @@ static int ppc_gdb_register_len(int n)
return sizeof(target_ulong);
case 32 ... 63:
/* fprs */
- if (gdb_has_xml) {
+ if (gdb_has_xml()) {
return 0;
}
return 8;
@@ -76,7 +76,7 @@ static int ppc_gdb_register_len(int n)
return sizeof(target_ulong);
case 70:
/* fpscr */
- if (gdb_has_xml) {
+ if (gdb_has_xml()) {
return 0;
}
return sizeof(target_ulong);
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 11/12] gdbstub: replace global gdb_has_xml with a function
2023-08-24 16:39 ` [PATCH v2 11/12] gdbstub: replace global gdb_has_xml with a function Alex Bennée
@ 2023-08-24 19:27 ` Richard Henderson
2023-08-25 7:33 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 26+ messages in thread
From: Richard Henderson @ 2023-08-24 19:27 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
On 8/24/23 09:39, Alex Bennée wrote:
> Try and make the self reported global hack a little less hackish by
> providing a query function instead. As gdb_has_xml was always set if
> we negotiated XML we can now use the presence of ->target_xml as the
> test instead.
>
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
> gdbstub/internals.h | 1 +
> include/exec/gdbstub.h | 10 +++++-----
> gdbstub/gdbstub.c | 12 +++++++-----
> gdbstub/softmmu.c | 1 -
> gdbstub/user.c | 1 -
> target/arm/gdbstub.c | 8 ++++----
> target/ppc/gdbstub.c | 4 ++--
> 7 files changed, 19 insertions(+), 18 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 11/12] gdbstub: replace global gdb_has_xml with a function
2023-08-24 16:39 ` [PATCH v2 11/12] gdbstub: replace global gdb_has_xml with a function Alex Bennée
2023-08-24 19:27 ` Richard Henderson
@ 2023-08-25 7:33 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-08-25 7:33 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki, qemu-arm,
Juan Quintela, Thomas Huth, Peter Maydell, Ilya Leoshkevich,
Daniel P. Berrangé, David Hildenbrand, Greg Kurz,
Wainer dos Santos Moschetta, Cédric Le Goater
On 24/8/23 18:39, Alex Bennée wrote:
> Try and make the self reported global hack a little less hackish by
> providing a query function instead. As gdb_has_xml was always set if
> we negotiated XML we can now use the presence of ->target_xml as the
> test instead.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> gdbstub/internals.h | 1 +
> include/exec/gdbstub.h | 10 +++++-----
> gdbstub/gdbstub.c | 12 +++++++-----
> gdbstub/softmmu.c | 1 -
> gdbstub/user.c | 1 -
> target/arm/gdbstub.c | 8 ++++----
> target/ppc/gdbstub.c | 4 ++--
> 7 files changed, 19 insertions(+), 18 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 12/12] gdbstub: move comment for gdb_register_coprocessor
2023-08-24 16:38 [PATCH v2 00/12] gdbstub and testing fixes for 8.2 Alex Bennée
` (10 preceding siblings ...)
2023-08-24 16:39 ` [PATCH v2 11/12] gdbstub: replace global gdb_has_xml with a function Alex Bennée
@ 2023-08-24 16:39 ` Alex Bennée
2023-08-24 19:29 ` Richard Henderson
2023-08-25 7:34 ` Philippe Mathieu-Daudé
11 siblings, 2 replies; 26+ messages in thread
From: Alex Bennée @ 2023-08-24 16:39 UTC (permalink / raw)
To: qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki,
Alex Bennée, qemu-arm, Juan Quintela, Thomas Huth,
Peter Maydell, Ilya Leoshkevich, Daniel P. Berrangé,
David Hildenbrand, Greg Kurz, Wainer dos Santos Moschetta,
Philippe Mathieu-Daudé, Cédric Le Goater
Use proper kdoc style comments for this API function.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/exec/gdbstub.h | 10 ++++++++++
gdbstub/gdbstub.c | 6 ------
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 0ee39cfdd1..16a139043f 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -14,6 +14,16 @@
/* Get or set a register. Returns the size of the register. */
typedef int (*gdb_get_reg_cb)(CPUArchState *env, GByteArray *buf, int reg);
typedef int (*gdb_set_reg_cb)(CPUArchState *env, uint8_t *buf, int reg);
+
+/**
+ * gdb_register_coprocessor() - register a supplemental set of registers
+ * @cpu - the CPU associated with registers
+ * @get_reg - get function (gdb reading)
+ * @set_reg - set function (gdb modifying)
+ * @num_regs - number of registers in set
+ * @xml - xml name of set
+ * @gpos - non-zero to append to "general" register set at @gpos
+ */
void gdb_register_coprocessor(CPUState *cpu,
gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
int num_regs, const char *xml, int g_pos);
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 10fb755390..1fb0db747f 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -450,12 +450,6 @@ static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
return 0;
}
-/* Register a supplemental set of CPU registers. If g_pos is nonzero it
- specifies the first register number and these registers are included in
- a standard "g" packet. Direction is relative to gdb, i.e. get_reg is
- gdb reading a CPU register, and set_reg is gdb modifying a CPU register.
- */
-
void gdb_register_coprocessor(CPUState *cpu,
gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
int num_regs, const char *xml, int g_pos)
--
2.39.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 12/12] gdbstub: move comment for gdb_register_coprocessor
2023-08-24 16:39 ` [PATCH v2 12/12] gdbstub: move comment for gdb_register_coprocessor Alex Bennée
@ 2023-08-24 19:29 ` Richard Henderson
2023-08-25 7:34 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 26+ messages in thread
From: Richard Henderson @ 2023-08-24 19:29 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
On 8/24/23 09:39, Alex Bennée wrote:
> Use proper kdoc style comments for this API function.
>
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
> include/exec/gdbstub.h | 10 ++++++++++
> gdbstub/gdbstub.c | 6 ------
> 2 files changed, 10 insertions(+), 6 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 12/12] gdbstub: move comment for gdb_register_coprocessor
2023-08-24 16:39 ` [PATCH v2 12/12] gdbstub: move comment for gdb_register_coprocessor Alex Bennée
2023-08-24 19:29 ` Richard Henderson
@ 2023-08-25 7:34 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-08-25 7:34 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: Beraldo Leal, Nicholas Piggin, Richard Henderson,
Markus Armbruster, Yonggang Luo, qemu-ppc, David Gibson,
Daniel Henrique Barboza, qemu-s390x, Akihiko Odaki, qemu-arm,
Juan Quintela, Thomas Huth, Peter Maydell, Ilya Leoshkevich,
Daniel P. Berrangé, David Hildenbrand, Greg Kurz,
Wainer dos Santos Moschetta, Cédric Le Goater
On 24/8/23 18:39, Alex Bennée wrote:
> Use proper kdoc style comments for this API function.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> include/exec/gdbstub.h | 10 ++++++++++
> gdbstub/gdbstub.c | 6 ------
> 2 files changed, 10 insertions(+), 6 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 26+ messages in thread