Hi Stefano,
On 14/02/2023 00:56, Stefano Stabellini wrote:
>
>
> On Mon, 13 Feb 2023, Michal Orzel wrote:
>> Add a debian container with cppcheck installation routine inside,
>> capable of performing cppcheck analysis on Xen-only build including
>> cross-builds for arm32 and arm64.
>>
>> Populate build jobs making use of that container to run cppcheck
>> analysis to produce a text report (xen-cppcheck.txt) containing the list
>> of all the findings.
>>
>> This patch does not aim at performing any sort of bisection. Cppcheck is
>> imperfect and for now, our goal is to at least be aware of its reports,
>> so that we can compare them with the ones produced by better tools and
>> to be able to see how these reports change as a result of further
>> infrastructure improvements (e.g. exception list, rules exclusion).
>>
>> Signed-off-by: Michal Orzel <michal.or...@amd.com>
>
> Thanks for the patch, very nice!
>
>
>> ---
>> For those interested in, here is a sample pipeline:
>> https://gitlab.com/xen-project/people/morzel/xen-orzelmichal/-/pipelines/775769167
>> ---
>> .../build/debian/unstable-cppcheck.dockerfile | 37 +++++++++++++++++
>> automation/gitlab-ci/build.yaml | 40 +++++++++++++++++++
>> automation/scripts/build | 11 ++++-
>> 3 files changed, 87 insertions(+), 1 deletion(-)
>> create mode 100644 automation/build/debian/unstable-cppcheck.dockerfile
>>
>> diff --git a/automation/build/debian/unstable-cppcheck.dockerfile
>> b/automation/build/debian/unstable-cppcheck.dockerfile
>> new file mode 100644
>> index 000000000000..39bcc50673c8
>> --- /dev/null
>> +++ b/automation/build/debian/unstable-cppcheck.dockerfile
>> @@ -0,0 +1,37 @@
>> +FROM debian:unstable
>> +LABEL maintainer.name="The Xen Project" \
>> + maintainer.email="xen-devel@lists.xenproject.org"
>> +
>> +ENV DEBIAN_FRONTEND=noninteractive
>> +ENV CPPCHECK_VERSION=2.7
>> +ENV USER root
>> +
>> +RUN mkdir /build
>> +WORKDIR /build
>> +
>> +# dependencies for cppcheck and Xen-only build/cross-build
>> +RUN apt-get update && \
>> + apt-get --quiet --yes install \
>> + build-essential \
>> + curl \
>> + python-is-python3 \
>> + libpcre3-dev \
>> + flex \
>> + bison \
>> + gcc-arm-linux-gnueabihf \
>> + gcc-aarch64-linux-gnu
>> +
>> +# cppcheck release build (see cppcheck readme.md)
>> +RUN curl -fsSLO
>> https://github.com/danmar/cppcheck/archive/"$CPPCHECK_VERSION".tar.gz && \
>> + tar xvzf "$CPPCHECK_VERSION".tar.gz && \
>> + cd cppcheck-"$CPPCHECK_VERSION" && \
>> + make install -j$(nproc) \
>> + MATCHCOMPILER=yes \
>> + FILESDIR=/usr/share/cppcheck \
>> + HAVE_RULES=yes CXXFLAGS="-O2 -DNDEBUG -Wall -Wno-sign-compare
>> -Wno-unused-function"
>> +
>> +# clean
>> +RUN apt-get autoremove -y && \
>> + apt-get clean && \
>> + rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/* && \
>> + rm -rf cppcheck-"$CPPCHECK_VERSION"* "$CPPCHECK_VERSION".tar.gz
>> diff --git a/automation/gitlab-ci/build.yaml
>> b/automation/gitlab-ci/build.yaml
>> index a053c5c7325d..c8831ccbec7a 100644
>> --- a/automation/gitlab-ci/build.yaml
>> +++ b/automation/gitlab-ci/build.yaml
>> @@ -7,6 +7,7 @@
>> paths:
>> - binaries/
>> - xen-config
>> + - xen-cppcheck.txt
>> - '*.log'
>> - '*/*.log'
>> when: always
>> @@ -145,6 +146,23 @@
>> variables:
>> <<: *gcc
>>
>> +.arm64-cross-build-tmpl:
>> + <<: *build
>> + variables:
>> + XEN_TARGET_ARCH: arm64
>> + tags:
>> + - x86_64
>> +
>> +.arm64-cross-build:
>> + extends: .arm64-cross-build-tmpl
>> + variables:
>> + debug: n
>> +
>> +.gcc-arm64-cross-build:
>> + extends: .arm64-cross-build
>> + variables:
>> + <<: *gcc
>> +
>> .arm64-build-tmpl:
>> <<: *build
>> variables:
>> @@ -679,6 +697,28 @@ archlinux-current-gcc-riscv64-debug-randconfig:
>> EXTRA_FIXED_RANDCONFIG:
>> CONFIG_COVERAGE=n
>>
>> +# Cppcheck analysis jobs
>> +
>> +debian-unstable-gcc-cppcheck:
>> + extends: .gcc-x86-64-build
>> + variables:
>> + CONTAINER: debian:unstable-cppcheck
>> + CPPCHECK: y
>> +
>> +debian-unstable-gcc-arm32-cppcheck:
>> + extends: .gcc-arm32-cross-build
>> + variables:
>> + CONTAINER: debian:unstable-cppcheck
>> + CROSS_COMPILE: /usr/bin/arm-linux-gnueabihf-
>> + CPPCHECK: y
>> +
>> +debian-unstable-gcc-arm64-cppcheck:
>> + extends: .gcc-arm64-cross-build
>> + variables:
>> + CONTAINER: debian:unstable-cppcheck
>> + CROSS_COMPILE: /usr/bin/aarch64-linux-gnu-
>> + CPPCHECK: y
>> +
>> ## Test artifacts common
>>
>> .test-jobs-artifact-common:
>> diff --git a/automation/scripts/build b/automation/scripts/build
>> index f2f5e55bc04f..c219752d553e 100755
>> --- a/automation/scripts/build
>> +++ b/automation/scripts/build
>> @@ -38,7 +38,16 @@ cp xen/.config xen-config
>> # Directory for the artefacts to be dumped into
>> mkdir binaries
>>
>> -if [[ "${HYPERVISOR_ONLY}" == "y" ]]; then
>> +if [[ "${CPPCHECK}" == "y" ]]; then
>> + # Cppcheck analysis invokes Xen-only build.
>
> Given that when $CPPCHECK == y we are doing a hypervisor-only build,
> what do you think of also specifying $HYPERVISOR_ONLY == y in these
> cases?
>
> We could set both CPPCHECK=y and HYPERVISOR_ONLY=y in build.yaml and
> then here also check for both.
Well, these are just cosmetic changes not impacting anything important, so
if you want, I am ok to explicitly set HYPERVISOR_ONLY=y for cppcheck jobs.
>
>
>> + # Known limitation: cppcheck generates inconsistent reports when running
>> + # in parallel mode, therefore do not specify -j<n>.
>
> I take you tried -j$(nproc) on gitlab-ci and didn't work well? I tested
> -j$(nproc) in my native arm64 environment and seemed to work well.
Both me and Luca agreed on the fact that the reports are inconsistent when
running cppcheck analysis with -j$(nproc). It is not that it would fail to
create
a report. Instead, there will be some internal cppcheck errors present in a
report.
Therefore, to minimize the risk of producing incorrect list of findings it is
best
not to specify -j<n>.
>
>
>> + xen/scripts/xen-analysis.py --run-cppcheck --cppcheck-misra
>> +
>> + # Preserve artefacts
>> + cp xen/xen binaries/xen
>> + cp xen/cppcheck-report/xen-cppcheck.txt xen-cppcheck.txt
>> +elif [[ "${HYPERVISOR_ONLY}" == "y" ]]; then
>> # Xen-only build
>> make -j$(nproc) xen
>>
>> --
>> 2.25.1
>>
~Michal
