All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] gitlab-ci.yml: Add jobs to test CFI
@ 2021-03-04  3:09 Daniele Buono
  2021-03-04  3:09 ` [PATCH v3 1/2] gitlab-ci.yml: Allow custom # of parallel linkers Daniele Buono
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Daniele Buono @ 2021-03-04  3:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Daniel P . Berrangé, Daniele Buono

For a few months now QEMU has had options to enable compiler-based
control-flow integrity if built with clang.

While this feature has a low maintenance, It's probably still better to
add tests to the CI environment to check that an update doesn't break it.

The patchset allow gitlab testing of:
* --enable-cfi: forward-edge cfi (function pointers)
* --enable-safe-stack: backward-edge cfi (return pointers)
As an added benefit, this also inherently tests LTO. 

The first patch allows a custom selection for linker parallelism.
Currently, make parallelism at build time is based on the number
of cpus available.
This doesn't work well with LTO at linking, because the linker has to
load in memory all the intermediate object files for optimization.
If the gitlab runner happens to run two linking processes at the same
time, the job will fail with an out-of-memory error,
The patch leverages the ability to maintain high parallelism at
compile time, but limit the number of linkers executed in parallel.

The second patch introduces the ci/cd jobs in the gitlab pipeline.
To maintain a limited number of short jobs, Daniel suggested to only
test targets where KVM is available. This restricted the jobs to
x86_64, ppc64, aarch64 and s390x. To keep the jobs under 1 hour, I
created three chains of build -> check -> acceptance jobs, divided by
architecture vendor (Intel, ARM, IBM).

For build, we have to select --enable-slirp=git, because CFI needs a
statically linked version of slirp, with CFI information. More info on
this can be found in a comment in .gitlab-ci.yml, or on a patch for
mason currently in ML:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg787636.html

Test runs of the full pipeline are here (cfi-ci-v3 branch):
https://gitlab.com/dbuono/qemu/-/pipelines/264484574

v3:
- Restricted the targets to x86_64, ppc64, aarch64 and s390x, under
  suggestion from Daniel.

v2:
- More details in the code about the issue of using system-wide slirp
- Use meson to only limit linker parallelism instead of forcing no
  parallelism on the whole compilation process.

Daniele Buono (2):
  gitlab-ci.yml: Allow custom # of parallel linkers
  gitlab-ci.yml: Add jobs to test CFI flags

 .gitlab-ci.yml | 123 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 123 insertions(+)

-- 
2.30.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/2] gitlab-ci.yml: Allow custom # of parallel linkers
  2021-03-04  3:09 [PATCH v3 0/2] gitlab-ci.yml: Add jobs to test CFI Daniele Buono
@ 2021-03-04  3:09 ` Daniele Buono
  2021-03-04 10:38   ` Daniel P. Berrangé
  2021-03-04  3:09 ` [PATCH v3 2/2] gitlab-ci.yml: Add jobs to test CFI flags Daniele Buono
  2021-03-04 19:56 ` [PATCH v3 0/2] gitlab-ci.yml: Add jobs to test CFI Alex Bennée
  2 siblings, 1 reply; 8+ messages in thread
From: Daniele Buono @ 2021-03-04  3:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Daniel P . Berrangé,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Paolo Bonzini, Alex Bennée,
	Daniele Buono

Define a new variable LD_JOBS, that can be used to select
the maximum number of linking jobs to be executed in parallel.
If the variable is not defined, maintain the default given by
make -j

Currently, make parallelism at build time is based on the number
of cpus available.

This doesn't work well with LTO at linking, because with LTO the
linker has to load in memory all the intermediate object files
for optimization.
The end result is that, if the gitlab runner happens to run two
linking processes at the same time, the job will fail with an
out-of-memory error,

This patch leverages the ability to maintain high parallelism at
compile time, but limit the number of linkers executed in parallel.

Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
---
 .gitlab-ci.yml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 8b6d495288..814f51873f 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -27,6 +27,10 @@ include:
       else
         ../configure --enable-werror $CONFIGURE_ARGS ;
       fi || { cat config.log meson-logs/meson-log.txt && exit 1; }
+    - if test -n "$LD_JOBS";
+      then
+        meson configure . -Dbackend_max_links="$LD_JOBS" ;
+      fi || exit 1;
     - make -j"$JOBS"
     - if test -n "$MAKE_CHECK_ARGS";
       then
-- 
2.30.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/2] gitlab-ci.yml: Add jobs to test CFI flags
  2021-03-04  3:09 [PATCH v3 0/2] gitlab-ci.yml: Add jobs to test CFI Daniele Buono
  2021-03-04  3:09 ` [PATCH v3 1/2] gitlab-ci.yml: Allow custom # of parallel linkers Daniele Buono
