Re: [PATCH] automation: Add container and build jobs to run cppcheck analysis

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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.orzel@xxxxxxx>

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@xxxxxxxxxxxxxxxxxxxx"
> +
> +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.


> +    # 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.


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