[meta-virtualization] [PATCH] xen: fix build with _FORTIFY_SOURCE, gcc 8.2 and glibc 2.28

Tue Sep 25 14:40:19 PDT 2018

merged

Bruce

On Tue, Sep 25, 2018 at 1:40 PM,  <christopher.w.clark at gmail.com> wrote:
> From: Christopher Clark <christopher.w.clark at gmail.com>
>
> Apply upstream-reviewed patch to fix compilation.
> Patch reorders header includes to issue a pragma to disable SSE before
> including any potentially always_inline functions that would use SSE.
>
> Also modify the recipe to supply compiler flags via the tools variables where
> they will get used, necessary as _FORTIFY_SOURCE requires optimization flags to
> be supplied.
>
> Signed-off-by: Christopher Clark <christopher.clark6 at baesystems.com>
> ---
>  .../xen/files/xen-disable-sse-before-inlines.patch | 140 +++++++++++++++++++++
>  recipes-extended/xen/xen.inc                       |   6 +
>  recipes-extended/xen/xen_4.11.0.bb                 |   1 +
>  recipes-extended/xen/xen_git.bb                    |   1 +
>  4 files changed, 148 insertions(+)
>  create mode 100644 recipes-extended/xen/files/xen-disable-sse-before-inlines.patch
>
> diff --git a/recipes-extended/xen/files/xen-disable-sse-before-inlines.patch b/recipes-extended/xen/files/xen-disable-sse-before-inlines.patch
> new file mode 100644
> index 0000000..54a28ee
> --- /dev/null
> +++ b/recipes-extended/xen/files/xen-disable-sse-before-inlines.patch
> @@ -0,0 +1,140 @@
> +From 6d50ae155c0f736aa6239eabf1bc8c8e3704742d Mon Sep 17 00:00:00 2001
> +From: Christopher Clark <christopher.w.clark at gmail.com>
> +Date: Fri, 21 Sep 2018 08:28:02 -0700
> +Subject: [PATCH v2] fuzz, test x86_emulator: disable sse before including
> + always_inline fns
> +To: xen-devel at lists.xenproject.org,
> +    jbeulich at suse.com
> +Cc: ian.jackson at eu.citrix.com,
> +    wei.liu2 at citrix.com,
> +    andrew.cooper3 at citrix.com
> +
> +Workaround for compiler rejection of SSE-using always_inlines defined before
> +SSE is disabled.
> +
> +Compiling with _FORTIFY_SOURCE or higher levels of optimization enabled
> +will always_inline several library fns (memset, memcpy, ...)
> +(with gcc 8.2.0 and glibc 2.28).
> +
> +In fuzz and x86_emulator test, the compiler is instructed not
> +to generate SSE instructions via: #pragma GCC target("no-sse")
> +because those registers are needed for use by the workload.
> +
> +The combination above causes compilation failure as the inline functions
> +use those instructions. This is resolved by reordering the inclusion of
> +<stdio.h> and <string.h> to after the pragma disabling SSE generation.
> +
> +It would be preferable to locate the no-sse pragma within x86-emulate.h at the
> +top of the file, prior to including any other headers; unfortunately doing so
> +before <stdlib.h> causes compilation failure due to declaration of 'atof' with:
> +  "SSE register return with SSE disabled".
> +Fortunately there is no (known) current dependency on any always_inline
> +SSE-inclined function declared in <stdlib.h> or any of its dependencies, so the
> +pragma is therefore issued immediately after inclusion of <stdlib.h> with a
> +comment introduced to explain its location there.
> +
> +Add compile-time checks for unwanted prior inclusion of <string.h> and
> +<stdio.h>, which are the two headers that provide the library functions that
> +are handled with wrappers and listed within "x86-emulate.h" as ones "we think
> +might access any of the FPU state".
> +* Use standard-defined "EOF" macro to detect prior <stdio.h> inclusion.
> +* Use "_STRING_H" (non-standardized guard macro) as best-effort
> +  for detection of prior <string.h> inclusion. This is non-universally
> +  viable but will provide error output on common GLIBC systems, so
> +  provides some defensive coverage.
> +
> +Adds conditional #include <stdio.h> to x86-emulate.h because fwrite, printf,
> +etc. are referenced when WRAP has been defined.
> +
> +Signed-off-by: Christopher Clark <christopher.clark6 at baesystems.com>
> +Reviewed-by: Jan Beulich <jbeulich at suse.com>
> +---
> + tools/fuzz/x86_instruction_emulator/fuzz-emul.c | 10 +++++++--
> + tools/tests/x86_emulator/wrappers.c             |  1 -
> + tools/tests/x86_emulator/x86-emulate.h          | 28 +++++++++++++++++++++++--
> + 3 files changed, 34 insertions(+), 5 deletions(-)
> +
> +diff --git a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> +index 03a2473..0ffd0fb 100644
> +--- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> ++++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
> +@@ -6,9 +6,7 @@
> + #include <stdbool.h>
> + #include <stddef.h>
> + #include <stdint.h>
> +-#include <stdio.h>
> + #include <stdlib.h>
> +-#include <string.h>
> + #include <sys/types.h>
> + #include <sys/stat.h>
> + #include <sys/mman.h>
> +@@ -16,6 +14,14 @@
> + #include <xen/xen.h>
> +
> + #include "x86-emulate.h"
> ++/*
> ++ * include "x86-emulate.h" prior to <stdio.h> and <string.h>:
> ++ * x86-emulate.h disables use of SSE registers, while <stdio.h> and <string.h>
> ++ * declare functions that may be always_inline and use those registers
> ++ * unless they have been disabled earlier, which can fail to compile.
> ++ */
> ++#include <stdio.h>
> ++#include <string.h>
> + #include "fuzz-emul.h"
> +
> + #define MSR_INDEX_MAX 16
> +diff --git a/tools/tests/x86_emulator/wrappers.c b/tools/tests/x86_emulator/wrappers.c
> +index d02013c..eba7cc9 100644
> +--- a/tools/tests/x86_emulator/wrappers.c
> ++++ b/tools/tests/x86_emulator/wrappers.c
> +@@ -1,5 +1,4 @@
> + #include <stdarg.h>
> +-#include <stdio.h>
> +
> + #define WRAP(x) typeof(x) emul_##x
> + #include "x86-emulate.h"
> +diff --git a/tools/tests/x86_emulator/x86-emulate.h b/tools/tests/x86_emulator/x86-emulate.h
> +index b249e46..07ea1e8 100644
> +--- a/tools/tests/x86_emulator/x86-emulate.h
> ++++ b/tools/tests/x86_emulator/x86-emulate.h
> +@@ -3,11 +3,35 @@
> + #include <stddef.h>
> + #include <stdint.h>
> + #include <stdlib.h>
> +-#include <string.h>
> +-
> ++/*
> ++ * Use of sse registers must be disabled prior to the definition of
> ++ * always_inline functions that would use them (memcpy, memset, etc),
> ++ * so do this as early as possible, aiming to be before any always_inline
> ++ * functions that are used are declared.
> ++ * Unfortunately, this cannot be done prior to inclusion of <stdlib.h>
> ++ * due to functions such as 'atof' that have SSE register return declared,
> ++ * so do so here, immediately after that.
> ++ */
> + #if __GNUC__ >= 6
> + #pragma GCC target("no-sse")
> + #endif
> ++ /*
> ++ * Attempt detection of unwanted prior inclusion of some headers known to use
> ++ * always_inline with SSE registers in some library / compiler / optimization
> ++ * combinations.
> ++ */
> ++#ifdef _STRING_H
> ++#error "Must not include <string.h> before x86-emulate.h"
> ++#endif
> ++#include <string.h>
> ++
> ++/* EOF is a standard macro defined in <stdio.h> so use it for detection */
> ++#ifdef EOF
> ++#error "Must not include <stdio.h> before x86-emulate.h"
> ++#endif
> ++#ifdef WRAP
> ++#include <stdio.h>
> ++#endif
> +
> + #include <xen/xen.h>
> +
> +--
> +2.1.4
> +
> diff --git a/recipes-extended/xen/xen.inc b/recipes-extended/xen/xen.inc
> index bf4b3c2..365f71d 100644
> --- a/recipes-extended/xen/xen.inc
> +++ b/recipes-extended/xen/xen.inc
> @@ -842,6 +842,12 @@ export LDFLAGS=""
>  # patching the build to be ok with this
>  TUNE_CCARGS := "${@oe.utils.str_filter_out('-mfpmath=sse', '${TUNE_CCARGS}', d)}"
>
> +# Supply the full set of compiler flags via the tool variables
> +# Yocto supplys _FORTIFY_SOURCE via CC/CPP/CXX but optimization -O via C*FLAGS
> +CC_append = " ${CFLAGS}"
> +CPP_append = " ${CPPFLAGS}"
> +CXX_append = " ${CXXFLAGS}"
> +
>  EXTRA_OECONF += " \
>      --exec-prefix=/usr \
>      --prefix=/usr \
> diff --git a/recipes-extended/xen/xen_4.11.0.bb b/recipes-extended/xen/xen_4.11.0.bb
> index 40e7340..b275066 100644
> --- a/recipes-extended/xen/xen_4.11.0.bb
> +++ b/recipes-extended/xen/xen_4.11.0.bb
> @@ -6,6 +6,7 @@ SRC_URI = " \
>      file://tools-xentop-vwprintw.patch \
>      file://xen-4.11-arm-acpi-fix-string-lengths.patch \
>      file://xen-tools-xenpmd-snprintf.patch \
> +    file://xen-disable-sse-before-inlines.patch \
>      "
>
>  SRC_URI[md5sum] = "cbec0600284921744bc14119f4ed3fff"
> diff --git a/recipes-extended/xen/xen_git.bb b/recipes-extended/xen/xen_git.bb
> index da1eea6..81ee100 100644
> --- a/recipes-extended/xen/xen_git.bb
> +++ b/recipes-extended/xen/xen_git.bb
> @@ -12,6 +12,7 @@ S = "${WORKDIR}/git"
>
>  SRC_URI = " \
>      git://xenbits.xen.org/xen.git;branch=${XEN_BRANCH} \
> +    file://xen-disable-sse-before-inlines.patch \
>      "
>
>  DEFAULT_PREFERENCE = "-1"
> --
> 2.1.4
>


-- 
"Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end"