@ 2021-03-04  3:09 ` Daniele Buono
  2021-03-04 10:39   ` Daniel P. Berrangé
  2021-03-04 19:56 ` [PATCH v3 0/2] gitlab-ci.yml: Add jobs to test CFI Alex Bennée
  2 siblings, 1 reply; 8+ messages in thread
From: Daniele Buono @ 2021-03-04  3:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Daniel P . Berrangé,
	Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Paolo Bonzini, Alex Bennée,
	Daniele Buono

QEMU has had options to enable control-flow integrity features
for a few months now. Add two sets of build/check/acceptance
jobs to ensure the binary produced is working fine.

The three sets allow testing of x86_64 binaries for x86_64, s390x,
ppc64 and aarch64 targets

Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
---
 .gitlab-ci.yml | 119 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 814f51873f..7b1f25c92e 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -483,6 +483,125 @@ clang-user:
       --extra-cflags=-fsanitize=undefined --extra-cflags=-fno-sanitize-recover=undefined
     MAKE_CHECK_ARGS: check-unit check-tcg
 
+# Set LD_JOBS=1 because this requires LTO and ld consumes a large amount of memory.
+# On gitlab runners, default value sometimes end up calling 2 lds concurrently and
+# triggers an Out-Of-Memory error
+#
+# Since slirp callbacks are used in QEMU Timers, slirp needs to be compiled together
+# with QEMU and linked as a static library to avoid false positives in CFI checks.
+# This can be accomplished by using -enable-slirp=git, which avoids the use of
+# a system-wide version of the library
+#
+# Split in three sets of build/check/acceptance to limit the execution time of each
+# job
+build-cfi-arm:
+  <<: *native_build_job_definition
+  needs:
+  - job: amd64-fedora-container
+  variables:
+    LD_JOBS: 1
+    AR: llvm-ar
+    IMAGE: fedora
+    CONFIGURE_ARGS: --cc=clang --cxx=clang++ --enable-cfi --enable-cfi-debug
+      --enable-safe-stack --enable-slirp=git
+    TARGETS: aarch64-softmmu
+    MAKE_CHECK_ARGS: check-build
+  artifacts:
+    expire_in: 2 days
+    paths:
+      - build
+
+check-cfi-arm:
+  <<: *native_test_job_definition
+  needs:
+    - job: build-cfi-arm
+      artifacts: true
+  variables:
+    IMAGE: fedora
+    MAKE_CHECK_ARGS: check
+
+acceptance-cfi-arm:
+  <<: *native_test_job_definition
+  needs:
+    - job: build-cfi-arm
+      artifacts: true
+  variables:
+    IMAGE: fedora
+    MAKE_CHECK_ARGS: check-acceptance
+  <<: *acceptance_definition
+
+build-cfi-ibm:
+  <<: *native_build_job_definition
+  needs:
+  - job: amd64-fedora-container
+  variables:
+    LD_JOBS: 1
+    AR: llvm-ar
+    IMAGE: fedora
+    CONFIGURE_ARGS: --cc=clang --cxx=clang++ --enable-cfi --enable-cfi-debug
+      --enable-safe-stack --enable-slirp=git
+    TARGETS: ppc64-softmmu s390x-softmmu
+    MAKE_CHECK_ARGS: check-build
+  artifacts:
+    expire_in: 2 days
+    paths:
+      - build
+
+check-cfi-ibm:
+  <<: *native_test_job_definition
+  needs:
+    - job: build-cfi-ibm
+      artifacts: true
+  variables:
+    IMAGE: fedora
+    MAKE_CHECK_ARGS: check
+
+acceptance-cfi-ibm:
+  <<: *native_test_job_definition
+  needs:
+    - job: build-cfi-ibm
+      artifacts: true
+  variables:
+    IMAGE: fedora
+    MAKE_CHECK_ARGS: check-acceptance
+  <<: *acceptance_definition
+
+build-cfi-intel:
+  <<: *native_build_job_definition
+  needs:
+  - job: amd64-fedora-container
+  variables:
+    LD_JOBS: 1
+    AR: llvm-ar
+    IMAGE: fedora
+    CONFIGURE_ARGS: --cc=clang --cxx=clang++ --enable-cfi --enable-cfi-debug
+      --enable-safe-stack --enable-slirp=git
+    TARGETS: x86_64-softmmu
+    MAKE_CHECK_ARGS: check-build
+  artifacts:
+    expire_in: 2 days
+    paths:
+      - build
+
+check-cfi-intel:
+  <<: *native_test_job_definition
+  needs:
+    - job: build-cfi-intel
+      artifacts: true
+  variables:
+    IMAGE: fedora
+    MAKE_CHECK_ARGS: check
+
+acceptance-cfi-intel:
+  <<: *native_test_job_definition
+  needs:
+    - job: build-cfi-intel
+      artifacts: true
+  variables:
+    IMAGE: fedora
+    MAKE_CHECK_ARGS: check-acceptance
+  <<: *acceptance_definition
+
 tsan-build:
   <<: *native_build_job_definition
   variables:
-- 
2.30.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] gitlab-ci.yml: Allow custom # of parallel linkers
  2021-03-04  3:09 ` [PATCH v3 1/2] gitlab-ci.yml: Allow custom # of parallel linkers Daniele Buono
