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

From: Thomas Huth <thuth@redhat.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>,
	"Daniele Buono" <dbuono@linux.vnet.ibm.com>
Cc: "Alex Bennée" <alex.bennee@linaro.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	qemu-devel@nongnu.org,
	"Wainer dos Santos Moschetta" <wainersm@redhat.com>
Subject: Re: [PATCH v3 2/2] gitlab-ci.yml: Add jobs to test CFI flags
Date: Thu, 4 Mar 2021 12:21:16 +0100	[thread overview]
Message-ID: <571c0872-3755-3877-f284-01698264f4b7@redhat.com> (raw)
In-Reply-To: <YEC47lN4Xt3LUsP6@redhat.com>

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