Re: [PATCH v3 2/2] gitlab-ci.yml: Add jobs to test CFI flags

[Daniel P . Berrangé]

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