@ 2021-03-04 10:38   ` Daniel P. Berrangé
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel P. Berrangé @ 2021-03-04 10:38 UTC (permalink / raw)
  To: Daniele Buono
  Cc: Thomas Huth, Philippe Mathieu-Daudé,
	qemu-devel, Wainer dos Santos Moschetta, Paolo Bonzini,
	Alex Bennée

On Wed, Mar 03, 2021 at 10:09:47PM -0500, Daniele Buono wrote:
> Define a new variable LD_JOBS, that can be used to select
> the maximum number of linking jobs to be executed in parallel.
> If the variable is not defined, maintain the default given by
> make -j
> 
> Currently, make parallelism at build time is based on the number
> of cpus available.
> 
> This doesn't work well with LTO at linking, because with LTO the
> linker has to load in memory all the intermediate object files
> for optimization.
> The end result is that, if the gitlab runner happens to run two
> linking processes at the same time, the job will fail with an
> out-of-memory error,
> 
> This patch leverages the ability to maintain high parallelism at
> compile time, but limit the number of linkers executed in parallel.
> 
> Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
> ---
>  .gitlab-ci.yml | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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] 8+ messages in thread

* Re: [PATCH v3 2/2] gitlab-ci.yml: Add jobs to test CFI flags
  2021-03-04  3:09 ` [PATCH v3 2/2] gitlab-ci.yml: Add jobs to test CFI flags Daniele Buono
@ 2021-03-04 10:39   ` Daniel P. Berrangé
  2021-03-04 11:21     ` Thomas Huth
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel P. Berrangé @ 2021-03-04 10:39 UTC (permalink / raw)
  To: Daniele Buono
  Cc: Thomas Huth, Philippe Mathieu-Daudé,
	qemu-devel, Wainer dos Santos Moschetta, Paolo Bonzini,
	Alex Bennée

