Re: [PATCH v3 59/70] x86: Use control flow typechecking where possible

From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v3 59/70] x86: Use control flow typechecking where possible
Date: Wed, 23 Feb 2022 15:21:44 +0100	[thread overview]
Message-ID: <d376a457-ef35-4fcb-38d9-d495b385fb65@suse.com> (raw)
In-Reply-To: <20220222152645.8844-14-andrew.cooper3@citrix.com>

On 22.02.2022 16:26, Andrew Cooper wrote:
> Now all callees have been annotated, turn on typechecking to catch issues in
> the future.
> 
> This extension isn't in a released version of GCC yet, so provide a container
> to use with the extention included, and add it to CI.  RANDCONFIG is necessary
> because some stubs for compiled-out subsystems are used as function pointer
> targets.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

I certainly see that we will want to have this extra level of safety.
Hence feel free to add
Acked-by: Jan Beulich <jbeulich@suse.com>

But I'd like us to form a plan in case the change doesn't get committed
relatively soon after gcc re-opens for general development after gcc12
was branched off. We don't want to get into the position of needing to
maintain a gcc patch. Do you have any insight as to what is actually
blocking the committing of that change, seeing that it has been around
for a while?

Jan

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> 
> v3:
>  * Provide container.  Already pushed to Gitlab.
> ---
>  automation/build/debian/buster-gcc-ibt.dockerfile | 66 +++++++++++++++++++++++
>  automation/gitlab-ci/build.yaml                   |  6 +++
>  automation/scripts/containerize                   |  1 +
>  xen/arch/x86/arch.mk                              |  1 +
>  4 files changed, 74 insertions(+)
>  create mode 100644 automation/build/debian/buster-gcc-ibt.dockerfile
> 
> diff --git a/automation/build/debian/buster-gcc-ibt.dockerfile b/automation/build/debian/buster-gcc-ibt.dockerfile
> new file mode 100644
> index 000000000000..441d9a9ab37a
> --- /dev/null
> +++ b/automation/build/debian/buster-gcc-ibt.dockerfile
> @@ -0,0 +1,66 @@
> +FROM debian:buster-slim AS builder
> +
> +ENV DEBIAN_FRONTEND=noninteractive
> +ENV USER root
> +
> +RUN apt-get update && \
> +    apt-get --quiet --yes install \
> +        bison \
> +        build-essential \
> +        flex \
> +        libc6-dev-i386 \
> +        libgmp-dev \
> +        libisl-dev \
> +        libmpc-dev \
> +        libmpfr-dev \
> +        patch \
> +        wget
> +
> +RUN mkdir /build
> +WORKDIR /build
> +
> +RUN wget -q https://ftp.gnu.org/gnu/gcc/gcc-11.2.0/gcc-11.2.0.tar.xz -O - | tar xJ --strip=1
> +RUN wget -q https://xenbits.xen.org/people/andrewcoop/gcc-11.2-Add-fcf-check-attribute-yes-no.patch -O - | patch -p1
> +RUN ./configure \
> +        --prefix=/opt/gcc-11-ibt \
> +        --enable-languages=c \
> +        --disable-nls \
> +        --disable-threads \
> +        --disable-bootstrap \
> +        --disable-shared \
> +        --disable-libmudflap \
> +        --disable-libssp \
> +        --disable-libgomp \
> +        --disable-decimal-float \
> +        --disable-libquadmath \
> +        --disable-libatomic \
> +        --disable-libcc1 \
> +        --disable-libmpx
> +RUN make -j`nproc` && make -j`nproc` install
> +
> +
> +FROM debian:buster-slim
> +COPY --from=builder /opt/gcc-11-ibt /opt/gcc-11-ibt
> +
> +LABEL maintainer.name="The Xen Project" \
> +      maintainer.email="xen-devel@lists.xenproject.org"
> +
> +ENV DEBIAN_FRONTEND=noninteractive
> +ENV USER root
> +ENV PATH="/opt/gcc-11-ibt/bin:${PATH}"
> +
> +RUN mkdir /build
> +WORKDIR /build
> +
> +RUN apt-get update && \
> +    apt-get --quiet --yes install \
> +        bison \
> +        checkpolicy \
> +        flex \
> +        gawk \
> +        make \
> +        python3 \
> +        && \
> +        apt-get autoremove -y && \
> +        apt-get clean && \
> +        rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/*
> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
> index fdd5c76582b3..cc36428cf55b 100644
> --- a/automation/gitlab-ci/build.yaml
> +++ b/automation/gitlab-ci/build.yaml
> @@ -294,6 +294,12 @@ debian-stretch-32-gcc-debug:
>    variables:
>      CONTAINER: debian:stretch-i386
>  
> +debian-buster-gcc-ibt:
> +  extends: .gcc-x86-64-build
> +  variables:
> +    CONTAINER: debian:buster-gcc-ibt
> +    RANDCONFIG: y
> +
>  debian-unstable-clang:
>    extends: .clang-x86-64-build
>    variables:
> diff --git a/automation/scripts/containerize b/automation/scripts/containerize
> index 7682ccd34759..8992c67278ae 100755
> --- a/automation/scripts/containerize
> +++ b/automation/scripts/containerize
> @@ -33,6 +33,7 @@ case "_${CONTAINER}" in
>      _focal) CONTAINER="${BASE}/ubuntu:focal" ;;
>      _jessie) CONTAINER="${BASE}/debian:jessie" ;;
>      _stretch|_) CONTAINER="${BASE}/debian:stretch" ;;
> +    _buster-gcc-ibt) CONTAINER="${BASE}/debian:buster-gcc-ibt" ;;
>      _unstable|_) CONTAINER="${BASE}/debian:unstable" ;;
>      _trusty) CONTAINER="${BASE}/ubuntu:trusty" ;;
>      _xenial) CONTAINER="${BASE}/ubuntu:xenial" ;;
> diff --git a/xen/arch/x86/arch.mk b/xen/arch/x86/arch.mk
> index f780c912a9cf..92fd19811013 100644
> --- a/xen/arch/x86/arch.mk
> +++ b/xen/arch/x86/arch.mk
> @@ -54,6 +54,7 @@ endif
>  
>  ifdef CONFIG_XEN_IBT
>  CFLAGS += -fcf-protection=branch -mmanual-endbr
> +$(call cc-option-add,CFLAGS,CC,-fcf-check-attribute=no)
>  else
>  $(call cc-option-add,CFLAGS,CC,-fcf-protection=none)
>  endif