On Wed, Mar 03, 2021 at 10:09:48PM -0500, Daniele Buono wrote:
> QEMU has had options to enable control-flow integrity features
> for a few months now. Add two sets of build/check/acceptance
> jobs to ensure the binary produced is working fine.
> 
> The three sets allow testing of x86_64 binaries for x86_64, s390x,
> ppc64 and aarch64 targets
> 
> Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
> ---
>  .gitlab-ci.yml | 119 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 119 insertions(+)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 814f51873f..7b1f25c92e 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -483,6 +483,125 @@ clang-user:
>        --extra-cflags=-fsanitize=undefined --extra-cflags=-fno-sanitize-recover=undefined
>      MAKE_CHECK_ARGS: check-unit check-tcg
>  
> +# Set LD_JOBS=1 because this requires LTO and ld consumes a large amount of memory.
> +# On gitlab runners, default value sometimes end up calling 2 lds concurrently and
> +# triggers an Out-Of-Memory error
> +#
> +# Since slirp callbacks are used in QEMU Timers, slirp needs to be compiled together
> +# with QEMU and linked as a static library to avoid false positives in CFI checks.
> +# This can be accomplished by using -enable-slirp=git, which avoids the use of
> +# a system-wide version of the library
> +#
> +# Split in three sets of build/check/acceptance to limit the execution time of each
> +# job
> +build-cfi-arm:

s/arm/aarch64/

> +  <<: *native_build_job_definition
> +  needs:
> +  - job: amd64-fedora-container
> +  variables:
> +    LD_JOBS: 1
> +    AR: llvm-ar
> +    IMAGE: fedora
> +    CONFIGURE_ARGS: --cc=clang --cxx=clang++ --enable-cfi --enable-cfi-debug
> +      --enable-safe-stack --enable-slirp=git
> +    TARGETS: aarch64-softmmu
> +    MAKE_CHECK_ARGS: check-build
> +  artifacts:
> +    expire_in: 2 days
> +    paths:
> +      - build
> +
> +check-cfi-arm:
> +  <<: *native_test_job_definition
> +  needs:
> +    - job: build-cfi-arm
> +      artifacts: true
> +  variables:
> +    IMAGE: fedora
> +    MAKE_CHECK_ARGS: check
> +
> +acceptance-cfi-arm:
> +  <<: *native_test_job_definition
> +  needs:
> +    - job: build-cfi-arm
> +      artifacts: true
> +  variables:
> +    IMAGE: fedora
> +    MAKE_CHECK_ARGS: check-acceptance
> +  <<: *acceptance_definition
> +
> +build-cfi-ibm:

Lets not use vendor names here - keep the target names. ie

  build-cfi-s390x-ppc64

and equivalent for the rest of the jobs below....

> +  <<: *native_build_job_definition
> +  needs:
> +  - job: amd64-fedora-container
> +  variables:
> +    LD_JOBS: 1
> +    AR: llvm-ar
> +    IMAGE: fedora
> +    CONFIGURE_ARGS: --cc=clang --cxx=clang++ --enable-cfi --enable-cfi-debug
> +      --enable-safe-stack --enable-slirp=git
> +    TARGETS: ppc64-softmmu s390x-softmmu
> +    MAKE_CHECK_ARGS: check-build
> +  artifacts:
> +    expire_in: 2 days
> +    paths:
> +      - build
> +
> +check-cfi-ibm:
> +  <<: *native_test_job_definition
> +  needs:
> +    - job: build-cfi-ibm
> +      artifacts: true
> +  variables:
> +    IMAGE: fedora
> +    MAKE_CHECK_ARGS: check
> +
> +acceptance-cfi-ibm:
> +  <<: *native_test_job_definition
> +  needs:
> +    - job: build-cfi-ibm
> +      artifacts: true
> +  variables:
> +    IMAGE: fedora
> +    MAKE_CHECK_ARGS: check-acceptance
> +  <<: *acceptance_definition
> +
> +build-cfi-intel:
> +  <<: *native_build_job_definition
> +  needs:
> +  - job: amd64-fedora-container
> +  variables:
> +    LD_JOBS: 1
> +    AR: llvm-ar
> +    IMAGE: fedora
> +    CONFIGURE_ARGS: --cc=clang --cxx=clang++ --enable-cfi --enable-cfi-debug
> +      --enable-safe-stack --enable-slirp=git
> +    TARGETS: x86_64-softmmu
> +    MAKE_CHECK_ARGS: check-build
> +  artifacts:
> +    expire_in: 2 days
> +    paths:
> +      - build
> +
> +check-cfi-intel:
> +  <<: *native_test_job_definition
> +  needs:
> +    - job: build-cfi-intel
> +      artifacts: true
> +  variables:
> +    IMAGE: fedora
> +    MAKE_CHECK_ARGS: check
> +
> +acceptance-cfi-intel:
> +  <<: *native_test_job_definition
> +  needs:
> +    - job: build-cfi-intel
> +      artifacts: true
> +  variables:
> +    IMAGE: fedora
> +    MAKE_CHECK_ARGS: check-acceptance
> +  <<: *acceptance_definition
> +
>  tsan-build:
>    <<: *native_build_job_definition
>    variables:
> -- 
> 2.30.0
> 

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] 8+ messages in thread

* Re: [PATCH v3 2/2] gitlab-ci.yml: Add jobs to test CFI flags
  2021-03-04 10:39   ` Daniel P. Berrangé
@ 2021-03-04 11:21     ` Thomas Huth
  2021-03-04 11:58       ` Daniel P. Berrangé
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Huth @ 2021-03-04 11:21 UTC (permalink / raw)
  To: Daniel P. Berrangé, Daniele Buono
  Cc: Alex Bennée, Paolo Bonzini, Philippe Mathieu-Daudé,
	qemu-devel, Wainer dos Santos Moschetta

On 04/03/2021 11.39, Daniel P. Berrangé wrote:
> On Wed, Mar 03, 2021 at 10:09:48PM -0500, Daniele Buono wrote:
>> QEMU has had options to enable control-flow integrity features
>> for a few months now. Add two sets of build/check/acceptance
>> jobs to ensure the binary produced is working fine.
>>
>> The three sets allow testing of x86_64 binaries for x86_64, s390x,
>> ppc64 and aarch64 targets
>>
>> Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
>> ---
>>   .gitlab-ci.yml | 119 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 119 insertions(+)
>>
>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>> index 814f51873f..7b1f25c92e 100644
>> --- a/.gitlab-ci.yml
>> +++ b/.gitlab-ci.yml
>> @@ -483,6 +483,125 @@ clang-user:
>>         --extra-cflags=-fsanitize=undefined --extra-cflags=-fno-sanitize-recover=undefined
>>       MAKE_CHECK_ARGS: check-unit check-tcg
>>   
>> +# Set LD_JOBS=1 because this requires LTO and ld consumes a large amount of memory.
>> +# On gitlab runners, default value sometimes end up calling 2 lds concurrently and
>> +# triggers an Out-Of-Memory error
>> +#
>> +# Since slirp callbacks are used in QEMU Timers, slirp needs to be compiled together
>> +# with QEMU and linked as a static library to avoid false positives in CFI checks.
>> +# This can be accomplished by using -enable-slirp=git, which avoids the use of
>> +# a system-wide version of the library
>> +#
>> +# Split in three sets of build/check/acceptance to limit the execution time of each
>> +# job
>> +build-cfi-arm:
> 
> s/arm/aarch64/
> 
>> +  <<: *native_build_job_definition
>> +  needs:
>> +  - job: amd64-fedora-container
>> +  variables:
>> +    LD_JOBS: 1
>> +    AR: llvm-ar
>> +    IMAGE: fedora
>> +    CONFIGURE_ARGS: --cc=clang --cxx=clang++ --enable-cfi --enable-cfi-debug
>> +      --enable-safe-stack --enable-slirp=git
>> +    TARGETS: aarch64-softmmu
>> +    MAKE_CHECK_ARGS: check-build
>> +  artifacts:
>> +    expire_in: 2 days
>> +    paths:
>> +      - build
>> +
>> +check-cfi-arm:
>> +  <<: *native_test_job_definition
>> +  needs:
>> +    - job: build-cfi-arm
>> +      artifacts: true
>> +  variables:
>> +    IMAGE: fedora
>> +    MAKE_CHECK_ARGS: check
>> +
>> +acceptance-cfi-arm:
>> +  <<: *native_test_job_definition
>> +  needs:
>> +    - job: build-cfi-arm
>> +      artifacts: true
>> +  variables:
>> +    IMAGE: fedora
>> +    MAKE_CHECK_ARGS: check-acceptance
>> +  <<: *acceptance_definition
>> +
>> +build-cfi-ibm:
> 
> Lets not use vendor names here - keep the target names. ie
> 
>    build-cfi-s390x-ppc64
> 
> and equivalent for the rest of the jobs below....

I agree for not using vendor names here ... but I've got a different 
suggestion instead: Since our list of jobs has grown very big already, I 
think it would be nicer to group the jobs, see: 
https://docs.gitlab.com/ee/ci/jobs/#group-jobs-in-a-pipeline

That means, use "build-cfi 1/3", "build-cfi 2/3" and "build-cfi 3/3" (and do 
the same numbering for the check- and acceptance- jobs, too).

  Thomas




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/2] gitlab-ci.yml: Add jobs to test CFI flags
  2021-03-04 11:21     ` Thomas Huth
@ 2021-03-04 11:58       ` Daniel P. Berrangé
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel P. Berrangé @ 2021-03-04 11:58 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Philippe Mathieu-Daudé,
	qemu-devel, Wainer dos Santos Moschetta, Paolo Bonzini,
	Alex Bennée, Daniele Buono

On Thu, Mar 04, 2021 at 12:21:16PM +0100, Thomas Huth wrote:
> On 04/03/2021 11.39, Daniel P. Berrangé wrote:
> > On Wed, Mar 03, 2021 at 10:09:48PM -0500, Daniele Buono wrote:
> > > QEMU has had options to enable control-flow integrity features
> > > for a few months now. Add two sets of build/check/acceptance
> > > jobs to ensure the binary produced is working fine.
> > > 
> > > The three sets allow testing of x86_64 binaries for x86_64, s390x,
> > > ppc64 and aarch64 targets
> > > 
> > > Signed-off-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
> > > ---
> > >   .gitlab-ci.yml | 119 +++++++++++++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 119 insertions(+)
> > > 
> > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > > index 814f51873f..7b1f25c92e 100644
> > > --- a/.gitlab-ci.yml
> > > +++ b/.gitlab-ci.yml
> > > @@ -483,6 +483,125 @@ clang-user:
> > >         --extra-cflags=-fsanitize=undefined --extra-cflags=-fno-sanitize-recover=undefined
> > >       MAKE_CHECK_ARGS: check-unit check-tcg
> > > +# Set LD_JOBS=1 because this requires LTO and ld consumes a large amount of memory.
> > > +# On gitlab runners, default value sometimes end up calling 2 lds concurrently and
> > > +# triggers an Out-Of-Memory error
> > > +#
> > > +# Since slirp callbacks are used in QEMU Timers, slirp needs to be compiled together
> > > +# with QEMU and linked as a static library to avoid false positives in CFI checks.
> > > +# This can be accomplished by using -enable-slirp=git, which avoids the use of
> > > +# a system-wide version of the library
> > > +#
> > > +# Split in three sets of build/check/acceptance to limit the execution time of each
> > > +# job
> > > +build-cfi-arm:
> > 
> > s/arm/aarch64/
> > 
> > > +  <<: *native_build_job_definition
> > > +  needs:
> > > +  - job: amd64-fedora-container
> > > +  variables:
> > > +    LD_JOBS: 1
> > > +    AR: llvm-ar
> > > +    IMAGE: fedora
> > > +    CONFIGURE_ARGS: --cc=clang --cxx=clang++ --enable-cfi --enable-cfi-debug
> > > +      --enable-safe-stack --enable-slirp=git
> > > +    TARGETS: aarch64-softmmu
> > > +    MAKE_CHECK_ARGS: check-build
> > > +  artifacts:
> > > +    expire_in: 2 days
> > > +    paths:
> > > +      - build
> > > +
> > > +check-cfi-arm:
> > > +  <<: *native_test_job_definition
> > > +  needs:
> > > +    - job: build-cfi-arm
> > > +      artifacts: true
> > > +  variables:
> > > +    IMAGE: fedora
> > > +    MAKE_CHECK_ARGS: check
> > > +
> > > +acceptance-cfi-arm:
> > > +  <<: *native_test_job_definition
> > > +  needs:
> > > +    - job: build-cfi-arm
> > > +      artifacts: true
> > > +  variables:
> > > +    IMAGE: fedora
> > > +    MAKE_CHECK_ARGS: check-acceptance
> > > +  <<: *acceptance_definition
> > > +
> > > +build-cfi-ibm:
> > 
> > Lets not use vendor names here - keep the target names. ie
> > 
> >    build-cfi-s390x-ppc64
> > 
> > and equivalent for the rest of the jobs below....
> 
> I agree for not using vendor names here ... but I've got a different
> suggestion instead: Since our list of jobs has grown very big already, I
> think it would be nicer to group the jobs, see:
> https://docs.gitlab.com/ee/ci/jobs/#group-jobs-in-a-pipeline
> 
> That means, use "build-cfi 1/3", "build-cfi 2/3" and "build-cfi 3/3" (and do
> the same numbering for the check- and acceptance- jobs, too).

Oooh, that's an interesting feature. We could certainly benefit from that
in our existing jobs too


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] 8+ messages in thread

* Re: [PATCH v3 0/2] gitlab-ci.yml: Add jobs to test CFI
  2021-03-04  3:09 [PATCH v3 0/2] gitlab-ci.yml: Add jobs to test CFI Daniele Buono
  2021-03-04  3:09 ` [PATCH v3 1/2] gitlab-ci.yml: Allow custom # of parallel linkers Daniele Buono
  2021-03-04  3:09 ` [PATCH v3 2/2] gitlab-ci.yml: Add jobs to test CFI flags Daniele Buono
@ 2021-03-04 19:56 ` Alex Bennée
  2 siblings, 0 replies; 8+ messages in thread
From: Alex Bennée @ 2021-03-04 19:56 UTC (permalink / raw)
  To: Daniele Buono; +Cc: Paolo Bonzini, Daniel P . Berrangé, qemu-devel


Daniele Buono <dbuono@linux.vnet.ibm.com> writes:

> For a few months now QEMU has had options to enable compiler-based
> control-flow integrity if built with clang.
>
> While this feature has a low maintenance, It's probably still better to
> add tests to the CI environment to check that an update doesn't break it.
>
> The patchset allow gitlab testing of:
> * --enable-cfi: forward-edge cfi (function pointers)
> * --enable-safe-stack: backward-edge cfi (return pointers)
> As an added benefit, this also inherently tests LTO. 
>
> The first patch allows a custom selection for linker parallelism.
> Currently, make parallelism at build time is based on the number
> of cpus available.
> This doesn't work well with LTO at linking, because the linker has to
> load in memory all the intermediate object files for optimization.
> If the gitlab runner happens to run two linking processes at the same
> time, the job will fail with an out-of-memory error,
> The patch leverages the ability to maintain high parallelism at
> compile time, but limit the number of linkers executed in parallel.
>
> The second patch introduces the ci/cd jobs in the gitlab pipeline.
> To maintain a limited number of short jobs, Daniel suggested to only
> test targets where KVM is available. This restricted the jobs to
> x86_64, ppc64, aarch64 and s390x. To keep the jobs under 1 hour, I
> created three chains of build -> check -> acceptance jobs, divided by
> architecture vendor (Intel, ARM, IBM).
>
> For build, we have to select --enable-slirp=git, because CFI needs a
> statically linked version of slirp, with CFI information. More info on
> this can be found in a comment in .gitlab-ci.yml, or on a patch for
> mason currently in ML:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg787636.html
>
> Test runs of the full pipeline are here (cfi-ci-v3 branch):
> https://gitlab.com/dbuono/qemu/-/pipelines/264484574

Queued to testing/next, thanks.

-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-03-04 19:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-04  3:09 [PATCH v3 0/2] gitlab-ci.yml: Add jobs to test CFI Daniele Buono
2021-03-04  3:09 ` [PATCH v3 1/2] gitlab-ci.yml: Allow custom # of parallel linkers Daniele Buono
2021-03-04 10:38   ` Daniel P. Berrangé
2021-03-04  3:09 ` [PATCH v3 2/2] gitlab-ci.yml: Add jobs to test CFI flags Daniele Buono
2021-03-04 10:39   ` Daniel P. Berrangé
2021-03-04 11:21     ` Thomas Huth
2021-03-04 11:58       ` Daniel P. Berrangé
2021-03-04 19:56 ` [PATCH v3 0/2] gitlab-ci.yml: Add jobs to test CFI Alex Bennée

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.